← index
πŸ“¦ platform/cache-service Private
β—΄ Code
βŠ™ Issues 28
⇑ Pull requests 7
β–Ά Actions
β–¦ Projects
β–€ Wiki
πŸ›‘ Security
πŸ“‰ Insights

Fix cache eviction race under concurrent writes #4821

Open rafael-silva wants to merge 3 commits into main from fix/eviction-race
πŸ’¬ Conversation 8
βŠ• Commits 3
β¬’ Checks 5
πŸ“„ Files changed 4
βœ“

This branch has no conflicts with the base branch

Merging can be performed automatically.

βœ“

All checks have passed

5 successful checks Β· ci / unit, ci / integration, ci / lint, codeql, deploy-preview

⚠

1 review required Β· approved by hana-takeda

Waiting on @platform/oncall before this can be merged.

Showing 4 changed files with 128 additions and 10 deletions.
β–Ύ src/cache/evict.ts +24 -9
☐ Viewed
……@@ -18,14 +18,29 @@ export async function evictAndReload(key: string) {
1818 const redis = getClient();
1919 const existing = await redis.get(key);
2020 if (!existing) return null;
21 // TODO: add lock before eviction to prevent races
22 - await redis.del(key);
23 - const fresh = await loadFromSource(key);
24 - await redis.set(key, fresh, 'EX', 300);
25 - return fresh;
21+ const lease = await acquireLease(`lease:${key}`, { ttl: 5000 });
22+ if (!lease) {
23+ // another worker is already rebuilding; serve stale and bail
24+ metrics.inc('cache.evict.contended');
25+ return parse(existing);
26+ }
27+ try {
28+ const fresh = await loadFromSource(key);
29+ await redis.set(key, stringify(fresh), 'EX', 300);
30+ return fresh;
31+ } finally {
32+ await lease.release();
33+ }
2634}
HT
hana-takeda commented on line 28 3 hours ago
Nice. One nit: should we wrap loadFromSource(key) in a timeout so a slow origin doesn't hold the lease the full 5s? Even 2s would cap the blast radius on the checkout path.
β–Ύ src/cache/lease.ts +38 -0
☐ Viewed
……@@ -0,0 +1,38 @@
1+// Token-based lease using SET NX + unique owner id.
2+// Release is safe: only the owner can release their own lease.
3+import { randomUUID } from 'node:crypto';
4+import { getClient } from './client';
5+
6+export async function acquireLease(key: string, opts: { ttl: number }) {
7+ const token = randomUUID();
8+ const ok = await getClient().set(key, token, 'PX', opts.ttl, 'NX');
9+ if (!ok) return null;
10+ return { release: () => releaseLease(key, token) };
11+}
TM Finish your review

Commits on this branch

RS Add token-based lease around cache eviction
a1b2c3d
RS Add race test covering 10k concurrent writes
e4f5g6h
RS Emit metrics for contended eviction paths
i7j8k9l