[26.02] Allows inconsistent reads in concurrent cache during shutdown

Author: reederc42

core/concurrent_cache/concurrent_cache.go

@@ -218,8 +218,10 @@ func Lock(ctx context.Context, queries ...[]Subquery) (results []Result, unlocke
 
 	roots := make([][]int, len(queries))
 	cachesPresent := make(map[resource]struct{}, resourceCount)
+	// if there are no consistent locks, we don't add the root lock
+	hasConsistentLocks := false
 	// phase 1, requires no locks, check errors, dedupe, and build trees
-	if err = assembleQueries(queries, roots, cachesPresent); err != nil {
+	if hasConsistentLocks, err = assembleQueries(queries, roots, cachesPresent); err != nil {
 		return nil, func() {}, err
 	}
 
@@ -229,30 +231,34 @@ func Lock(ctx context.Context, queries ...[]Subquery) (results []Result, unlocke
 	}
 
 	// phase 3, takes per-resource locks
-	merged := mergeQueries(queries)
+	merged := mergeQueries(queries, hasConsistentLocks)
 	results, unlocker, err = lockQuery(merged, len(queries))
 	if err == nil && ctx.Err() != nil {
 		err = ctx.Err()
 	}
 	return results, unlocker, err
 }
 
-func assembleQueries(queries [][]Subquery, roots [][]int, cachesPresent map[resource]struct{}) error {
+func assembleQueries(queries [][]Subquery, roots [][]int, cachesPresent map[resource]struct{}) (bool, error) {
+	hasConsistentLocks := false
 	for i, q := range queries {
 		if err := checkError(q); err != nil {
-			return err
+			return false, err
 		}
 		ri, err := dedupe(q)
 		if err != nil {
-			return err
+			return false, err
 		}
 		for _, sq := range q {
 			cachesPresent[sq.res] = struct{}{}
+			if sq.op != list && sq.op != inconsistentRead {
+				hasConsistentLocks = true
+			}
 		}
 		queries[i], roots[i] = buildTrees(ri, q)
 	}
 
-	return nil
+	return hasConsistentLocks, nil
 }
 
 func lockCachesAndFillInIDs(queries [][]Subquery, roots [][]int, cachesPresent map[resource]struct{}) error {
@@ -486,14 +492,16 @@ func fillInIDs(root int, query []Subquery) error {
 	return nil
 }
 
-func mergeQueries(queries [][]Subquery) []Subquery {
+func mergeQueries(queries [][]Subquery, hasConsistentLocks bool) []Subquery {
 	length := 1
 	for _, q := range queries {
 		length += len(q)
 	}
 	merged := make([]Subquery, 0, length)
-	// always add implied read root, if write root is present it will take precedence
-	merged = append(merged, Subquery{res: root, op: read, id: "."})
+	// if there are consistent locks, we need to add the root lock to block during shutdown. Inconsistent reads are still allowed.
+	if hasConsistentLocks {
+		merged = append(merged, Subquery{res: root, op: read, id: "."})
+	}
 	for i, q := range queries {
 		for j := range q {
 			q[j].result = i

core/concurrent_cache/concurrent_cache_test.go

@@ -226,7 +226,7 @@ func TestMergeQueries(t *testing.T) {
 				queries = append(queries, q)
 			}
 
-			merged := mergeQueries(queries)
+			merged := mergeQueries(queries, true)
 			assert.Len(t, merged, 14)
 		})
 	}
@@ -1041,3 +1041,18 @@ func TestCheckDependency(t *testing.T) {
 		})
 	}
 }
+
+// TestAllowInconsistentReadWithRootLock tests that inconsistent reads and lists can occur while the root lock is held.
+func TestAllowInconsistentReadWithRootLock(t *testing.T) {
+	initCaches()
+
+	_, unlocker, err := Lock(context.Background(), Query(LockCache()))
+	defer unlocker()
+
+	assert.NoError(t, err)
+	results, unlocker2, err := Lock(context.Background(), Query(InconsistentReadBackend("backend1"), ListVolumes()))
+	defer unlocker2()
+	assert.NoError(t, err)
+	assert.NotNil(t, results[0].Backend.Read)
+	assert.NotEmpty(t, results[0].Volumes)
+}