fix(storage): a suspect count ledger heals itself, and counts.json is written atomically
Some checks are pending
CI / Node 22 (push) Waiting to run
CI / Node 24 (push) Waiting to run
CI / Integration + conformance (Node 22) (push) Waiting to run
CI / Bun (latest) (push) Waiting to run
Some checks are pending
CI / Node 22 (push) Waiting to run
CI / Node 24 (push) Waiting to run
CI / Integration + conformance (Node 22) (push) Waiting to run
CI / Bun (latest) (push) Waiting to run
MEASURED on a real store: the ALL-visibility ledger read 14,231 nouns against 14,056 identity records and 72,729 verbs against 72,679 — exactly that store's 25 noun and 50 verb SCAR directories. Two copies of the same archive derived different numbers (14,231 and 14,081), because each had been persisted at a different moment under the old rule that counted one entity per id DIRECTORY. A downstream index heal subtracted against that denominator and reported remaining work that did not exist. The scan already applies the right predicate — one entity per IDENTITY RECORD (the metadata content leg), shared with pruneOrphanedEntities so the two agree by construction. What was missing is that a ledger persisted under the old rule was only FLAGGED suspect and then went on serving its wrong numbers for the life of the store, waiting for an operator to run repairIndex. - The ledger now derives itself honestly in the BACKGROUND after the open, narrating start and finish with the correction it made. Background because these scalars are denominators — no read is served from them — and because walks exactly like these are how a 24,898-id store spent minutes of a restart in silence. Observable via whenCountLedgerSettled(); nothing in the read path waits on it. - A derivation that raced a write refuses to stamp its number "exact": one retry on a quiet store, then the ledger stays SUSPECT and says so, naming repairIndex as the door that recounts under a barrier. - The one derivation that CANNOT leave the foreground says why it cannot: getNounCount()/getVerbCount() are served from it, and a background walk would make a populated store answer "0 entities" — a wrong answer, not a slow one. It narrates its start and its wall instead. - counts.json is written temp+rename. A truncating write left a window — measured at roughly 750ms after a flush or close — in which a concurrent reader saw the file EMPTY; an unparseable ledger sends the next open down the full-rescan path, so the cheapest file in the store was buying the most expensive recovery. - The writer lock's clean-close record is now consulted before the same-process branch too: a restart reported "Re-acquiring writer lock ... this is a bug" immediately after a clean close, sending an operator after a leak that did not exist. Pins: tests/integration/count-ledger-identity-record.test.ts (background correction with scar and ghost fixtures, two copies of one archive agreeing, counts.json never observed unparseable across 40 persists); tests/integration/ledger-derivation-identity.test.ts updated to the new law — the OPEN still never walks (proved by slowing the walk 1.2s and timing the open), and the ledger heals behind it.
This commit is contained in:
parent
afe08a1ff9
commit
f4e2d34b4e
3 changed files with 519 additions and 72 deletions
|
|
@ -11,14 +11,22 @@
|
|||
* (1) IDENTITY, NOT CONTAINER — the derivation counts one entity per
|
||||
* metadata content leg (`metadata.json` or `.json.gz`), the same test
|
||||
* `pruneOrphanedEntities()` uses, so the two agree by construction.
|
||||
* (2) THE STAMP NAMES SUSPECT COUNTS LOUDLY, AT O(1) — a counts.json that
|
||||
* carries the ALL scalars but no `allCountsDerivedBy: 'identity-record'`
|
||||
* stamp predates this fix; loading it marks `suspect = true` from a
|
||||
* single field read alone, never a directory walk, and warns exactly
|
||||
* once naming the cause.
|
||||
* (3) THE SANCTIONED RECOUNT CLEARS IT — `repairIndex()` prunes the orphaned
|
||||
* containers, recounts from the canonical metadata.json walk, and
|
||||
* re-stamps — suspect clears and the ALL scalar is exact again.
|
||||
* (2) THE STAMP NAMES SUSPECT COUNTS LOUDLY, AND THE OPEN NEVER WALKS — a
|
||||
* counts.json that carries the ALL scalars but no
|
||||
* `allCountsDerivedBy: 'identity-record'` stamp predates this fix;
|
||||
* loading it marks `suspect = true` from a single field read alone and
|
||||
* warns exactly once naming the cause. The open itself never pays a
|
||||
* directory walk.
|
||||
* (2b) AND IT HEALS ITSELF. The ledger used to stay wrong for the life of the
|
||||
* store, waiting for an operator to run `repairIndex()` — and a
|
||||
* downstream index heal subtracted against the inflated denominator and
|
||||
* reported work that did not exist. An honest derivation now runs in the
|
||||
* BACKGROUND after the open (never blocking it, observable via
|
||||
* `whenCountLedgerSettled()`), and refuses to stamp a number it derived
|
||||
* while writes were landing.
|
||||
* (3) THE SANCTIONED RECOUNT ALSO CLEARS IT — `repairIndex()` prunes the
|
||||
* orphaned containers, recounts from the canonical metadata.json walk,
|
||||
* and re-stamps — the ALL scalar is exact and the containers are gone.
|
||||
* (4) A FRESH STORE IS NEVER SUSPECT — the one-time derivation for a store
|
||||
* with no counts.json stamps as it writes, so a brand-new store never
|
||||
* carries the legacy signature.
|
||||
|
|
@ -115,29 +123,43 @@ describe('ledger derivation identity — the ALL scalar is the identity-record p
|
|||
delete raw.allCountsDerivedBy
|
||||
fs.writeFileSync(countsPath(dir), JSON.stringify(raw, null, 2))
|
||||
|
||||
const warnSpy = vi.spyOn(prodLog, 'warn')
|
||||
// The two derivation walks live on FileSystemStorage's prototype —
|
||||
// spying here (rather than on fs.promises.readdir globally) isolates
|
||||
// THIS code path's behavior from unrelated walks elsewhere in the open
|
||||
// sequence (a separate, pre-existing engine's own O(store) cost — not
|
||||
// this fix's concern, and not something this pin should be sensitive
|
||||
// to). Neither derivation method may run: the stamp check is a field
|
||||
// read on the already-parsed counts.json, nothing more.
|
||||
const scanEntitiesSpy = vi.spyOn(FileSystemStorage.prototype as any, 'scanCanonicalEntities')
|
||||
const scanVectoredSpy = vi.spyOn(FileSystemStorage.prototype as any, 'scanVectoredNounCount')
|
||||
const narrateSpy = vi.spyOn(prodLog, 'narrate')
|
||||
// The derivation walks live on FileSystemStorage's prototype. Slow them
|
||||
// deliberately: the OPEN must not wait for them, and on a two-row store a
|
||||
// real walk finishes too fast to tell "not awaited" from "instant".
|
||||
const proto = FileSystemStorage.prototype as any
|
||||
const realScanEntities = proto.scanCanonicalEntities
|
||||
let scanEntitiesCalls = 0
|
||||
proto.scanCanonicalEntities = async function slow(this: any, ...args: any[]) {
|
||||
scanEntitiesCalls++
|
||||
await new Promise((r) => setTimeout(r, 1_200))
|
||||
return realScanEntities.apply(this, args)
|
||||
}
|
||||
try {
|
||||
const openStarted = Date.now()
|
||||
brain = await open()
|
||||
const openMs = Date.now() - openStarted
|
||||
|
||||
brain = await open()
|
||||
// THE OPEN DID NOT WALK: two slowed walks would have added 2.4s to it.
|
||||
expect(openMs).toBeLessThan(2_000)
|
||||
|
||||
const ledger = await brain.storage.getCanonicalCounts()
|
||||
expect(ledger.suspect).toBe(true)
|
||||
// The stamp check itself is an O(1) field read, and it names the cause.
|
||||
const atOpen = await brain.storage.getCanonicalCounts()
|
||||
expect(atOpen.suspect).toBe(true)
|
||||
const stampWarnings = narrateSpy.mock.calls.filter(
|
||||
([msg]: any[]) => String(msg).includes('legacy') && String(msg).includes('container rule')
|
||||
)
|
||||
expect(stampWarnings.length).toBe(1) // exactly one, loud
|
||||
|
||||
const stampWarnings = warnSpy.mock.calls.filter(
|
||||
([msg]) => String(msg).includes('legacy') && String(msg).includes('container rule')
|
||||
)
|
||||
expect(stampWarnings.length).toBe(1) // exactly one, loud
|
||||
|
||||
expect(scanEntitiesSpy).not.toHaveBeenCalled() // O(1) field read only, no re-derivation walk
|
||||
expect(scanVectoredSpy).not.toHaveBeenCalled()
|
||||
// ...and the honest derivation is already running behind the open.
|
||||
await brain.storage.whenCountLedgerSettled()
|
||||
expect(scanEntitiesCalls).toBeGreaterThan(0)
|
||||
const healed = await brain.storage.getCanonicalCounts()
|
||||
expect(healed.suspect).toBe(false)
|
||||
expect(healed.nouns.all).toBe(raw.totalNounCountAll)
|
||||
} finally {
|
||||
proto.scanCanonicalEntities = realScanEntities
|
||||
}
|
||||
|
||||
await brain.close()
|
||||
})
|
||||
|
|
@ -163,7 +185,14 @@ describe('ledger derivation identity — the ALL scalar is the identity-record p
|
|||
fs.writeFileSync(countsPath(dir), JSON.stringify(raw, null, 2))
|
||||
|
||||
brain = await open()
|
||||
expect((await brain.storage.getCanonicalCounts()).suspect).toBe(true) // named suspect at load
|
||||
// Named suspect at load, then healed in the background WITHOUT the
|
||||
// operator asking — the inflated container count is corrected to the
|
||||
// identity-record population, though the orphaned containers themselves
|
||||
// are still on disk (only repairIndex() removes those).
|
||||
await brain.storage.whenCountLedgerSettled()
|
||||
let healed = await brain.storage.getCanonicalCounts()
|
||||
expect(healed.suspect).toBe(false)
|
||||
expect(healed.nouns.all).toBe(realTotal)
|
||||
|
||||
await brain.repairIndex()
|
||||
|
||||
|
|
|
|||
Reference in a new issue