X-Git-Url: https://git.arvados.org/arvados.git/blobdiff_plain/3dbbbd49b1889d832c295a4bb69dd360abf614fa..d4ed3e6460469f2766e1f1676c538d6c86e000b6:/lib/controller/federation/list.go

diff --git a/lib/controller/federation/list.go b/lib/controller/federation/list.go
index 5a171c9c37..414870d24f 100644
--- a/lib/controller/federation/list.go
+++ b/lib/controller/federation/list.go
@@ -68,8 +68,7 @@ func (conn *Conn) CollectionList(ctx context.Context, options arvados.ListOption
 //
 // * len(Order)==0
 //
-// * there are no filters other than the "uuid = ..." and "uuid in
-//   ..." filters mentioned above.
+// * Each filter must be either "uuid = ..." or "uuid in [...]".
 //
 // * The maximum possible response size (total number of objects that
 //   could potentially be matched by all of the specified filters)
@@ -130,7 +129,8 @@ func (conn *Conn) splitListRequest(ctx context.Context, opts arvados.ListOptions
 		if matchAllFilters == nil {
 			matchAllFilters = matchThisFilter
 		} else {
-			// matchAllFilters = intersect(matchAllFilters, matchThisFilter)
+			// Reduce matchAllFilters to the intersection
+			// of matchAllFilters ∩ matchThisFilter.
 			for uuid := range matchAllFilters {
 				if !matchThisFilter[uuid] {
 					delete(matchAllFilters, uuid)
@@ -139,6 +139,11 @@ func (conn *Conn) splitListRequest(ctx context.Context, opts arvados.ListOptions
 		}
 	}
 
+	// Collate UUIDs in matchAllFilters by remote cluster ID --
+	// e.g., todoByRemote["aaaaa"]["aaaaa-4zz18-000000000000000"]
+	// will be true -- and count the total number of UUIDs we're
+	// filtering on, so we can compare it to our max page size
+	// limit.
 	nUUIDs := 0
 	todoByRemote := map[string]map[string]bool{}
 	for uuid := range matchAllFilters {
@@ -155,7 +160,7 @@ func (conn *Conn) splitListRequest(ctx context.Context, opts arvados.ListOptions
 
 	if len(todoByRemote) > 1 {
 		if cannotSplit {
-			return httpErrorf(http.StatusBadRequest, "cannot execute federated list query with filters other than 'uuid = ...' and 'uuid in [...]'")
+			return httpErrorf(http.StatusBadRequest, "cannot execute federated list query: each filter must be either 'uuid = ...' or 'uuid in [...]'")
 		}
 		if opts.Count != "none" {
 			return httpErrorf(http.StatusBadRequest, "cannot execute federated list query unless count==\"none\"")
@@ -181,14 +186,14 @@ func (conn *Conn) splitListRequest(ctx context.Context, opts arvados.ListOptions
 	defer cancel()
 	errs := make(chan error, len(todoByRemote))
 	for clusterID, todo := range todoByRemote {
-		clusterID, todo := clusterID, todo
-		batch := make([]string, 0, len(todo))
-		for uuid := range todo {
-			batch = append(batch, uuid)
-		}
-		go func() {
+		go func(clusterID string, todo map[string]bool) {
 			// This goroutine sends exactly one value to
 			// errs.
+			batch := make([]string, 0, len(todo))
+			for uuid := range todo {
+				batch = append(batch, uuid)
+			}
+
 			var backend arvados.API
 			if clusterID == conn.cluster.ClusterID {
 				backend = conn.local
@@ -225,13 +230,13 @@ func (conn *Conn) splitListRequest(ctx context.Context, opts arvados.ListOptions
 				}
 			}
 			errs <- nil
-		}()
+		}(clusterID, todo)
 	}
 
 	// Wait for all goroutines to return, then return the first
 	// non-nil error, if any.
 	var firstErr error
-	for i := 0; i < len(todoByRemote); i++ {
+	for range todoByRemote {
 		if err := <-errs; err != nil && firstErr == nil {
 			firstErr = err
 			// Signal to any remaining fn() calls that