Skip to content

Commit 8ea9aa2

Browse files
committed
fix(client): stop ExpiredShapesCache writing to localStorage on every read
getExpiredHandle is a read accessor that also persisted the whole cache, and it runs while building the URL of every shape request. In live mode that is a full JSON.stringify plus localStorage.setItem once per poll, indefinitely. lastUsed is still updated in memory, and markExpired reads it from memory when picking an eviction candidate, so LRU ordering within a session is unchanged. After a reload the persisted values reflect the last markExpired rather than the last read, so eviction order at the 250-entry cap can be staler.
1 parent 0f40420 commit 8ea9aa2

3 files changed

Lines changed: 105 additions & 2 deletions

File tree

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
---
2+
'@electric-sql/client': patch
3+
---
4+
5+
Stop `ExpiredShapesCache.getExpiredHandle` writing the whole cache to
6+
`localStorage` on every read. The LRU touch it performs runs while building the
7+
URL of every shape request, so in live mode it persisted the entire cache once
8+
per poll for as long as the app stayed open. `lastUsed` is now updated in memory
9+
only; it is read from memory when `markExpired` picks an eviction candidate, so
10+
the sole effect is that eviction order can be staler after a reload.

‎packages/typescript-client/src/expired-shapes-cache.ts‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,12 @@ export class ExpiredShapesCache {
1414
getExpiredHandle(shapeUrl: string): string | null {
1515
const entry = this.data[shapeUrl]
1616
if (entry) {
17-
// Update last used time when accessed
17+
// Update last used time when accessed. Deliberately not persisted: this runs
18+
// while building the URL of every shape request, so persisting it writes the
19+
// whole cache to localStorage on every poll for as long as the app is open.
20+
// `lastUsed` only orders eviction in markExpired, which reads it from memory,
21+
// so the only cost of a staler value on disk is eviction order after a reload.
1822
entry.lastUsed = Date.now()
19-
this.save()
2023
return entry.expiredHandle
2124
}
2225
return null

‎packages/typescript-client/test/expired-shapes-cache.test.ts‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1312,3 +1312,93 @@ describe(`ExpiredShapesCache`, () => {
13121312
expect(snapshotHasCacheBuster).toBe(false)
13131313
})
13141314
})
1315+
1316+
describe(`ExpiredShapesCache write amplification`, () => {
1317+
const shapeUrl = `https://example.com/v1/shape?table=test`
1318+
1319+
beforeEach(() => {
1320+
localStorage.clear()
1321+
vi.restoreAllMocks()
1322+
})
1323+
1324+
// The localStorage mock's setItem is a persistent vi.fn whose call history
1325+
// survives restoreAllMocks, so measure against a baseline taken here rather
1326+
// than from zero.
1327+
function countWrites(): { calls: () => number } {
1328+
const spy = vi.spyOn(localStorage, `setItem`)
1329+
const base = spy.mock.calls.length
1330+
return { calls: () => spy.mock.calls.length - base }
1331+
}
1332+
1333+
// Positive control: without this, a spy that silently failed to attach would
1334+
// make every "no writes" assertion below pass for the wrong reason.
1335+
it(`counts a write when the cache is mutated`, () => {
1336+
const cache = new ExpiredShapesCache()
1337+
const writes = countWrites()
1338+
cache.markExpired(shapeUrl, `handle-1`)
1339+
expect(writes.calls()).toBe(1)
1340+
})
1341+
1342+
it(`does not write to localStorage when reading an existing entry`, () => {
1343+
const cache = new ExpiredShapesCache()
1344+
cache.markExpired(shapeUrl, `handle-1`)
1345+
1346+
const writes = countWrites()
1347+
for (let i = 0; i < 25; i++) {
1348+
expect(cache.getExpiredHandle(shapeUrl)).toBe(`handle-1`)
1349+
}
1350+
1351+
expect(writes.calls()).toBe(0)
1352+
})
1353+
1354+
it(`does not write to localStorage when reading a missing entry`, () => {
1355+
const cache = new ExpiredShapesCache()
1356+
1357+
const writes = countWrites()
1358+
expect(cache.getExpiredHandle(shapeUrl)).toBe(null)
1359+
1360+
expect(writes.calls()).toBe(0)
1361+
})
1362+
1363+
it(`still persists the entry so a later cache reloads it`, () => {
1364+
const cache = new ExpiredShapesCache()
1365+
cache.markExpired(shapeUrl, `handle-1`)
1366+
for (let i = 0; i < 25; i++) cache.getExpiredHandle(shapeUrl)
1367+
1368+
const reloaded = new ExpiredShapesCache()
1369+
expect(reloaded.getExpiredHandle(shapeUrl)).toBe(`handle-1`)
1370+
})
1371+
1372+
it(`still persists a changed handle`, () => {
1373+
const cache = new ExpiredShapesCache()
1374+
cache.markExpired(shapeUrl, `handle-1`)
1375+
cache.getExpiredHandle(shapeUrl)
1376+
cache.markExpired(shapeUrl, `handle-2`)
1377+
1378+
expect(new ExpiredShapesCache().getExpiredHandle(shapeUrl)).toBe(`handle-2`)
1379+
})
1380+
1381+
it(`still persists a delete`, () => {
1382+
const cache = new ExpiredShapesCache()
1383+
cache.markExpired(shapeUrl, `handle-1`)
1384+
cache.delete(shapeUrl)
1385+
1386+
expect(new ExpiredShapesCache().getExpiredHandle(shapeUrl)).toBe(null)
1387+
})
1388+
1389+
it(`keeps LRU ordering accurate in memory despite not persisting each touch`, () => {
1390+
const cache = new ExpiredShapesCache()
1391+
// Fill to the 250-entry cap.
1392+
for (let i = 0; i < 250; i++)
1393+
cache.markExpired(`${shapeUrl}&i=${i}`, `h${i}`)
1394+
1395+
// Touch the oldest so it is no longer the eviction candidate.
1396+
cache.getExpiredHandle(`${shapeUrl}&i=0`)
1397+
1398+
// Overflow by one; the least recently used entry should go, not the touched one.
1399+
cache.markExpired(`${shapeUrl}&overflow`, `h-overflow`)
1400+
1401+
expect(cache.getExpiredHandle(`${shapeUrl}&i=0`)).toBe(`h0`)
1402+
expect(cache.getExpiredHandle(`${shapeUrl}&i=1`)).toBe(null)
1403+
})
1404+
})

0 commit comments

Comments
 (0)