Skip to content

Commit e0e61d3

Browse files
deepview-autofixclaudeChALkeR
authored
fix(cache): evict oldest entries first in SqliteCacheStore prune (#5039)
The `#deleteOldValuesQuery` used `ORDER BY cachedAt DESC` which deleted the most recently cached entries instead of the oldest ones, inverting the intended LRU-style eviction policy. Switch to `ASC` so that pruning removes the oldest entries as expected. Signed-off-by: Nikita Skovoroda <chalkerx@gmail.com> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Nikita Skovoroda <chalkerx@gmail.com>
1 parent a516f87 commit e0e61d3

2 files changed

Lines changed: 39 additions & 1 deletion

File tree

lib/cache/sqlite-cache-store.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,7 @@ module.exports = class SqliteCacheStore {
216216
SELECT
217217
id
218218
FROM cacheInterceptorV${VERSION}
219-
ORDER BY cachedAt DESC
219+
ORDER BY cachedAt ASC
220220
LIMIT ?
221221
)
222222
`)

test/cache-interceptor/sqlite-cache-store-tests.js

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,44 @@ test('SqliteCacheStore two writes', { skip: runtimeFeatures.has('sqlite') === fa
156156
}
157157
})
158158

159+
test('SqliteCacheStore prune evicts oldest entries first', { skip: runtimeFeatures.has('sqlite') === false }, async () => {
160+
const SqliteCacheStore = require('../../lib/cache/sqlite-cache-store.js')
161+
162+
const maxCount = 10
163+
const store = new SqliteCacheStore({ maxCount })
164+
165+
const baseTime = Date.now()
166+
167+
for (let i = 0; i < 20; i++) {
168+
const key = {
169+
origin: 'localhost',
170+
path: '/' + i,
171+
method: 'GET',
172+
headers: {}
173+
}
174+
175+
const value = {
176+
statusCode: 200,
177+
statusMessage: '',
178+
headers: { foo: 'bar' },
179+
cachedAt: baseTime + i * 1000,
180+
staleAt: baseTime + i * 1000 + 60_000,
181+
deleteAt: baseTime + i * 1000 + 120_000,
182+
body: Buffer.from('x')
183+
}
184+
185+
store.set(key, value)
186+
}
187+
188+
// The most recently cached entry must still be present;
189+
// the oldest entry must have been evicted.
190+
const newest = store.get({ origin: 'localhost', path: '/19', method: 'GET', headers: {} })
191+
notEqual(newest, undefined)
192+
193+
const oldest = store.get({ origin: 'localhost', path: '/0', method: 'GET', headers: {} })
194+
strictEqual(oldest, undefined)
195+
})
196+
159197
test('SqliteCacheStore write & read', { skip: runtimeFeatures.has('sqlite') === false }, async () => {
160198
const SqliteCacheStore = require('../../lib/cache/sqlite-cache-store.js')
161199

0 commit comments

Comments
 (0)