mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-09-13 23:14:20 +00:00
fix(parse-cache): retire a chunk whose durable generation could not be reset (#3271)
* fix(parse-cache): retire a chunk whose durable generation could not be reset #3200 skipped the parse-cache write when `prepareDurableParsedFileChunk` failed, but the chunk hash was already in `usedKeys` from the lookup. When a previous generation existed on disk — reachable because the coherence gate re-dispatches a chunk whose `.v8` shard is live but whose durable shards are unreadable — `saveParseCache` copied that old shard forward, and the durable prune, which keeps exactly the saved keys, retained the mixed directory. The next run then served a warm hit out of a directory the previous run had already decided it could not account for. Retire the hash instead of only skipping the write: - `ParseCache.staleKeys` is a transient set that `saveParseCache` filters out of its key list. Filtering at save is what makes it survive the post-parse key merges in run-analyze (#2106 sibling fold, unreadable-meta retention), and it reaches both stores at once because the durable prune keeps exactly the keys `saveParseCache` returns. - The hash is retired at the reset-failure site, which runs unconditionally. The parse-cache write branch sits behind `rawResults.length > 0`, so a chunk whose worker round returns nothing would never have been retired there. - Worker-quarantined chunks get the same treatment for the same reason: they also reach the save with no in-memory entry, which is what triggers the copy-forward. That branch was previously unreachable when the worker died on the chunk and returned no results. - Guard the durable prune's non-survivor `fs.rm`. The causes that break the reset break that delete too, and it sat outside the validation try — one undeletable directory aborted the loop and cost every remaining chunk its index entry. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(review): apply review findings Retire a chunk only when a generation nobody cleared is still on disk. `prepareDurableParsedFileChunk` is rm-then-mkdir, and the catch could not tell the two apart: an rm that succeeded before a failing mkdir leaves NO directory, so the workers recreate it and write a clean generation. Retiring there discarded a good `.v8` for no safety gain — and under a correlated failure (an empty durable index turns every chunk into a re-dispatched miss, then a descriptor burst rejects the resets en masse) it would have wiped both shared stores for every branch, where the pre-#3204 posture cost only the writes. `durableChunkHasStaleShards` is the discriminator. Finish the delete guard on the path that runs before it. The staged→live overlay in `mergeStagedDurableParsedFileStore` awaited `replaceDurableChunkDir` unguarded, so on the cold-rebuild path one undeletable directory threw out of the merge before the prune ever ran — the durable index was never rewritten and a retired chunk kept its directory. Same log-and-continue treatment, plus best-effort handling of the two `.replacing` backup removals. Aggregate the prune's delete-failure warning: a store-wide cause hits every non-survivor, and one line per directory buries the message that matters. Tests: - Guard the chmod-based prune test with the repo's `skipIf` for root/Windows and assert the directory survived, so it cannot pass vacuously where the delete succeeds. - Add a two-chunk control: one chunk's reset fails, and the sibling must stay warm through the next run. One chunk plus a global spawn marker could not tell "retires the failing chunk" from "retires everything". - Add the rm-succeeded/mkdir-failed case, which must NOT retire. - Model both post-parse merges in the R4 test (the sibling fold re-adds the key, the unreadable-meta fallback unions `entries`), and move the in-memory `entries` assertion to a direct helper test — the sharded path never populates `entries`, so the old assertion proved nothing. - Type the cache factory as `ParseCache`; the `staleKeys` assertions were TS2339 and `?? false` read as a pass regardless. - Register the store test in the cross-platform filesystem list. Correct two comments that still described a quarantined chunk by the premise this fix disproves. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(review): stop swallowing the backup removal in replaceDurableChunkDir Swallowing that `fs.rm` manufactured the very hazard this PR removes. When a non-empty `${to}.replacing` survives, the following `fs.rename(to, backup)` cannot overwrite it and is suppressed as "dest was missing", so `backedUp` stays false and the `fs.cp(from, to)` fallback merges the staged generation INTO the live directory — old shards alongside new, which the prune then indexes as one valid survivor. Let it throw; the per-entry guard added to `mergeStagedDurableParsedFileStore` already stops one such chunk from costing the others their prune. The post-publish backup cleanup stays best-effort, where an undeletable leftover really is litter. Also: - Make the sibling-isolation test perform the run its title claims. It asserted index membership and stopped; an index entry does not exercise the warm-hit path, so it would have passed even if the sibling re-dispatched. Each chunk now runs alone so the single spawn marker names which one re-parsed. - Correct two comments that outran the implementation: retirement is gated on shards actually surviving, and an undeletable directory is dropped from the index rather than removed from disk. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Gergő Magyar
GitHub
parent
3f5ca8cdb7
commit
ceaff27c1e
8 changed files with 480 additions and 19 deletions
|
|
@ -290,6 +290,11 @@ const NATIVE_ADDON_SMOKE = [
|
|||
// Filesystem behavior tests — exercise operations that vary across
|
||||
// platforms (CRLF, symlinks, permissions, temp dirs)
|
||||
const FILESYSTEM = [
|
||||
// The durable ParsedFile store's prune tolerates a chunk directory it cannot
|
||||
// delete (#3204). The failures that motivate it — held handles, read-only
|
||||
// mounts — are Windows- and macOS-flavored, and the permission-based case
|
||||
// skips itself where chmod cannot block a delete, so run it everywhere.
|
||||
'test/unit/parsedfile-store.test.ts',
|
||||
'test/integration/filesystem-walker.test.ts',
|
||||
'test/integration/watch-filesystem.test.ts',
|
||||
'test/integration/markdown-processor-crlf.test.ts',
|
||||
|
|
|
|||
|
|
@ -26,6 +26,7 @@ import {
|
|||
computeChunkHash,
|
||||
loadParseCacheChunk,
|
||||
persistParseCacheChunk,
|
||||
markParseCacheChunkStale,
|
||||
PARSE_CACHE_VERSION,
|
||||
packParseCacheChunks,
|
||||
} from '../../../storage/parse-cache.js';
|
||||
|
|
@ -37,6 +38,7 @@ import {
|
|||
loadDurableParsedFileIndex,
|
||||
prepareDurableParsedFileChunk,
|
||||
durableChunkHasShards,
|
||||
durableChunkHasStaleShards,
|
||||
} from '../../../storage/parsedfile-store.js';
|
||||
import type { ParseWorkerResult } from '../workers/parse-worker.js';
|
||||
import { DEFAULT_PDG_MAX_FUNCTION_LINES } from '../cfg/collect.js';
|
||||
|
|
@ -932,8 +934,12 @@ export async function runChunkedParseAndResolve(
|
|||
* Chunk hashes whose durable ParsedFile directory could not be reset. The
|
||||
* old generation's shards are still on disk, so a warm hit would union
|
||||
* stale shards with the new ones. Treated exactly like a quarantined chunk:
|
||||
* skip the parse-cache write so the next run re-dispatches into a clean
|
||||
* directory rather than trusting a generation we could not clear.
|
||||
* skip the parse-cache write AND, when shards from that generation are
|
||||
* still on disk, retire the hash (#3204): the old `.v8` is not carried
|
||||
* forward and the directory is dropped from the durable index, so the next
|
||||
* run re-dispatches. Kept as its own set rather than
|
||||
* read back off `staleKeys`, which is a superset — it is what selects the
|
||||
* warn below over the quarantine branch's dev-only log.
|
||||
*/
|
||||
const durablePrepareFailures = new Set<string>();
|
||||
|
||||
|
|
@ -1073,7 +1079,11 @@ export async function runChunkedParseAndResolve(
|
|||
// Persist raw results for this chunk hash (skipping when any chunk file
|
||||
// was worker-quarantined, so the narrower rawResults isn't cached under
|
||||
// the full-chunk key — see the original inline note / U20.U2).
|
||||
if (parseCache && p.chunkHash && rawResults.length > 0) {
|
||||
// `rawResults.length > 0` guards the WRITE only. A quarantined chunk
|
||||
// often returns nothing at all (the worker died on it), and that chunk
|
||||
// still has to be retired — otherwise its pre-existing `.v8` is copied
|
||||
// forward at save time (#3204).
|
||||
if (parseCache && p.chunkHash) {
|
||||
const quarantineSet = new Set(workerPool?.getQuarantinedPaths?.() ?? []);
|
||||
const chunkHadQuarantine = p.chunkFiles.some((f) => quarantineSet.has(f.path));
|
||||
const durableGenerationStale = durablePrepareFailures.has(p.chunkHash);
|
||||
|
|
@ -1084,6 +1094,10 @@ export async function runChunkedParseAndResolve(
|
|||
'so its shards may be stale; next run will re-dispatch it',
|
||||
);
|
||||
} else if (chunkHadQuarantine) {
|
||||
// This chunk's durable directory now holds only this run's NARROWER
|
||||
// shards, so a warm hit would replay the full-coverage `.v8` over
|
||||
// partial ParsedFiles (#3204).
|
||||
markParseCacheChunkStale(parseCache, p.chunkHash);
|
||||
if (isDev) {
|
||||
const quarantinedInChunk = p.chunkFiles.filter((f) => quarantineSet.has(f.path)).length;
|
||||
logger.info(
|
||||
|
|
@ -1092,7 +1106,7 @@ export async function runChunkedParseAndResolve(
|
|||
`next run will rediscover (${p.chunkHash.slice(0, 8)})`,
|
||||
);
|
||||
}
|
||||
} else {
|
||||
} else if (rawResults.length > 0) {
|
||||
await persistParseCacheChunk(parseCache, p.chunkHash, rawResults);
|
||||
if (isDev) {
|
||||
logger.info(
|
||||
|
|
@ -1142,6 +1156,22 @@ export async function runChunkedParseAndResolve(
|
|||
// directory on write, so at worst the old generation lingers.
|
||||
// Caught per chunk so one failure cannot abort the others.
|
||||
durablePrepareFailures.add(miss.chunkHash);
|
||||
// Retire ONLY when a generation nobody cleared is still on disk.
|
||||
// `prepareDurableParsedFileChunk` is rm-then-mkdir: an rm failure
|
||||
// leaves the old shards to be unioned into a warm hit, but an rm
|
||||
// that succeeded before a failing mkdir leaves nothing — the
|
||||
// workers recreate the directory and write a clean generation, so
|
||||
// retiring there would discard a good `.v8` for no safety gain
|
||||
// (and a correlated burst would discard the whole shared cache).
|
||||
// Retire here rather than at finalize: that branch sits behind
|
||||
// `rawResults.length > 0`, so a chunk whose worker round returns
|
||||
// nothing would keep its stale entry.
|
||||
if (
|
||||
parseCache &&
|
||||
(await durableChunkHasStaleShards(durableParsedFileDir, miss.chunkHash))
|
||||
) {
|
||||
markParseCacheChunkStale(parseCache, miss.chunkHash);
|
||||
}
|
||||
logger.warn(
|
||||
{ err, chunkHash: miss.chunkHash.slice(0, 8) },
|
||||
'parsedfile-cache: could not reset durable chunk generation; ' +
|
||||
|
|
|
|||
|
|
@ -4312,9 +4312,11 @@ async function runFullAnalysisInner(
|
|||
// Prune the durable ParsedFile store to EXACTLY the parse cache's
|
||||
// surviving keys (#2038 warm-cache coverage), so the two content-addressed
|
||||
// stores stay coherent: a chunk is "cached" iff both its parse-cache shard
|
||||
// and its durable shards exist. A quarantined chunk (in usedKeys but with
|
||||
// no parse-cache shard) drops its durable subdir here and re-dispatches
|
||||
// next run. Same try/catch — a durable-store write failure must never
|
||||
// and its durable shards exist. A retired chunk — worker-quarantined, or
|
||||
// one whose failed durable reset left an uncleared generation behind
|
||||
// (#3204) — is filtered out of `savedKeys`, so it drops out of the
|
||||
// durable index here and re-dispatches next run. Same try/catch — a
|
||||
// durable-store write must never
|
||||
// break an otherwise successful run (next run treats it as a miss).
|
||||
await mergeStagedDurableParsedFileStore(
|
||||
storagePath,
|
||||
|
|
|
|||
|
|
@ -896,6 +896,16 @@ export interface ParseCache {
|
|||
* Transient — never serialized to disk.
|
||||
*/
|
||||
usedKeys: Set<string>;
|
||||
/**
|
||||
* Hashes this run decided it cannot vouch for — its durable generation could
|
||||
* not be reset, or its chunk was worker-quarantined (#3204). `saveParseCache`
|
||||
* refuses them, so neither a pre-existing `.v8` nor the chunk's durable
|
||||
* directory survives into the next run. Kept separate from `usedKeys`
|
||||
* because the orchestrator re-adds keys to that set after the parse phase
|
||||
* (#2106 sibling fold), which would undo a deletion.
|
||||
* Transient — never serialized to disk.
|
||||
*/
|
||||
staleKeys?: Set<string>;
|
||||
/**
|
||||
* When set, chunk payloads are loaded from / flushed to sharded files on
|
||||
* demand instead of retaining every chunk in `entries` for the whole run
|
||||
|
|
@ -1125,6 +1135,24 @@ export const persistParseCacheChunk = async (
|
|||
cache.entries.set(chunkHash, slim);
|
||||
};
|
||||
|
||||
/**
|
||||
* Retire a chunk this run cannot vouch for — its durable ParsedFile generation
|
||||
* could not be reset, or its chunk was worker-quarantined (#3204).
|
||||
*
|
||||
* `saveParseCache` refuses a stale key, so no pre-existing `.v8` is copied
|
||||
* forward and the durable store — pruned to exactly the keys that save
|
||||
* returns — drops the chunk in the same step. The two deletes matter because
|
||||
* `loadParseCacheChunk` reads `entries` and `onDiskKeys` and does NOT consult
|
||||
* `staleKeys`: without them a second lookup of the same hash inside this run
|
||||
* would still serve the retired shard.
|
||||
*/
|
||||
export const markParseCacheChunkStale = (cache: ParseCache, chunkHash: string): void => {
|
||||
cache.staleKeys ??= new Set<string>();
|
||||
cache.staleKeys.add(chunkHash);
|
||||
cache.entries.delete(chunkHash);
|
||||
cache.onDiskKeys?.delete(chunkHash);
|
||||
};
|
||||
|
||||
const loadLegacyParseCache = async (storagePath: string): Promise<ParseCache> => {
|
||||
const cachePath = getLegacyCachePath(storagePath);
|
||||
try {
|
||||
|
|
@ -1210,7 +1238,14 @@ export const saveParseCache = async (storagePath: string, cache: ParseCache): Pr
|
|||
await fs.rm(tmpDir, { recursive: true, force: true });
|
||||
await fs.mkdir(tmpDir, { recursive: true });
|
||||
|
||||
const keys = [...cache.usedKeys].filter(isValidChunkCacheKey).sort();
|
||||
// A stale key is dropped here rather than at the failure site: the
|
||||
// orchestrator folds sibling-branch keys back into `usedKeys` after the parse
|
||||
// phase (#2106), so this is the last point that sees the final key set. The
|
||||
// exclusion also reaches the durable store, which prunes to the keys this
|
||||
// function returns — both stores drop the chunk together (#3204).
|
||||
const keys = [...cache.usedKeys]
|
||||
.filter((key) => isValidChunkCacheKey(key) && !cache.staleKeys?.has(key))
|
||||
.sort();
|
||||
// Track hashes whose shard was actually written/copied this save. A hash can
|
||||
// be in `usedKeys` without a backing shard — its in-memory serialize threw, or
|
||||
// its on-disk copy failed/was-absent (e.g. a worker-quarantined chunk added to
|
||||
|
|
|
|||
|
|
@ -619,6 +619,22 @@ export const prepareDurableParsedFileChunk = async (
|
|||
await fs.mkdir(dir, { recursive: true });
|
||||
};
|
||||
|
||||
/**
|
||||
* Does this chunk still hold shards from a generation nobody cleared?
|
||||
*
|
||||
* Asked only after {@link prepareDurableParsedFileChunk} rejected, to tell its
|
||||
* two failure modes apart (#3204). The `rm` failing leaves the previous
|
||||
* generation in place, and a warm hit would union it with whatever this run's
|
||||
* workers write — that chunk must be retired. The `rm` succeeding and the
|
||||
* `mkdir` then failing leaves NO directory: the workers recreate it and write
|
||||
* a clean generation, so retiring would throw away a good cache entry for
|
||||
* nothing. Absent or empty ⇒ nothing to distrust.
|
||||
*/
|
||||
export const durableChunkHasStaleShards = async (
|
||||
durableDir: string,
|
||||
chunkHash: string,
|
||||
): Promise<boolean> => (await listV8Shards(durableChunkDir(durableDir, chunkHash))).length > 0;
|
||||
|
||||
/**
|
||||
* Synchronous durable-shard writer for use INSIDE a parse worker, alongside
|
||||
* {@link persistParsedFileShardSync}. Writes the SAME bytes to a content-addressed
|
||||
|
|
@ -728,8 +744,9 @@ export const loadDurableParsedFileIndex = async (
|
|||
* Prune the durable store to `keepKeys` and rewrite its index. `keepKeys` must
|
||||
* be the parse cache's surviving on-disk keys (so the two stores stay coherent:
|
||||
* a chunk is "cached" iff BOTH its parse-cache shard and its durable shards
|
||||
* exist; a quarantined chunk — no parse-cache shard — drops its durable subdir
|
||||
* here and re-dispatches next run). Only chunks whose envelopes all validate
|
||||
* exist; a chunk retired by `markParseCacheChunkStale` — worker-quarantined, or
|
||||
* holding a durable generation that could not be reset — is absent from
|
||||
* `keepKeys`, so it drops its durable subdir here and re-dispatches next run). Only chunks whose envelopes all validate
|
||||
* are indexed, together with their exact persisted path coverage (never vouch
|
||||
* for a missing/corrupt shard). The index write is tmp+rename atomic.
|
||||
*/
|
||||
|
|
@ -745,6 +762,8 @@ export const pruneAndSaveDurableParsedFileStore = async (
|
|||
return; // nothing written this run
|
||||
}
|
||||
const survivors: Record<string, string[]> = {};
|
||||
const undeletable: string[] = [];
|
||||
let firstRemoveError: unknown;
|
||||
for (const name of entries) {
|
||||
if (name === DURABLE_INDEX_FILENAME) continue;
|
||||
const full = path.join(durableDir, name);
|
||||
|
|
@ -771,7 +790,29 @@ export const pruneAndSaveDurableParsedFileStore = async (
|
|||
/* not a readable dir → drop below */
|
||||
}
|
||||
}
|
||||
await fs.rm(full, { recursive: true, force: true });
|
||||
// The causes that break `prepareDurableParsedFileChunk` — permissions, a
|
||||
// locked file, a read-only mount — break this rm too (#3204). Dropping the
|
||||
// entry from the index is what makes the chunk unreachable; losing the
|
||||
// directory is a cleanup bonus. Never let one of them abort the loop and
|
||||
// cost every remaining chunk its index entry.
|
||||
try {
|
||||
await fs.rm(full, { recursive: true, force: true });
|
||||
} catch (err) {
|
||||
// `name` reached the drop branch precisely because it is not a live
|
||||
// chunk key, so it may be any stray directory — report it as an entry.
|
||||
undeletable.push(name);
|
||||
firstRemoveError ??= err;
|
||||
}
|
||||
}
|
||||
if (undeletable.length > 0) {
|
||||
// One line per RUN, not per directory: a store-wide cause (read-only mount,
|
||||
// wrong ownership) hits every non-survivor, and thousands of warns would
|
||||
// bury the message that matters.
|
||||
logger.warn(
|
||||
{ err: firstRemoveError, count: undeletable.length, firstEntry: undeletable[0] },
|
||||
'parsedfile-cache: could not remove pruned durable chunk directories; ' +
|
||||
'they are excluded from the index and will be re-attempted next run',
|
||||
);
|
||||
}
|
||||
const idx: DurableParsedFileIndex = { version, entries: survivors };
|
||||
const tmp = path.join(durableDir, `${DURABLE_INDEX_FILENAME}.tmp`);
|
||||
|
|
@ -810,7 +851,20 @@ export const mergeStagedDurableParsedFileStore = async (
|
|||
if (name === DURABLE_INDEX_FILENAME) continue;
|
||||
const from = path.join(stagedDir, name);
|
||||
const to = path.join(liveDir, name);
|
||||
await replaceDurableChunkDir(from, to);
|
||||
// Same reasoning as the prune's per-entry guard, on the loop that runs
|
||||
// BEFORE it (#3204): this overlay targets the same live chunk directories,
|
||||
// so the causes that break a reset break a replacement too. Letting one
|
||||
// throw here would skip the prune entirely — the durable index would not be
|
||||
// rewritten this run, and a retired chunk would keep its directory.
|
||||
try {
|
||||
await replaceDurableChunkDir(from, to);
|
||||
} catch (err) {
|
||||
logger.warn(
|
||||
{ err, entry: name },
|
||||
'parsedfile-cache: could not publish a staged durable chunk; ' +
|
||||
'it stays uncached and will re-dispatch next run',
|
||||
);
|
||||
}
|
||||
}
|
||||
await pruneAndSaveDurableParsedFileStore(liveDir, version, keepKeys);
|
||||
};
|
||||
|
|
@ -824,6 +878,12 @@ const replaceDurableChunkDir = async (from: string, to: string): Promise<void> =
|
|||
/* dest exists, or the rename is cross-device */
|
||||
}
|
||||
const backup = `${to}.replacing`;
|
||||
// Deliberately NOT best-effort. If a non-empty backup survives, the
|
||||
// `fs.rename(to, backup)` below cannot overwrite it and is swallowed as
|
||||
// "dest was missing", so the `fs.cp` fallback would merge the staged
|
||||
// generation INTO the live directory — manufacturing exactly the old+new
|
||||
// union this fix exists to prevent. Let it throw; the caller's per-entry
|
||||
// guard keeps one such chunk from costing the others their prune.
|
||||
await fs.rm(backup, { recursive: true, force: true });
|
||||
let backedUp = false;
|
||||
try {
|
||||
|
|
@ -847,6 +907,8 @@ const replaceDurableChunkDir = async (from: string, to: string): Promise<void> =
|
|||
throw err;
|
||||
}
|
||||
if (backedUp) {
|
||||
await fs.rm(backup, { recursive: true, force: true });
|
||||
// The new generation is already in place; an undeletable backup is litter,
|
||||
// not a failure. The next prune re-attempts it.
|
||||
await fs.rm(backup, { recursive: true, force: true }).catch(() => {});
|
||||
}
|
||||
};
|
||||
|
|
|
|||
|
|
@ -78,6 +78,7 @@ import {
|
|||
fileContentHash,
|
||||
packParseCacheChunks,
|
||||
} from '../../src/storage/parse-cache.js';
|
||||
import type { ParseCache } from '../../src/storage/parse-cache.js';
|
||||
import type { ParseWorkerResult } from '../../src/core/ingestion/workers/parse-worker.js';
|
||||
|
||||
/**
|
||||
|
|
@ -257,7 +258,7 @@ describe('U20: parse-impl quarantine + chunk-cache integration (PR #1693 Codex f
|
|||
|
||||
const expectedChunkHash = hashPacks(scanned).poison;
|
||||
|
||||
const parseCache = {
|
||||
const parseCache: ParseCache = {
|
||||
version: 'test',
|
||||
entries: new Map<string, ParseWorkerResult[]>(),
|
||||
usedKeys: new Set<string>(),
|
||||
|
|
@ -323,6 +324,9 @@ describe('U20: parse-impl quarantine + chunk-cache integration (PR #1693 Codex f
|
|||
// against a fresh-quarantine pool.
|
||||
expect(parseCache.entries.has(expectedChunkHash)).toBe(false);
|
||||
expect(parseCache.usedKeys.has(expectedChunkHash)).toBe(true);
|
||||
// #3204: `usedKeys` alone would let `saveParseCache` copy a pre-existing
|
||||
// shard forward, so the skipped chunk is also retired from the save.
|
||||
expect(parseCache.staleKeys?.has(expectedChunkHash)).toBe(true);
|
||||
for (const hash of hashPacks(scanned).others) {
|
||||
expect(parseCache.entries.has(hash)).toBe(true);
|
||||
}
|
||||
|
|
@ -338,7 +342,7 @@ describe('U20: parse-impl quarantine + chunk-cache integration (PR #1693 Codex f
|
|||
}));
|
||||
const expectedChunkHash = hashPacks(scanned).poison;
|
||||
|
||||
const parseCache = {
|
||||
const parseCache: ParseCache = {
|
||||
version: 'test',
|
||||
entries: new Map<string, ParseWorkerResult[]>(),
|
||||
usedKeys: new Set<string>(),
|
||||
|
|
|
|||
|
|
@ -34,8 +34,10 @@ import { pathToFileURL } from 'node:url';
|
|||
|
||||
// Partial mock: lets one test make prepareDurableParsedFileChunk fail without
|
||||
// touching the worker-side persist path (which shares the same directory).
|
||||
// Receives the chunk hash so a test can fail the reset for ONE chunk while its
|
||||
// siblings reset normally — the shape a correlated-vs-isolated failure needs.
|
||||
const prepareOverride = vi.hoisted(() => ({
|
||||
impl: undefined as undefined | (() => Promise<void>),
|
||||
impl: undefined as undefined | ((durableDir: string, chunkHash: string) => Promise<void>),
|
||||
}));
|
||||
const persistOverride = vi.hoisted(() => ({
|
||||
impl: undefined as undefined | (() => Promise<boolean>),
|
||||
|
|
@ -46,7 +48,7 @@ vi.mock('../../src/storage/parsedfile-store.js', async (importOriginal) => {
|
|||
...real,
|
||||
prepareDurableParsedFileChunk: (durableDir: string, chunkHash: string) =>
|
||||
prepareOverride.impl
|
||||
? prepareOverride.impl()
|
||||
? prepareOverride.impl(durableDir, chunkHash)
|
||||
: real.prepareDurableParsedFileChunk(durableDir, chunkHash),
|
||||
persistParsedFileChunk: (
|
||||
storagePath: string,
|
||||
|
|
@ -78,6 +80,7 @@ import {
|
|||
clearParsedFileStore,
|
||||
} from '../../src/storage/parsedfile-store.js';
|
||||
import type { ParseWorkerResult } from '../../src/core/ingestion/workers/parse-worker.js';
|
||||
import type { ParseCache } from '../../src/storage/parse-cache.js';
|
||||
import type { ParsedFile } from 'gitnexus-shared';
|
||||
|
||||
// A structurally-minimal ParsedFile. `loadParsedFilesForPaths` keys on
|
||||
|
|
@ -309,7 +312,7 @@ describe('parse-impl warm-cache ParsedFile coverage (#2038)', () => {
|
|||
return { path: rel, size: fs.statSync(full).size };
|
||||
};
|
||||
|
||||
const newCache = () => ({
|
||||
const newCache = (): ParseCache => ({
|
||||
version: PARSE_CACHE_VERSION,
|
||||
entries: new Map<string, ParseWorkerResult[]>(),
|
||||
usedKeys: new Set<string>(),
|
||||
|
|
@ -401,6 +404,279 @@ describe('parse-impl warm-cache ParsedFile coverage (#2038)', () => {
|
|||
expect(cache.onDiskKeys.size + cache.entries.size).toBe(0);
|
||||
});
|
||||
|
||||
// #3204: the chunk above had no prior generation. When one EXISTS, skipping
|
||||
// the write is not enough — `saveParseCache` copies the pre-existing `.v8`
|
||||
// forward from `usedKeys`, and the durable prune then keeps the mixed
|
||||
// directory because it prunes to exactly those saved keys.
|
||||
const seedThenFailReset = async (
|
||||
rel: string,
|
||||
source: string,
|
||||
): Promise<{
|
||||
file: { path: string; size: number };
|
||||
chunkHash: string;
|
||||
warm: ReturnType<typeof newCache>;
|
||||
}> => {
|
||||
const file = writeFile(rel, source);
|
||||
const chunkHash = computeChunkHash([
|
||||
{ filePath: file.path, contentHash: fileContentHash(source) },
|
||||
]);
|
||||
const cold = newCache();
|
||||
await run(cold, [file]); // miss → populates the parse cache + durable shards
|
||||
await persistCaches(cold);
|
||||
|
||||
// Corrupt (do not delete) one durable shard: the coherence gate then
|
||||
// re-dispatches while the previous generation stays on disk, which is the
|
||||
// only way a chunk is both a live `.v8` entry and a miss in one run.
|
||||
const chunkDir = path.join(getDurableParsedFileDir(storageDir), chunkHash);
|
||||
const shard = fs.readdirSync(chunkDir).find((name) => name.endsWith('.v8'));
|
||||
if (!shard) throw new Error('expected a durable shard to corrupt');
|
||||
fs.writeFileSync(path.join(chunkDir, shard), Buffer.from([0, 1, 2]));
|
||||
|
||||
const { loadParseCache } = await import('../../src/storage/parse-cache.js');
|
||||
const warm = (await loadParseCache(storageDir)) as ReturnType<typeof newCache>;
|
||||
expect(warm.onDiskKeys.has(chunkHash)).toBe(true); // the old generation is live
|
||||
fs.rmSync(markerPath, { force: true });
|
||||
|
||||
prepareOverride.impl = () => Promise.reject(new Error('EACCES: simulated cache failure'));
|
||||
try {
|
||||
await run(warm, [file]);
|
||||
} finally {
|
||||
prepareOverride.impl = undefined;
|
||||
}
|
||||
expect(fs.existsSync(markerPath)).toBe(true); // the gate did fall through
|
||||
return { file, chunkHash, warm };
|
||||
};
|
||||
|
||||
const readSavedIndexKeys = (): string[] => {
|
||||
const raw = fs.readFileSync(path.join(storageDir, 'parse-cache', 'index.json'), 'utf-8');
|
||||
return (JSON.parse(raw) as { keys: string[] }).keys;
|
||||
};
|
||||
|
||||
it('drops a pre-existing cache entry when the durable generation could not be reset', async () => {
|
||||
const { chunkHash, warm } = await seedThenFailReset(
|
||||
'src/stale-carryforward.ts',
|
||||
'export function carried() { return 1; }\n',
|
||||
);
|
||||
|
||||
const { saveParseCache, pruneCache } = await import('../../src/storage/parse-cache.js');
|
||||
pruneCache(warm, warm.usedKeys);
|
||||
const saved = await saveParseCache(storageDir, warm);
|
||||
|
||||
expect(saved).not.toContain(chunkHash);
|
||||
expect(readSavedIndexKeys()).not.toContain(chunkHash);
|
||||
expect(fs.existsSync(path.join(storageDir, 'parse-cache', `${chunkHash}.v8`))).toBe(false);
|
||||
|
||||
await pruneAndSaveDurableParsedFileStore(
|
||||
getDurableParsedFileDir(storageDir),
|
||||
PARSE_CACHE_VERSION,
|
||||
new Set(saved),
|
||||
);
|
||||
const { loadDurableParsedFileIndex } = await import('../../src/storage/parsedfile-store.js');
|
||||
const durable = await loadDurableParsedFileIndex(
|
||||
getDurableParsedFileDir(storageDir),
|
||||
PARSE_CACHE_VERSION,
|
||||
);
|
||||
expect(durable.has(chunkHash)).toBe(false);
|
||||
});
|
||||
|
||||
it('keeps the chunk excluded when a post-parse merge re-adds its key', async () => {
|
||||
// run-analyze folds sibling-branch keys into usedKeys AFTER the parse phase
|
||||
// (#2106), and retains every loaded key when a sibling meta is unreadable.
|
||||
// Invalidation has to outlive both, which is why it is filtered at save.
|
||||
const { chunkHash, warm } = await seedThenFailReset(
|
||||
'src/stale-readd.ts',
|
||||
'export function readded() { return 1; }\n',
|
||||
);
|
||||
|
||||
// Model both merges: the sibling fold re-adds the key, and the
|
||||
// unreadable-meta fallback unions `entries.keys()` back into `usedKeys`.
|
||||
// Either one would resurrect the chunk if invalidation were a deletion
|
||||
// from `usedKeys` instead of a filter at save time.
|
||||
warm.usedKeys.add(chunkHash);
|
||||
warm.entries.set(chunkHash, [] as unknown as ParseWorkerResult[]);
|
||||
warm.onDiskKeys?.add(chunkHash);
|
||||
for (const key of warm.entries.keys()) warm.usedKeys.add(key);
|
||||
|
||||
const { saveParseCache } = await import('../../src/storage/parse-cache.js');
|
||||
const saved = await saveParseCache(storageDir, warm);
|
||||
|
||||
expect(saved).not.toContain(chunkHash);
|
||||
expect(readSavedIndexKeys()).not.toContain(chunkHash);
|
||||
});
|
||||
|
||||
it('retires the chunk when the failed reset left the old generation on disk', async () => {
|
||||
const { chunkHash, warm } = await seedThenFailReset(
|
||||
'src/stale-marking-site.ts',
|
||||
'export function marked() { return 1; }\n',
|
||||
);
|
||||
|
||||
expect(warm.staleKeys?.has(chunkHash)).toBe(true);
|
||||
// `onDiskKeys` carried this hash from the loaded index, so clearing it is
|
||||
// observable; `entries` is empty on the sharded path, which is why the
|
||||
// in-memory half is proved by the legacy-cache case below instead.
|
||||
expect(warm.onDiskKeys?.has(chunkHash)).toBe(false);
|
||||
});
|
||||
|
||||
it('clears an in-memory entry for a retired chunk', async () => {
|
||||
// `markParseCacheChunkStale` also drops `entries`, which only matters for a
|
||||
// legacy (non-sharded) cache whose payloads live in memory. Drive the
|
||||
// helper directly — the sharded pipeline never populates `entries`, so the
|
||||
// pipeline test above cannot observe this half.
|
||||
const { markParseCacheChunkStale, saveParseCache } =
|
||||
await import('../../src/storage/parse-cache.js');
|
||||
const cache = newCache();
|
||||
const chunkHash = 'a'.repeat(64);
|
||||
cache.entries.set(chunkHash, [] as unknown as ParseWorkerResult[]);
|
||||
cache.onDiskKeys?.add(chunkHash);
|
||||
cache.usedKeys.add(chunkHash);
|
||||
|
||||
markParseCacheChunkStale(cache, chunkHash);
|
||||
|
||||
expect(cache.entries.has(chunkHash)).toBe(false);
|
||||
expect(cache.onDiskKeys?.has(chunkHash)).toBe(false);
|
||||
expect(await saveParseCache(storageDir, cache)).not.toContain(chunkHash);
|
||||
});
|
||||
|
||||
it('does NOT retire a chunk when the failed reset left no old generation', async () => {
|
||||
// `prepareDurableParsedFileChunk` is rm-then-mkdir. When the rm succeeded
|
||||
// and the mkdir failed there is nothing stale to protect against — the
|
||||
// workers recreate the directory and write a clean generation — so
|
||||
// retiring would throw away a good cache entry. Only an rm failure, which
|
||||
// leaves shards behind, justifies retirement.
|
||||
const source = 'export function mkdirOnly() { return 1; }\n';
|
||||
const file = writeFile('src/mkdir-only.ts', source);
|
||||
const chunkHash = computeChunkHash([
|
||||
{ filePath: file.path, contentHash: fileContentHash(source) },
|
||||
]);
|
||||
const cold = newCache();
|
||||
await run(cold, [file]);
|
||||
await persistCaches(cold);
|
||||
|
||||
// Corrupt a shard so the coherence gate re-dispatches, then model the
|
||||
// rm-succeeded/mkdir-failed shape: the directory is gone when prepare throws.
|
||||
const chunkDir = path.join(getDurableParsedFileDir(storageDir), chunkHash);
|
||||
const shard = fs.readdirSync(chunkDir).find((name) => name.endsWith('.v8'));
|
||||
if (!shard) throw new Error('expected a durable shard to corrupt');
|
||||
fs.writeFileSync(path.join(chunkDir, shard), Buffer.from([0, 1, 2]));
|
||||
|
||||
const { loadParseCache, saveParseCache } = await import('../../src/storage/parse-cache.js');
|
||||
const warm = (await loadParseCache(storageDir)) as ParseCache;
|
||||
prepareOverride.impl = async (durableDir, hash) => {
|
||||
fs.rmSync(path.join(durableDir, hash), { recursive: true, force: true });
|
||||
throw new Error('EMFILE: simulated mkdir failure after a clean rm');
|
||||
};
|
||||
try {
|
||||
await run(warm, [file]);
|
||||
} finally {
|
||||
prepareOverride.impl = undefined;
|
||||
}
|
||||
|
||||
expect(warm.staleKeys?.has(chunkHash) ?? false).toBe(false);
|
||||
expect(await saveParseCache(storageDir, warm)).toContain(chunkHash);
|
||||
});
|
||||
|
||||
it('still saves a chunk whose durable generation reset succeeded', async () => {
|
||||
const f = writeFile('src/healthy.ts', 'export function healthy() { return 1; }\n');
|
||||
const chunkHash = computeChunkHash([
|
||||
{
|
||||
filePath: f.path,
|
||||
contentHash: fileContentHash('export function healthy() { return 1; }\n'),
|
||||
},
|
||||
]);
|
||||
const cache = newCache();
|
||||
|
||||
await run(cache, [f]);
|
||||
const { saveParseCache } = await import('../../src/storage/parse-cache.js');
|
||||
const saved = await saveParseCache(storageDir, cache);
|
||||
|
||||
expect(saved).toContain(chunkHash);
|
||||
expect(cache.staleKeys?.has(chunkHash) ?? false).toBe(false);
|
||||
expect(fs.existsSync(path.join(storageDir, 'parse-cache', `${chunkHash}.v8`))).toBe(true);
|
||||
const { loadDurableParsedFileIndex: loadIdx } =
|
||||
await import('../../src/storage/parsedfile-store.js');
|
||||
await pruneAndSaveDurableParsedFileStore(
|
||||
getDurableParsedFileDir(storageDir),
|
||||
PARSE_CACHE_VERSION,
|
||||
new Set(saved),
|
||||
);
|
||||
expect(
|
||||
(await loadIdx(getDurableParsedFileDir(storageDir), PARSE_CACHE_VERSION)).has(chunkHash),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it('re-dispatches on the run after a failed reset instead of taking a warm hit', async () => {
|
||||
const { file, chunkHash, warm } = await seedThenFailReset(
|
||||
'src/stale-nextrun.ts',
|
||||
'export function nextRun() { return 1; }\n',
|
||||
);
|
||||
await persistCaches(warm);
|
||||
|
||||
const { loadParseCache } = await import('../../src/storage/parse-cache.js');
|
||||
const third = (await loadParseCache(storageDir)) as ReturnType<typeof newCache>;
|
||||
expect(third.onDiskKeys.has(chunkHash)).toBe(false);
|
||||
fs.rmSync(markerPath, { force: true });
|
||||
|
||||
await run(third, [file]);
|
||||
|
||||
expect(fs.existsSync(markerPath)).toBe(true);
|
||||
});
|
||||
|
||||
it('retires only the failing chunk — a sibling chunk stays warm through the next run', async () => {
|
||||
// The single-chunk tests cannot tell "retires the failing chunk" from
|
||||
// "retires everything": one chunk plus a global spawn marker look the same
|
||||
// either way. Two chunks, one failure, and a per-chunk assertion can.
|
||||
const aSrc = 'export function a() { return 1; }\n';
|
||||
const bSrc = 'export function b() { return 2; }\n';
|
||||
const a = writeFile('src/a.ts', aSrc);
|
||||
const b = writeFile('src/b.ts', bSrc);
|
||||
const hashOf = (rel: string, src: string): string =>
|
||||
computeChunkHash([{ filePath: rel, contentHash: fileContentHash(src) }]);
|
||||
const aHash = hashOf(a.path, aSrc);
|
||||
const bHash = hashOf(b.path, bSrc);
|
||||
|
||||
// chunkByteBudget 1 forces one file per chunk, so a and b hash distinctly.
|
||||
const cold = newCache();
|
||||
await run(cold, [a, b], 1);
|
||||
await persistCaches(cold);
|
||||
|
||||
// Corrupt only a's durable shard: a re-dispatches, b still hits warm.
|
||||
const aDir = path.join(getDurableParsedFileDir(storageDir), aHash);
|
||||
const aShard = fs.readdirSync(aDir).find((name) => name.endsWith('.v8'));
|
||||
if (!aShard) throw new Error('expected a durable shard for a');
|
||||
fs.writeFileSync(path.join(aDir, aShard), Buffer.from([0, 1, 2]));
|
||||
|
||||
const { loadParseCache } = await import('../../src/storage/parse-cache.js');
|
||||
const warm = (await loadParseCache(storageDir)) as ParseCache;
|
||||
prepareOverride.impl = (_durableDir, hash) =>
|
||||
hash === aHash
|
||||
? Promise.reject(new Error('EACCES: simulated cache failure'))
|
||||
: Promise.resolve();
|
||||
try {
|
||||
await run(warm, [a, b], 1);
|
||||
} finally {
|
||||
prepareOverride.impl = undefined;
|
||||
}
|
||||
await persistCaches(warm);
|
||||
|
||||
// b survived the save; only a was retired.
|
||||
expect(warm.staleKeys?.has(aHash)).toBe(true);
|
||||
expect(warm.staleKeys?.has(bHash) ?? false).toBe(false);
|
||||
|
||||
// Actually perform the next run. An index entry alone does not exercise the
|
||||
// warm-hit/coherence path, so the claim in the title has to be paid for.
|
||||
// Run each chunk on its own so the single global spawn marker is
|
||||
// unambiguous about WHICH chunk re-dispatched.
|
||||
const bOnly = (await loadParseCache(storageDir)) as ParseCache;
|
||||
fs.rmSync(markerPath, { force: true });
|
||||
await run(bOnly, [b], 1);
|
||||
expect(fs.existsSync(markerPath)).toBe(false); // sibling served warm
|
||||
|
||||
const aOnly = (await loadParseCache(storageDir)) as ParseCache;
|
||||
fs.rmSync(markerPath, { force: true });
|
||||
await run(aOnly, [a], 1);
|
||||
expect(fs.existsSync(markerPath)).toBe(true); // retired chunk re-parsed
|
||||
});
|
||||
|
||||
it('retains worker ParsedFiles when the main-thread run-store write fails', async () => {
|
||||
const f = writeFile(
|
||||
'src/persist-fallback.ts',
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
import { describe, it, expect, vi } from 'vitest';
|
||||
import { promises as nodeFsPromises } from 'node:fs';
|
||||
import { promises as nodeFsPromises, existsSync } from 'node:fs';
|
||||
import v8 from 'node:v8';
|
||||
import { mkdtemp, rm, readdir, readFile, writeFile } from 'fs/promises';
|
||||
import { tmpdir } from 'os';
|
||||
|
|
@ -18,6 +18,7 @@ import {
|
|||
prepareDurableParsedFileChunk,
|
||||
pruneAndSaveDurableParsedFileStore,
|
||||
mergeStagedDurableParsedFileStore,
|
||||
loadDurableParsedFileIndex,
|
||||
} from '../../src/storage/parsedfile-store.js';
|
||||
|
||||
/**
|
||||
|
|
@ -908,6 +909,52 @@ describe('parsedfile-store receiverChain sanitation', () => {
|
|||
}
|
||||
});
|
||||
|
||||
// chmod is the only lever that makes a real `fs.rm` reject here, and root
|
||||
// ignores directory write permission while Windows treats the mode bits as a
|
||||
// near no-op. Skipping is honest; a green vacuous run is not.
|
||||
it.skipIf(process.platform === 'win32' || process.getuid?.() === 0)(
|
||||
'keeps pruning and still writes the index when one chunk directory cannot be removed',
|
||||
async () => {
|
||||
// #3204: the non-survivor delete targets the same directory whose reset
|
||||
// may have failed, so the causes that break the reset (permissions, a
|
||||
// locked file, a read-only mount) break this rm too. One undeletable
|
||||
// directory must not cost every other chunk its index entry.
|
||||
const dir = await mkdtemp(path.join(tmpdir(), 'pf-rm-fail-'));
|
||||
const durableDir = getDurableParsedFileDir(dir);
|
||||
const undeletable = '3'.repeat(64);
|
||||
const keep = '4'.repeat(64);
|
||||
try {
|
||||
await prepareDurableParsedFileChunk(durableDir, undeletable);
|
||||
persistDurableParsedFileShardSync(durableDir, undeletable, 1, 0, [
|
||||
makeParsedFile('gone.c'),
|
||||
]);
|
||||
await prepareDurableParsedFileChunk(durableDir, keep);
|
||||
persistDurableParsedFileShardSync(durableDir, keep, 1, 0, [makeParsedFile('keep.c')]);
|
||||
|
||||
// Clearing write permission on the chunk directory makes its shards
|
||||
// un-unlinkable, so the recursive rm of that directory rejects while the
|
||||
// store root stays writable for the index rewrite.
|
||||
const doomed = path.join(durableDir, undeletable);
|
||||
await nodeFsPromises.chmod(doomed, 0o555);
|
||||
await expect(
|
||||
pruneAndSaveDurableParsedFileStore(durableDir, 'v-test', new Set([keep])),
|
||||
).resolves.toBeUndefined();
|
||||
|
||||
// Assert the premise: the directory SURVIVED, i.e. the rm really did
|
||||
// reject. Without this the test passes wherever the delete succeeds and
|
||||
// would stay green if the try/catch were reverted.
|
||||
expect(existsSync(doomed)).toBe(true);
|
||||
|
||||
const index = await loadDurableParsedFileIndex(durableDir, 'v-test');
|
||||
expect(index.has(keep)).toBe(true);
|
||||
expect(index.has(undeletable)).toBe(false);
|
||||
} finally {
|
||||
await nodeFsPromises.chmod(path.join(durableDir, undeletable), 0o755).catch(() => {});
|
||||
await rm(dir, { recursive: true, force: true });
|
||||
}
|
||||
},
|
||||
);
|
||||
|
||||
it('overlays staged durable chunks onto the live store without dropping live-only keys', async () => {
|
||||
const live = await mkdtemp(path.join(tmpdir(), 'pf-live-'));
|
||||
const staged = await mkdtemp(path.join(tmpdir(), 'pf-stg-'));
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue