Merge branch 'master' into 14715-keepprox-config

[arvados.git] / lib / controller / federation / list.go

diff --git a/lib/controller/federation/list.go b/lib/controller/federation/list.go
index 5a171c9c37992f128d14cf81028a00d113268db2..414870d24f5c42912785c4b29efa1fe4f91ec786 100644 (file)
--- 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