Skip to content

Commit 2427062

Browse files
jensneuseclaude
andcommitted
refactor(cache): per-entry TTL on CacheEntry, bulk L2 Set, Items log shape
Three intertwined changes shipped together because they touch the same cache fixture and call sites. 1. Per-entry TTL (interface change) - CacheEntry.TTL field added. - LoaderCache.Set drops its trailing ttl parameter. - Each CacheEntry now carries its own TTL, enabling mixed-TTL bulk writes in a single backend call. 2. Bulk L2 Set per cache instance - writeL2CacheSetContributors groups all per-fetch L2 writes by LoaderCache identity and issues one Set per instance. - Combined regular + negative entries (was 2 Sets per fetch, now 1). - Cross-fetch grouping in Phase 4 collapses N root-field fetches against the same cache into a single bulk Set. - Failure semantics preserved: bulk Set error records one CacheOperationError per contributing fetch, matching the bulkL2Lookup pattern. 3. CacheLogEntry.Items shape (test fixture) - Replaces parallel Keys/Hits/TTL/TTLs slices with a single Items []CacheLogItem slice. Each item bundles {Key, Hit, TTL} so the relationship between key and per-operation field is explicit instead of positional. - Single sortCacheLogEntries helper (was two). Also folds Task 4a in for the same files: TestRootFieldSplitByDatasource moved to its own sibling file with self-contained subtests per execution/engine/CLAUDE.md (no shared helpers, inline setup). Public API break: cosmo router and any downstream LoaderCache impl must update to the new Set signature. The feature has not been released yet, so no compat shim. Tests: full v2/pkg/* and execution/* suites green. Addresses the engineering question raised on PR #1259 about whether multiple root-field cache writes could be a single Redis round trip: yes, and now they are. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 77ea732 commit 2427062

31 files changed

Lines changed: 2019 additions & 2602 deletions

execution/engine/federation_caching_batch_test.go

Lines changed: 106 additions & 236 deletions
Large diffs are not rendered by default.

execution/engine/federation_caching_entity_field_args_test.go

Lines changed: 268 additions & 151 deletions
Large diffs are not rendered by default.

execution/engine/federation_caching_ext_invalidation_test.go

Lines changed: 54 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -44,17 +44,17 @@ func TestFederationCaching_ExtensionsInvalidation(t *testing.T) {
4444
resp := env.queryEntity(entityQuery)
4545
assert.Equal(t, entityResponseMe, resp)
4646
assert.Equal(t, 1, env.accountsCalls(), "first request fetches from accounts")
47-
assert.Equal(t, sortCacheLogKeys([]CacheLogEntry{
48-
{Operation: "get", Keys: []string{userKey}, Hits: []bool{false}}, // L2 empty on first request
49-
{Operation: "set", Keys: []string{userKey}}, // populate L2 after fetch
47+
assert.Equal(t, sortCacheLogEntries([]CacheLogEntry{
48+
{Operation: "get", Items: []CacheLogItem{{Key: userKey, Hit: false}}}, // L2 empty on first request
49+
{Operation: "set", Items: []CacheLogItem{{Key: userKey, TTL: 30 * time.Second}}}, // populate L2 after fetch
5050
}), env.cacheLog())
5151

5252
// Step 2: Same query — L2 hit, no subgraph call.
5353
resp = env.queryEntity(entityQuery)
5454
assert.Equal(t, entityResponseMe, resp)
5555
assert.Equal(t, 0, env.accountsCalls(), "L2 cache hit")
56-
assert.Equal(t, sortCacheLogKeys([]CacheLogEntry{
57-
{Operation: "get", Keys: []string{userKey}, Hits: []bool{true}}, // L2 hit from Step 1
56+
assert.Equal(t, sortCacheLogEntries([]CacheLogEntry{
57+
{Operation: "get", Items: []CacheLogItem{{Key: userKey, Hit: true}}}, // L2 hit from Step 1
5858
}), env.cacheLog())
5959

6060
// Step 3: Mutation with cacheInvalidation extensions deletes User:1234.
@@ -66,17 +66,17 @@ func TestFederationCaching_ExtensionsInvalidation(t *testing.T) {
6666
mutResp := env.mutate(mutationQuery)
6767
assert.Equal(t, mutationResponse, mutResp)
6868
env.clearModifier()
69-
assert.Equal(t, sortCacheLogKeys([]CacheLogEntry{
70-
{Operation: "delete", Keys: []string{userKey}}, // extensions-based invalidation
69+
assert.Equal(t, sortCacheLogEntries([]CacheLogEntry{
70+
{Operation: "delete", Items: []CacheLogItem{{Key: userKey}}}, // extensions-based invalidation
7171
}), env.cacheLog())
7272

7373
// Step 4: Re-query — L2 miss after invalidation, fetches updated username.
7474
resp = env.queryEntity(entityQuery)
7575
assert.Equal(t, entityResponseUpdated, resp)
7676
assert.Equal(t, 1, env.accountsCalls(), "re-fetched after invalidation")
77-
assert.Equal(t, sortCacheLogKeys([]CacheLogEntry{
78-
{Operation: "get", Keys: []string{userKey}, Hits: []bool{false}}, // L2 miss because Step 3 deleted it
79-
{Operation: "set", Keys: []string{userKey}}, // re-populate L2 after re-fetch
77+
assert.Equal(t, sortCacheLogEntries([]CacheLogEntry{
78+
{Operation: "get", Items: []CacheLogItem{{Key: userKey, Hit: false}}}, // L2 miss because Step 3 deleted it
79+
{Operation: "set", Items: []CacheLogItem{{Key: userKey, TTL: 30 * time.Second}}}, // re-populate L2 after re-fetch
8080
}), env.cacheLog())
8181
})
8282

@@ -105,16 +105,16 @@ func TestFederationCaching_ExtensionsInvalidation(t *testing.T) {
105105
mutResp := env.mutate(mutationQuery)
106106
assert.Equal(t, mutationResponse, mutResp)
107107
env.clearModifier()
108-
assert.Equal(t, sortCacheLogKeys([]CacheLogEntry{
109-
{Operation: "delete", Keys: []string{user9999Key}}, // delete called even though entry doesn't exist
108+
assert.Equal(t, sortCacheLogEntries([]CacheLogEntry{
109+
{Operation: "delete", Items: []CacheLogItem{{Key: user9999Key}}}, // delete called even though entry doesn't exist
110110
}), env.cacheLog())
111111

112112
// User:1234 should still be cached (unaffected by User:9999 invalidation).
113113
resp := env.queryEntity(entityQuery)
114114
assert.Equal(t, entityResponseMe, resp)
115115
assert.Equal(t, 0, env.accountsCalls(), "User:1234 still cached")
116-
assert.Equal(t, sortCacheLogKeys([]CacheLogEntry{
117-
{Operation: "get", Keys: []string{userKey}, Hits: []bool{true}}, // User:1234 still in L2
116+
assert.Equal(t, sortCacheLogEntries([]CacheLogEntry{
117+
{Operation: "get", Items: []CacheLogItem{{Key: userKey, Hit: true}}}, // User:1234 still in L2
118118
}), env.cacheLog())
119119
})
120120

@@ -139,19 +139,19 @@ func TestFederationCaching_ExtensionsInvalidation(t *testing.T) {
139139
})
140140
env.mutate(mutationQuery)
141141
env.clearModifier()
142-
assert.Equal(t, sortCacheLogKeys([]CacheLogEntry{
143-
{Operation: "delete", Keys: []string{
144-
`{"__typename":"User","key":{"id":"1234"}}`,
145-
`{"__typename":"User","key":{"id":"2345"}}`,
142+
assert.Equal(t, sortCacheLogEntries([]CacheLogEntry{
143+
{Operation: "delete", Items: []CacheLogItem{
144+
{Key: `{"__typename":"User","key":{"id":"1234"}}`},
145+
{Key: `{"__typename":"User","key":{"id":"2345"}}`},
146146
}}, // both entities deleted in single batch
147147
}), env.cacheLog())
148148

149149
// User:1234 must be re-fetched after invalidation.
150150
env.queryEntity(entityQuery)
151151
assert.Equal(t, 1, env.accountsCalls(), "re-fetched after invalidation")
152-
assert.Equal(t, sortCacheLogKeys([]CacheLogEntry{
153-
{Operation: "get", Keys: []string{userKey}, Hits: []bool{false}}, // L2 miss because mutation deleted it
154-
{Operation: "set", Keys: []string{userKey}}, // re-populate L2
152+
assert.Equal(t, sortCacheLogEntries([]CacheLogEntry{
153+
{Operation: "get", Items: []CacheLogItem{{Key: userKey, Hit: false}}}, // L2 miss because mutation deleted it
154+
{Operation: "set", Items: []CacheLogItem{{Key: userKey, TTL: 30 * time.Second}}}, // re-populate L2
155155
}), env.cacheLog())
156156
})
157157

@@ -176,14 +176,14 @@ func TestFederationCaching_ExtensionsInvalidation(t *testing.T) {
176176

177177
// Mutation WITHOUT extensions — no cache operations.
178178
env.mutate(mutationQuery)
179-
assert.Equal(t, sortCacheLogKeys([]CacheLogEntry{}), env.cacheLog(), "no cache operations for mutation without extensions")
179+
assert.Equal(t, sortCacheLogEntries([]CacheLogEntry{}), env.cacheLog(), "no cache operations for mutation without extensions")
180180

181181
// Cache should still be valid.
182182
resp = env.queryEntity(entityQuery)
183183
assert.Equal(t, entityResponseMe, resp)
184184
assert.Equal(t, 0, env.accountsCalls(), "cache still valid")
185-
assert.Equal(t, sortCacheLogKeys([]CacheLogEntry{
186-
{Operation: "get", Keys: []string{userKey}, Hits: []bool{true}}, // L2 still valid
185+
assert.Equal(t, sortCacheLogEntries([]CacheLogEntry{
186+
{Operation: "get", Items: []CacheLogItem{{Key: userKey, Hit: true}}}, // L2 still valid
187187
}), env.cacheLog())
188188
})
189189

@@ -215,8 +215,8 @@ func TestFederationCaching_ExtensionsInvalidation(t *testing.T) {
215215
})
216216
env.mutate(mutationQuery)
217217
env.clearModifier()
218-
assert.Equal(t, sortCacheLogKeys([]CacheLogEntry{
219-
{Operation: "delete", Keys: []string{userKey}}, // deduplicated: detectMutationEntityImpact fires, extensions-based skipped
218+
assert.Equal(t, sortCacheLogEntries([]CacheLogEntry{
219+
{Operation: "delete", Items: []CacheLogItem{{Key: userKey}}}, // deduplicated: detectMutationEntityImpact fires, extensions-based skipped
220220
}), env.cacheLog(), "single delete despite both mechanisms targeting same key")
221221

222222
// Cache invalidated — query should re-fetch.
@@ -260,9 +260,9 @@ func TestFederationCaching_ExtensionsInvalidation(t *testing.T) {
260260

261261
// Extensions-based delete is skipped because updateL2Cache will set the same
262262
// key with fresh data — only get(miss) + set remain.
263-
assert.Equal(t, sortCacheLogKeys([]CacheLogEntry{
264-
{Operation: "get", Keys: []string{userKey}, Hits: []bool{false}}, // L2 miss because we manually deleted it
265-
{Operation: "set", Keys: []string{userKey}}, // re-populate L2 (delete skipped: same key about to be set)
263+
assert.Equal(t, sortCacheLogEntries([]CacheLogEntry{
264+
{Operation: "get", Items: []CacheLogItem{{Key: userKey, Hit: false}}}, // L2 miss because we manually deleted it
265+
{Operation: "set", Items: []CacheLogItem{{Key: userKey, TTL: 30 * time.Second}}}, // re-populate L2 (delete skipped: same key about to be set)
266266
}), env.cacheLog())
267267
})
268268

@@ -281,16 +281,16 @@ func TestFederationCaching_ExtensionsInvalidation(t *testing.T) {
281281
// Populate cache (keys include header prefix).
282282
env.queryEntity(entityQuery)
283283
assert.Equal(t, 1, env.accountsCalls())
284-
assert.Equal(t, sortCacheLogKeys([]CacheLogEntry{
285-
{Operation: "get", Keys: []string{prefixedKey}, Hits: []bool{false}}, // L2 miss, prefixed key
286-
{Operation: "set", Keys: []string{prefixedKey}}, // populate L2 with prefixed key
284+
assert.Equal(t, sortCacheLogEntries([]CacheLogEntry{
285+
{Operation: "get", Items: []CacheLogItem{{Key: prefixedKey, Hit: false}}}, // L2 miss, prefixed key
286+
{Operation: "set", Items: []CacheLogItem{{Key: prefixedKey, TTL: 30 * time.Second}}}, // populate L2 with prefixed key
287287
}), env.cacheLog())
288288

289289
// Verify cache hit.
290290
env.queryEntity(entityQuery)
291291
assert.Equal(t, 0, env.accountsCalls(), "L2 cache hit")
292-
assert.Equal(t, sortCacheLogKeys([]CacheLogEntry{
293-
{Operation: "get", Keys: []string{prefixedKey}, Hits: []bool{true}}, // L2 hit with prefixed key
292+
assert.Equal(t, sortCacheLogEntries([]CacheLogEntry{
293+
{Operation: "get", Items: []CacheLogItem{{Key: prefixedKey, Hit: true}}}, // L2 hit with prefixed key
294294
}), env.cacheLog())
295295

296296
// Mutation with extensions invalidation.
@@ -301,16 +301,16 @@ func TestFederationCaching_ExtensionsInvalidation(t *testing.T) {
301301
})
302302
env.mutate(mutationQuery)
303303
env.clearModifier()
304-
assert.Equal(t, sortCacheLogKeys([]CacheLogEntry{
305-
{Operation: "delete", Keys: []string{prefixedKey}}, // delete key includes header prefix
304+
assert.Equal(t, sortCacheLogEntries([]CacheLogEntry{
305+
{Operation: "delete", Items: []CacheLogItem{{Key: prefixedKey}}}, // delete key includes header prefix
306306
}), env.cacheLog())
307307

308308
// Cache invalidated — re-fetch.
309309
env.queryEntity(entityQuery)
310310
assert.Equal(t, 1, env.accountsCalls(), "re-fetched after invalidation")
311-
assert.Equal(t, sortCacheLogKeys([]CacheLogEntry{
312-
{Operation: "get", Keys: []string{prefixedKey}, Hits: []bool{false}}, // L2 miss after delete
313-
{Operation: "set", Keys: []string{prefixedKey}}, // re-populate L2
311+
assert.Equal(t, sortCacheLogEntries([]CacheLogEntry{
312+
{Operation: "get", Items: []CacheLogItem{{Key: prefixedKey, Hit: false}}}, // L2 miss after delete
313+
{Operation: "set", Items: []CacheLogItem{{Key: prefixedKey, TTL: 30 * time.Second}}}, // re-populate L2
314314
}), env.cacheLog())
315315
})
316316

@@ -333,16 +333,16 @@ func TestFederationCaching_ExtensionsInvalidation(t *testing.T) {
333333
// Populate cache (keys include interceptor prefix).
334334
env.queryEntity(entityQuery)
335335
assert.Equal(t, 1, env.accountsCalls())
336-
assert.Equal(t, sortCacheLogKeys([]CacheLogEntry{
337-
{Operation: "get", Keys: []string{interceptedKey}, Hits: []bool{false}}, // L2 miss, intercepted key
338-
{Operation: "set", Keys: []string{interceptedKey}}, // populate L2 with intercepted key
336+
assert.Equal(t, sortCacheLogEntries([]CacheLogEntry{
337+
{Operation: "get", Items: []CacheLogItem{{Key: interceptedKey, Hit: false}}}, // L2 miss, intercepted key
338+
{Operation: "set", Items: []CacheLogItem{{Key: interceptedKey, TTL: 30 * time.Second}}}, // populate L2 with intercepted key
339339
}), env.cacheLog())
340340

341341
// Verify cache hit.
342342
env.queryEntity(entityQuery)
343343
assert.Equal(t, 0, env.accountsCalls(), "L2 cache hit")
344-
assert.Equal(t, sortCacheLogKeys([]CacheLogEntry{
345-
{Operation: "get", Keys: []string{interceptedKey}, Hits: []bool{true}}, // L2 hit with intercepted key
344+
assert.Equal(t, sortCacheLogEntries([]CacheLogEntry{
345+
{Operation: "get", Items: []CacheLogItem{{Key: interceptedKey, Hit: true}}}, // L2 hit with intercepted key
346346
}), env.cacheLog())
347347

348348
// Mutation with extensions invalidation.
@@ -353,16 +353,16 @@ func TestFederationCaching_ExtensionsInvalidation(t *testing.T) {
353353
})
354354
env.mutate(mutationQuery)
355355
env.clearModifier()
356-
assert.Equal(t, sortCacheLogKeys([]CacheLogEntry{
357-
{Operation: "delete", Keys: []string{interceptedKey}}, // delete key includes interceptor prefix
356+
assert.Equal(t, sortCacheLogEntries([]CacheLogEntry{
357+
{Operation: "delete", Items: []CacheLogItem{{Key: interceptedKey}}}, // delete key includes interceptor prefix
358358
}), env.cacheLog())
359359

360360
// Cache invalidated — re-fetch.
361361
env.queryEntity(entityQuery)
362362
assert.Equal(t, 1, env.accountsCalls(), "re-fetched after invalidation")
363-
assert.Equal(t, sortCacheLogKeys([]CacheLogEntry{
364-
{Operation: "get", Keys: []string{interceptedKey}, Hits: []bool{false}}, // L2 miss after delete
365-
{Operation: "set", Keys: []string{interceptedKey}}, // re-populate L2
363+
assert.Equal(t, sortCacheLogEntries([]CacheLogEntry{
364+
{Operation: "get", Items: []CacheLogItem{{Key: interceptedKey, Hit: false}}}, // L2 miss after delete
365+
{Operation: "set", Items: []CacheLogItem{{Key: interceptedKey, TTL: 30 * time.Second}}}, // re-populate L2
366366
}), env.cacheLog())
367367
})
368368

@@ -402,8 +402,8 @@ func TestFederationCaching_ExtensionsInvalidation(t *testing.T) {
402402
env.clearModifier()
403403

404404
// Cache should be invalidated despite errors in response.
405-
assert.Equal(t, sortCacheLogKeys([]CacheLogEntry{
406-
{Operation: "delete", Keys: []string{userKey}}, // invalidation runs despite errors
405+
assert.Equal(t, sortCacheLogEntries([]CacheLogEntry{
406+
{Operation: "delete", Items: []CacheLogItem{{Key: userKey}}}, // invalidation runs despite errors
407407
}), env.cacheLog())
408408

409409
// Re-query — L2 miss after invalidation, re-fetches updated data.
@@ -473,8 +473,8 @@ func TestFederationCaching_ExtensionsInvalidation(t *testing.T) {
473473
}), snap)
474474

475475
// Verify dedup still works — single delete despite both mechanisms.
476-
assert.Equal(t, sortCacheLogKeys([]CacheLogEntry{
477-
{Operation: "delete", Keys: []string{userKey}}, // config-based delete (extensions-based skipped via dedup)
476+
assert.Equal(t, sortCacheLogEntries([]CacheLogEntry{
477+
{Operation: "delete", Items: []CacheLogItem{{Key: userKey}}}, // config-based delete (extensions-based skipped via dedup)
478478
}), env.cacheLog(), "single delete despite both mechanisms; analytics must not read cache")
479479
})
480480

@@ -809,7 +809,7 @@ func (e *extInvalidationEnv) clearModifier() {
809809

810810
// cacheLog returns the current cache log with keys sorted for deterministic comparison.
811811
func (e *extInvalidationEnv) cacheLog() []CacheLogEntry {
812-
return sortCacheLogKeys(e.cache.GetLog())
812+
return sortCacheLogEntries(e.cache.GetLog())
813813
}
814814

815815
// accountsCalls returns the number of HTTP calls made to the accounts subgraph.

0 commit comments

Comments
 (0)