From 237a2e8894b88fd27c1be8aa13df301221743866 Mon Sep 17 00:00:00 2001 From: "Bill Gates (ops agent)" Date: Fri, 18 Sep 2026 17:53:22 +0000 Subject: [PATCH 1/2] feat(cache): make contiguous evictor unlink concurrency and sweep bound configurable The chunk evictor already derives its unlink fan-out from the thread pool (CHUNK_DATA_CACHE_INDEX_UNLINK_CONCURRENCY, max(1, UV_THREADPOOL_SIZE/8)), with a comment explaining that a hard-coded 50 "takes the entire pool on a stock node and queues every chunk read behind it -- on a device that is, by definition, already saturated when the evictor is running." The contiguous evictor still hard-codes both UNLINK_CONCURRENCY = 50 and MAX_BATCHES_PER_SWEEP = 50, so with the default batch size of 1000 a single 60s sweep can issue up to 50,000 unlinks at 50-way concurrency, and an operator has no way to pace it. Measured on a production gateway (20 TB btrfs cache on HDD, 88% full, UV_THREADPOOL_SIZE=64): during eviction bursts of 6,000-39,000 deletions per 10 minutes the disk sits at 100% utilisation and in-flight libuv requests reach a median of 3,376 (peak 24,483) against the 64-thread pool, versus 43 outside those windows. Everything file-backed queues behind it, including CDB64 root-tx index lookups on a separate NVMe device, which then hit their 60s circuit-breaker timeout. All 14 of the highest in-flight slots over 48h coincided with eviction bursts; public request volume, chunk ingest and the filesystem-walk cleanup worker showed no correlation. Adds, mirroring the chunk evictor: - CONTIGUOUS_DATA_CACHE_INDEX_UNLINK_CONCURRENCY, default max(1, UV_THREADPOOL_SIZE/8) - CONTIGUOUS_DATA_CACHE_INDEX_MAX_BATCHES_PER_SWEEP, default 50 (unchanged) The concurrency default drops from 50 to 8 on a 64-thread pool (1 on a stock 4-thread node), which is the behaviour change here: the same work is done per sweep, just without occupying the whole pool at once. Tests: unlink fan-out never exceeds the configured limit; a sweep stops at batchSize * maxBatchesPerSweep instead of draining the index. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012NWDKc9pST69qTEha4AGaB --- docs/envs.md | 2 + src/config.ts | 20 +++++++++ .../contiguous-data-cache-evictor.test.ts | 44 +++++++++++++++++++ src/workers/contiguous-data-cache-evictor.ts | 21 +++++---- 4 files changed, 76 insertions(+), 11 deletions(-) diff --git a/docs/envs.md b/docs/envs.md index 21e657704..c68add44c 100644 --- a/docs/envs.md +++ b/docs/envs.md @@ -237,6 +237,8 @@ This document describes the environment variables that can be used to configure | ENABLE_CONTIGUOUS_DATA_CACHE_INDEX | Boolean | false | Enables the SQLite cache cleanup index: each cache write records {hash, size, cached_at, tier} and a disk-pressure evictor reclaims by querying the (SSD-backed) index instead of walking the HDD-backed cache tree. Uses the LOW/HIGH_WATERMARK_PERCENT + MIN_FREE_BYTES thresholds above | | CONTIGUOUS_DATA_CACHE_INDEX_EVICTION_INTERVAL_MS | Number | 60000 | How often (ms) the index-driven evictor checks disk pressure | | CONTIGUOUS_DATA_CACHE_INDEX_EVICTION_BATCH_SIZE | Number | 1000 | Max index rows evicted (and blobs unlinked) per batch within an eviction sweep | +| CONTIGUOUS_DATA_CACHE_INDEX_UNLINK_CONCURRENCY | Number | max(1, UV_THREADPOOL_SIZE/8) | Concurrent blob unlinks per eviction batch. Each unlink holds a libuv thread, so this is derived from the pool rather than fixed: a large value takes the whole pool and queues every other file operation behind it, on a disk that is already saturated whenever the evictor runs | +| CONTIGUOUS_DATA_CACHE_INDEX_MAX_BATCHES_PER_SWEEP | Number | 50 | Batches per sweep. `EVICTION_BATCH_SIZE * this` bounds the unlinks a single sweep can issue, so a large backlog is reclaimed over several sweeps instead of one pass holding the disk | | CONTIGUOUS_DATA_CACHE_INDEX_UPDATE_ON_READ | Boolean | true | Refresh a cache entry's last_access (and promote its tier on a preferred-ArNS read) on cache hits => LRU eviction. Set false for FIFO by cache-write time (e.g. operators without an edge cache who want to avoid the per-hit index writes) | | ENABLE_CONTIGUOUS_DATA_CACHE_INDEX_BACKFILL | Boolean | false | One-time backfill: on startup, walk the existing on-disk cache once and seed index rows for untracked blobs (insert-if-absent, so live entries are untouched). Needed to adopt a pre-existing cache that predates the index; enable once, then disable. Runs in the background without blocking startup. Resumable across restarts (checkpoints per top-level shard); delete data/contiguous/.cache-index-backfill-checkpoint to force a fresh full pass | | CONTIGUOUS_DATA_CACHE_INDEX_BACKFILL_BATCH_SIZE | Number | 2000 | Rows buffered per backfill insert transaction | diff --git a/src/config.ts b/src/config.ts index 5c6f28201..cccc163f8 100644 --- a/src/config.ts +++ b/src/config.ts @@ -2877,6 +2877,26 @@ export const CONTIGUOUS_DATA_CACHE_INDEX_EVICTION_INTERVAL_MS = export const CONTIGUOUS_DATA_CACHE_INDEX_EVICTION_BATCH_SIZE = +env.varOrDefault('CONTIGUOUS_DATA_CACHE_INDEX_EVICTION_BATCH_SIZE', '1000'); +// Concurrent blob unlinks per eviction batch. DERIVED from UV_THREADPOOL_SIZE +// for the same reason as CHUNK_DATA_CACHE_INDEX_UNLINK_CONCURRENCY: every +// unlink occupies a libuv thread, so a hard-coded 50 takes the entire pool on a +// stock node (UV_THREADPOOL_SIZE defaults to 4) and queues every other file +// operation behind it -- on a device that is, by definition, already saturated +// when the evictor is running. +export const CONTIGUOUS_DATA_CACHE_INDEX_UNLINK_CONCURRENCY = + env.positiveIntOrDefault( + 'CONTIGUOUS_DATA_CACHE_INDEX_UNLINK_CONCURRENCY', + Math.max(1, Math.floor(UV_THREADPOOL_SIZE / 8)), + ); + +// Batches per sweep. batchSize * this is the upper bound on unlinks issued by a +// single sweep, so it bounds how long one sweep can hold the disk and the pool. +export const CONTIGUOUS_DATA_CACHE_INDEX_MAX_BATCHES_PER_SWEEP = + env.positiveIntOrDefault( + 'CONTIGUOUS_DATA_CACHE_INDEX_MAX_BATCHES_PER_SWEEP', + 50, + ); + // Whether to refresh a cache entry's last_access (and promote its tier on a // preferred-ArNS read) on cache HITS. On => LRU eviction; off => FIFO by // cache-write time. Operators without an edge cache see every read at the core, diff --git a/src/workers/contiguous-data-cache-evictor.test.ts b/src/workers/contiguous-data-cache-evictor.test.ts index 068e13247..d8a3056de 100644 --- a/src/workers/contiguous-data-cache-evictor.test.ts +++ b/src/workers/contiguous-data-cache-evictor.test.ts @@ -100,6 +100,50 @@ describe('ContiguousDataCacheEvictor', () => { assert.equal(h.remaining.size, 20); }); + // Every unlink occupies a libuv thread for its duration, so an unbounded (or + // hard-coded 50) fan-out takes the whole pool on a stock node and queues every + // other file operation behind it — on a disk that is already saturated, which + // is why the evictor is running at all. + it('never exceeds the configured unlink concurrency', async () => { + const h = makeHarness({ + initialUsedPercent: 95, + entryCount: 200, + freePerEvict: 1, + }); + let inFlight = 0; + let peak = 0; + (h.dataStore as any).delete = async (hash: string) => { + inFlight++; + peak = Math.max(peak, inFlight); + await new Promise((r) => setImmediate(r)); + inFlight--; + h.unlinked.push(hash); + }; + + await makeEvictor(h, { batchSize: 50, unlinkConcurrency: 4 }).sweep(); + + assert.ok( + h.unlinked.length > 4, + `expected real work, got ${h.unlinked.length}`, + ); + assert.ok(peak <= 4, `peak concurrent unlinks was ${peak}, limit was 4`); + }); + + it('bounds unlinks per sweep by batchSize * maxBatchesPerSweep', async () => { + const h = makeHarness({ + initialUsedPercent: 99, + entryCount: 500, + freePerEvict: 0, // never recovers: only the sweep bound can stop it + }); + + await makeEvictor(h, { batchSize: 5, maxBatchesPerSweep: 3 }).sweep(); + + // The sweep must stop at the bound rather than draining the index, so the + // next sweep resumes instead of one pass holding the disk indefinitely. + assert.equal(h.unlinked.length, 15); + assert.equal(h.remaining.size, 485); + }); + it('evicts oldest-first until usage recovers below the low watermark', async () => { const h = makeHarness({ initialUsedPercent: 90, // over high(80) diff --git a/src/workers/contiguous-data-cache-evictor.ts b/src/workers/contiguous-data-cache-evictor.ts index 10a9d6967..e959ae6a2 100644 --- a/src/workers/contiguous-data-cache-evictor.ts +++ b/src/workers/contiguous-data-cache-evictor.ts @@ -12,15 +12,6 @@ import * as config from '../config.js'; import * as metrics from '../metrics.js'; import { ContiguousDataCacheIndex, ContiguousDataStore } from '../types.js'; -// Bound work per sweep so a large backlog is reclaimed over several sweeps -// rather than one unbounded pass; the next sweep resumes. -const MAX_BATCHES_PER_SWEEP = 50; - -// Concurrent blob unlinks per batch. The index row deletes are already batched -// into one transaction; the unlinks are the remaining (HDD-bound) cost, so run -// them in parallel to saturate the disk instead of one seek at a time. -const UNLINK_CONCURRENCY = 50; - /** * Disk-pressure evictor for the contiguous data cache, driven by the SQLite * cleanup index instead of a filesystem walk. When usage on the cache @@ -44,6 +35,8 @@ export class ContiguousDataCacheEvictor { private minFreeBytes: number; private intervalMs: number; private batchSize: number; + private unlinkConcurrency: number; + private maxBatchesPerSweep: number; private timer: NodeJS.Timeout | undefined; private sweeping = false; @@ -58,6 +51,8 @@ export class ContiguousDataCacheEvictor { minFreeBytes = config.CONTIGUOUS_DATA_CACHE_MIN_FREE_BYTES, intervalMs = config.CONTIGUOUS_DATA_CACHE_INDEX_EVICTION_INTERVAL_MS, batchSize = config.CONTIGUOUS_DATA_CACHE_INDEX_EVICTION_BATCH_SIZE, + unlinkConcurrency = config.CONTIGUOUS_DATA_CACHE_INDEX_UNLINK_CONCURRENCY, + maxBatchesPerSweep = config.CONTIGUOUS_DATA_CACHE_INDEX_MAX_BATCHES_PER_SWEEP, }: { log: Logger; dataStore: ContiguousDataStore; @@ -68,6 +63,8 @@ export class ContiguousDataCacheEvictor { minFreeBytes?: number; intervalMs?: number; batchSize?: number; + unlinkConcurrency?: number; + maxBatchesPerSweep?: number; }) { this.log = log.child({ class: this.constructor.name }); this.dataStore = dataStore; @@ -78,6 +75,8 @@ export class ContiguousDataCacheEvictor { this.minFreeBytes = minFreeBytes; this.intervalMs = intervalMs; this.batchSize = Math.max(1, batchSize); + this.unlinkConcurrency = Math.max(1, unlinkConcurrency); + this.maxBatchesPerSweep = Math.max(1, maxBatchesPerSweep); } start(): void { @@ -169,7 +168,7 @@ export class ContiguousDataCacheEvictor { let evicted = 0; let bytesFreed = 0; - for (let batch = 0; batch < MAX_BATCHES_PER_SWEEP; batch++) { + for (let batch = 0; batch < this.maxBatchesPerSweep; batch++) { const candidates = await this.cacheIndex.selectContiguousDataCacheEvictionCandidates( this.batchSize, @@ -201,7 +200,7 @@ export class ContiguousDataCacheEvictor { metrics.cacheIndexEvictedTotal.inc({ reason: 'disk_pressure' }); metrics.cacheIndexEvictedBytesTotal.inc(size); } - const unlinkLimit = pLimit(UNLINK_CONCURRENCY); + const unlinkLimit = pLimit(this.unlinkConcurrency); await Promise.all( deletedHashes.map((hash) => unlinkLimit(() => From 2bc6bda689114dc0ac2e801c9b812df96810c93e Mon Sep 17 00:00:00 2001 From: "Bill Gates (ops agent)" Date: Fri, 18 Sep 2026 18:08:22 +0000 Subject: [PATCH 2/2] fix(cache): validate explicit evictor limits instead of clamping them Addresses CodeRabbit's review of the constructor options. Math.max(1, x) does not guard the values that matter. NaN clamps to NaN, so `batch < this.maxBatchesPerSweep` is false and a sweep evicts nothing while the cache keeps filling. Infinity removes the sweep bound entirely. A fractional unlinkConcurrency is rejected by p-limit mid-sweep -- after the index rows are deleted but before the blobs are unlinked, leaving orphans for the reconciler to find. All three fail silently or half-way through, which is the worst place for a configuration error to surface. Explicit limits are now validated as positive integers and throw at construction. Env-supplied values already go through positiveIntOrDefault, so this only affects direct callers passing bad values. Also documents that the derived default floors the division, per the same review: UV_THREADPOOL_SIZE=15 yields 1, not 1.875. Tests: NaN, Infinity, 0, -1 and 2.5 are each rejected for both options. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012NWDKc9pST69qTEha4AGaB --- docs/envs.md | 2 +- .../contiguous-data-cache-evictor.test.ts | 24 ++++++++++++++++++ src/workers/contiguous-data-cache-evictor.ts | 25 +++++++++++++++++-- 3 files changed, 48 insertions(+), 3 deletions(-) diff --git a/docs/envs.md b/docs/envs.md index c68add44c..9c4116510 100644 --- a/docs/envs.md +++ b/docs/envs.md @@ -237,7 +237,7 @@ This document describes the environment variables that can be used to configure | ENABLE_CONTIGUOUS_DATA_CACHE_INDEX | Boolean | false | Enables the SQLite cache cleanup index: each cache write records {hash, size, cached_at, tier} and a disk-pressure evictor reclaims by querying the (SSD-backed) index instead of walking the HDD-backed cache tree. Uses the LOW/HIGH_WATERMARK_PERCENT + MIN_FREE_BYTES thresholds above | | CONTIGUOUS_DATA_CACHE_INDEX_EVICTION_INTERVAL_MS | Number | 60000 | How often (ms) the index-driven evictor checks disk pressure | | CONTIGUOUS_DATA_CACHE_INDEX_EVICTION_BATCH_SIZE | Number | 1000 | Max index rows evicted (and blobs unlinked) per batch within an eviction sweep | -| CONTIGUOUS_DATA_CACHE_INDEX_UNLINK_CONCURRENCY | Number | max(1, UV_THREADPOOL_SIZE/8) | Concurrent blob unlinks per eviction batch. Each unlink holds a libuv thread, so this is derived from the pool rather than fixed: a large value takes the whole pool and queues every other file operation behind it, on a disk that is already saturated whenever the evictor runs | +| CONTIGUOUS_DATA_CACHE_INDEX_UNLINK_CONCURRENCY | Number | max(1, floor(UV_THREADPOOL_SIZE/8)) | Concurrent blob unlinks per eviction batch. Each unlink holds a libuv thread, so this is derived from the pool rather than fixed: a large value takes the whole pool and queues every other file operation behind it, on a disk that is already saturated whenever the evictor runs | | CONTIGUOUS_DATA_CACHE_INDEX_MAX_BATCHES_PER_SWEEP | Number | 50 | Batches per sweep. `EVICTION_BATCH_SIZE * this` bounds the unlinks a single sweep can issue, so a large backlog is reclaimed over several sweeps instead of one pass holding the disk | | CONTIGUOUS_DATA_CACHE_INDEX_UPDATE_ON_READ | Boolean | true | Refresh a cache entry's last_access (and promote its tier on a preferred-ArNS read) on cache hits => LRU eviction. Set false for FIFO by cache-write time (e.g. operators without an edge cache who want to avoid the per-hit index writes) | | ENABLE_CONTIGUOUS_DATA_CACHE_INDEX_BACKFILL | Boolean | false | One-time backfill: on startup, walk the existing on-disk cache once and seed index rows for untracked blobs (insert-if-absent, so live entries are untouched). Needed to adopt a pre-existing cache that predates the index; enable once, then disable. Runs in the background without blocking startup. Resumable across restarts (checkpoints per top-level shard); delete data/contiguous/.cache-index-backfill-checkpoint to force a fresh full pass | diff --git a/src/workers/contiguous-data-cache-evictor.test.ts b/src/workers/contiguous-data-cache-evictor.test.ts index d8a3056de..89e2fdafa 100644 --- a/src/workers/contiguous-data-cache-evictor.test.ts +++ b/src/workers/contiguous-data-cache-evictor.test.ts @@ -129,6 +129,30 @@ describe('ContiguousDataCacheEvictor', () => { assert.ok(peak <= 4, `peak concurrent unlinks was ${peak}, limit was 4`); }); + // Clamping these would fail silently in the worst way: Math.max(1, NaN) is + // NaN, so the sweep loop runs zero batches and the cache never drains, while + // a fractional unlinkConcurrency makes p-limit throw mid-sweep — after the + // index rows are deleted but before the blobs are unlinked. + it('rejects invalid explicit limits at construction', async () => { + const h = makeHarness({ + initialUsedPercent: 90, + entryCount: 10, + freePerEvict: 5, + }); + for (const bad of [Number.NaN, Infinity, 0, -1, 2.5]) { + assert.throws( + () => makeEvictor(h, { unlinkConcurrency: bad }), + /unlinkConcurrency must be a positive integer/, + `unlinkConcurrency=${bad} should be rejected`, + ); + assert.throws( + () => makeEvictor(h, { maxBatchesPerSweep: bad }), + /maxBatchesPerSweep must be a positive integer/, + `maxBatchesPerSweep=${bad} should be rejected`, + ); + } + }); + it('bounds unlinks per sweep by batchSize * maxBatchesPerSweep', async () => { const h = makeHarness({ initialUsedPercent: 99, diff --git a/src/workers/contiguous-data-cache-evictor.ts b/src/workers/contiguous-data-cache-evictor.ts index e959ae6a2..00415ebab 100644 --- a/src/workers/contiguous-data-cache-evictor.ts +++ b/src/workers/contiguous-data-cache-evictor.ts @@ -12,6 +12,21 @@ import * as config from '../config.js'; import * as metrics from '../metrics.js'; import { ContiguousDataCacheIndex, ContiguousDataStore } from '../types.js'; +// Explicit limits are validated rather than clamped: Math.max(1, NaN) is NaN, +// which makes the sweep loop run zero batches and evict nothing; Infinity +// removes the bound entirely; and a fractional unlinkConcurrency makes p-limit +// throw mid-sweep, after the index rows are deleted but before the blobs are +// unlinked, leaving orphans for the reconciler. Failing at construction is the +// only outcome that cannot silently misbehave in production. +function positiveIntLimit(name: string, value: number): number { + if (!Number.isInteger(value) || value < 1) { + throw new Error( + `${name} must be a positive integer, received ${String(value)}`, + ); + } + return value; +} + /** * Disk-pressure evictor for the contiguous data cache, driven by the SQLite * cleanup index instead of a filesystem walk. When usage on the cache @@ -75,8 +90,14 @@ export class ContiguousDataCacheEvictor { this.minFreeBytes = minFreeBytes; this.intervalMs = intervalMs; this.batchSize = Math.max(1, batchSize); - this.unlinkConcurrency = Math.max(1, unlinkConcurrency); - this.maxBatchesPerSweep = Math.max(1, maxBatchesPerSweep); + this.unlinkConcurrency = positiveIntLimit( + 'unlinkConcurrency', + unlinkConcurrency, + ); + this.maxBatchesPerSweep = positiveIntLimit( + 'maxBatchesPerSweep', + maxBatchesPerSweep, + ); } start(): void {