diff --git a/src/main/worktree-retirement-backfill-scan.test.ts b/src/main/worktree-retirement-backfill-scan.test.ts index 3bad50c923e..f5957b46e88 100644 --- a/src/main/worktree-retirement-backfill-scan.test.ts +++ b/src/main/worktree-retirement-backfill-scan.test.ts @@ -88,6 +88,24 @@ describe('runRetirementBackfillScan', () => { ) }) + it('never defers a healthy machine, however many namespaces seed at once', async () => { + // Nested workspaces give every repo its own scan key, so several first-time backfills are + // routinely in flight together. Counting merely-in-flight listings would reject the third and + // let its create pick a name against an unseeded registry. + const scans = Array.from({ length: RETIREMENT_BACKFILL_MAX_OUTSTANDING_SCANS + 2 }, () => + stallingScan() + ) + const pending = scans.map((scan, index) => + runRetirementBackfillScan({}, `ns-${index}`, scan.run) + ) + expect(getOutstandingRetirementBackfillScanCount()).toBe(0) + + scans.forEach((scan, index) => scan.finish([`name-${index}`])) + await expect(Promise.all(pending)).resolves.toEqual( + scans.map((_, index) => new Set([`name-${index}`])) + ) + }) + it('keeps a listing that lands after the deadline instead of discarding the answer', async () => { // The WSL gate admits one scan at a time and allows it 60s — four times this deadline — so a // late-but-correct listing is the normal case there, not an edge case. diff --git a/src/main/worktree-retirement-backfill-scan.ts b/src/main/worktree-retirement-backfill-scan.ts index 1d74cc4450c..40d9f249d9b 100644 --- a/src/main/worktree-retirement-backfill-scan.ts +++ b/src/main/worktree-retirement-backfill-scan.ts @@ -5,19 +5,20 @@ export const RETIREMENT_BACKFILL_SCAN_TIMEOUT_MS = 15_000 /** How long a failed scan stays failed, so a stalled mount is not re-probed on every create. */ export const RETIREMENT_BACKFILL_RETRY_AFTER_FAILURE_MS = 60_000 -/** Listings this module may leave outstanding at once, process-wide. +/** Listings that have outlived their deadline and are still holding a thread, process-wide. * - * The deadline above abandons a scan, it cannot cancel one: an unabortable `readdir` keeps its - * libuv threadpool thread until the OS releases it, and that pool defaults to four. So the - * backoff alone only paces the leak — each lapse still stacks another stuck thread. This bounds - * what retirement discovery can hold; it is not a guarantee the pool stays healthy, since the WSL - * gate and its close lane hold threads of their own. Two leaves headroom for both. */ + * The deadline abandons a scan, it cannot cancel one: an unabortable `readdir` keeps its libuv + * threadpool thread until the OS releases it, and that pool defaults to four. So the backoff alone + * only paces the leak — each lapse still stacks another stuck thread. Only listings past their + * deadline count, so healthy scans (which finish in milliseconds) never deny each other a slot. + * This bounds what retirement discovery can hold; it is not a guarantee the pool stays healthy, + * since the WSL gate and its close lane hold threads of their own. Two leaves headroom for both. */ export const RETIREMENT_BACKFILL_MAX_OUTSTANDING_SCANS = 2 type BackfillScan = { /** Resolves with the discovered names, or rejects once the deadline lapses. */ names: Promise> - /** Epoch ms this scan may be retried after; 0 while it is in flight or has succeeded. */ + /** Monotonic ms this scan may be retried after; 0 while it is in flight or has succeeded. */ retryAfter: number } @@ -26,6 +27,12 @@ const scansByStore = new WeakMap>() /** Process-wide on purpose: the threadpool these listings occupy is process-wide too. */ let outstandingScans = 0 +/** Monotonic, like the WSL gate's own stuck timer: wall time misjudges a backoff across laptop + * sleep or an NTP step, either pinning a namespace in its failure memo or ending it early. */ +function monotonicNow(): number { + return performance.now() +} + export function getOutstandingRetirementBackfillScanCount(): number { return outstandingScans } @@ -54,8 +61,8 @@ function withScanDeadline(scan: Promise>): Promise> { * `readdir` had no deadline, so one stalled listing wedged this create and every later * generated-name create for the namespace until Orca restarted. * - * Retrying a failure is therefore allowed, but only up to - * `RETIREMENT_BACKFILL_MAX_OUTSTANDING_SCANS` listings still stuck in the kernel at once. */ + * Retrying a failure is therefore allowed, but only while fewer than + * `RETIREMENT_BACKFILL_MAX_OUTSTANDING_SCANS` listings are still stuck past their deadline. */ export function runRetirementBackfillScan( store: object, scanKey: string, @@ -67,7 +74,7 @@ export function runRetirementBackfillScan( scansByStore.set(store, storeScans) } const cached = storeScans.get(scanKey) - if (cached && (cached.retryAfter === 0 || Date.now() < cached.retryAfter)) { + if (cached && (cached.retryAfter === 0 || monotonicNow() < cached.retryAfter)) { return cached.names } if (outstandingScans >= RETIREMENT_BACKFILL_MAX_OUTSTANDING_SCANS) { @@ -82,25 +89,35 @@ export function runRetirementBackfillScan( } const entry: BackfillScan = { names: Promise.resolve(new Set()), retryAfter: 0 } const started = scan() - outstandingScans += 1 - void started.then( - (names) => { - outstandingScans -= 1 - // A listing that lands after the deadline still holds the right answer, and under WSL gate - // contention that is the common case rather than the edge one — the gate admits a single - // scan at a time and gives it 60s, four times this deadline. Dropping it would leave the - // namespace unseeded on exactly the mounts this feature exists to cover. - if (entry.retryAfter !== 0) { - entry.names = Promise.resolve(names) - entry.retryAfter = 0 - } - }, - () => { + let settled = false + let holdsSlot = false + const releaseSlot = (): void => { + settled = true + if (holdsSlot) { outstandingScans -= 1 + holdsSlot = false } - ) + } + // Registered before the deadline wrapper so `settled` is already true when its catch runs, which + // is how a scan that failed fast is told apart from one that outlived its deadline. + void started.then((names) => { + releaseSlot() + // A listing that lands after the deadline still holds the right answer, and under WSL gate + // contention that is the common case rather than the edge one — the gate admits a single scan + // at a time and gives it 60s, four times this deadline. Dropping it would leave the namespace + // unseeded on exactly the mounts this feature exists to cover. No liveness check is needed: + // once a retry replaces this entry in the map nothing can read it again, so the write is inert. + if (entry.retryAfter !== 0) { + entry.names = Promise.resolve(names) + entry.retryAfter = 0 + } + }, releaseSlot) entry.names = withScanDeadline(started).catch((error: unknown) => { - entry.retryAfter = Date.now() + RETIREMENT_BACKFILL_RETRY_AFTER_FAILURE_MS + if (!settled) { + outstandingScans += 1 + holdsSlot = true + } + entry.retryAfter = monotonicNow() + RETIREMENT_BACKFILL_RETRY_AFTER_FAILURE_MS throw error }) storeScans.set(scanKey, entry)