diff --git a/changelog.d/1326-terrain-cache-eviction.md b/changelog.d/1326-terrain-cache-eviction.md new file mode 100644 index 000000000..ccb374ccd --- /dev/null +++ b/changelog.d/1326-terrain-cache-eviction.md @@ -0,0 +1,3 @@ +- Evict terrain-height cache entries in fetch-time order without repeatedly + sorting the full cache, while keeping refreshed points newest and ordering + restored entries by their fetch time (daikaginza, #1326). diff --git a/server/providers/terrain.js b/server/providers/terrain.js index c678ab7c0..f4817f602 100644 --- a/server/providers/terrain.js +++ b/server/providers/terrain.js @@ -90,15 +90,23 @@ export function terrainHeightsProxy(options = {}) { /** * Drop the oldest entries until the map is back inside its ceiling. - * Oldest-first by fetch time, the same shape `trackBackfillProxies` uses. - * Terrain does not move, so evicting an old point costs one refetch, never - * a wrong answer. + * The map is kept in fetch-time order (sorted once on load, and the resolver + * re-inserts every point it refetches), so the oldest are at the front and + * no write sorts the map. Terrain does not move, so evicting an old point + * costs one refetch, never a wrong answer. */ function evictOverflow() { - if (mem.size <= CACHE_MAX_ENTRIES) return; + for (const key of mem.keys()) { + if (mem.size <= CACHE_MAX_ENTRIES) return; + mem.delete(key); + } + } + + /** Put the map in fetch-time order; a file is not guaranteed to be. */ + function orderByAge() { const byAge = [...mem.entries()].sort((a, b) => a[1].at - b[1].at); - for (let i = 0; i < byAge.length && mem.size > CACHE_MAX_ENTRIES; i += 1) - mem.delete(byAge[i][0]); + mem.clear(); + for (const [key, entry] of byAge) mem.set(key, entry); } /** @type {Map>>} single-flight per missing-point subset. */ const inflight = new Map(); @@ -165,6 +173,7 @@ export function terrainHeightsProxy(options = {}) { } diskDirty = mem.size > 0; } + orderByAge(); evictOverflow(); } catch { /* no disk cache yet */ diff --git a/src/data/terrainHeightsProxy.js b/src/data/terrainHeightsProxy.js index 9134f9ec6..c83b30a87 100644 --- a/src/data/terrainHeightsProxy.js +++ b/src/data/terrainHeightsProxy.js @@ -233,6 +233,9 @@ export async function resolveTerrainHeightRequest({ for (let i = 0; i < missing.length; i += 1) { const result = fetched[i]; if (validTerrainResult(result)) { + // Re-insert rather than overwrite: a Map keeps an existing key in + // its old position, and the proxy evicts from the front. + cache.delete(missing[i].key); cache.set(missing[i].key, { at: fetchedAt, result }); cacheChanged = true; } else if (absentTerrainDatum(result)) { diff --git a/src/terrainHeightsBounds.test.mjs b/src/terrainHeightsBounds.test.mjs index 1676c80ba..7556e9aea 100644 --- a/src/terrainHeightsBounds.test.mjs +++ b/src/terrainHeightsBounds.test.mjs @@ -184,6 +184,44 @@ test('the point cache stops growing, and an evicted point is simply refetched', assert.equal(asked[4], point(1)); }); +test('a refetched point counts as new when the cache evicts', async (t) => { + const root = await mkdtemp(path.join(tmpdir(), 'terrain-evict-order-')); + t.after(() => rm(root, { recursive: true, force: true })); + const cachePath = path.join(root, 'cache.json'); + const day = 24 * 3600_000; + const now = Date.now(); + const point = (i) => `-97.7${i}000,30.26000`; + const entry = (at) => ({ at, result: { ellipsoid: 150 } }); + // Written newest first: the file's order is not the age order. Point 1 is + // past the 30-day TTL, so the next request for it refetches. + await writeFile( + cachePath, + JSON.stringify({ + version: 2, + points: { + [point(3)]: entry(now - 3600_000), + [point(2)]: entry(now - day), + [point(1)]: entry(now - 31 * day), + }, + }), + ); + const asked = stubTerrainUpstream(t); + const route = install( + terrainHeightsProxy({ cacheMaxEntries: 3, cachePath }), + ).get('/api/terrain/heights'); + + await request(route, { url: `/?points=${point(1)}` }); + assert.deepEqual(asked, [point(1)], 'the stale point is refetched'); + await request(route, { url: `/?points=${point(4)}` }); + assert.equal(asked.length, 2); + + // Point 2 is now the oldest fetch; point 1 was just refreshed. + await request(route, { url: `/?points=${point(1)};${point(3)}` }); + assert.equal(asked.length, 2, 'the refreshed and the recent point stay'); + await request(route, { url: `/?points=${point(2)}` }); + assert.deepEqual(asked.slice(2), [point(2)], 'the oldest fetch was evicted'); +}); + test('an over-large cache file is not read back into memory', async (t) => { const asked = stubTerrainUpstream(t); const point = '-97.74000,30.26000';