fix(locks): live writers are never auto-evicted; evicted writers are fenced at every commit barrier
The production dev-store split-brain (two live writers alternating a store's
id-mapper between two internally-consistent truths), cured at all three of
its roots. (1) STALENESS REQUIRES PID-DEATH: the old rule evicted on
heartbeat age alone, so a >60s event-loop stall (debugger pause, GC, heavy
sync work) handed the lock to a second opener while the first kept writing;
a live process is now never auto-evicted — a wedged-but-alive holder is the
operator's call via {force:true}, and the heartbeat stays for observability.
(2) THE CLAIM IS ATOMIC: writeFile(wx)'s open→write→close left an empty-file
window a concurrent opener could read as torn, unlink a LIVE claim, and take
the lock; the claim is now tmp-write + hard-link — the lock appears with its
full contents in one step. (3) THE FENCE: every flush commit and transact
barrier verifies lock ownership first (one small read per window) — a
forced-out or lock-deleted writer fails typed (BRAINY_WRITER_FENCED) before
a single staged byte or manifest advance, instead of writing on unaware.
Pinned: live-with-ancient-heartbeat refuses typed; dead-PID self-clears
narrated; a forced-out writer's flush and transact both fence, advancing
nothing. Requested by a downstream team as single-writer guard or loud
lockout — this is both.
This commit is contained in:
parent
9ac9e70686
commit
292e7c0406
5 changed files with 225 additions and 8 deletions
|
|
@ -1920,14 +1920,24 @@ export class FileSystemStorage extends BaseStorage {
|
|||
rootDir: this.rootDir
|
||||
}
|
||||
|
||||
// The atomic claim: create-exclusive, so exactly ONE racer wins.
|
||||
// The atomic claim: write the FULL contents to a temp file, then
|
||||
// hard-link it into place — link(2) fails EEXIST if the target exists,
|
||||
// and the lock file appears with its complete JSON in one atomic step.
|
||||
// (The previous claim was writeFile with O_EXCL, whose open→write→close
|
||||
// is NOT atomic: a concurrent opener could read the file in its empty
|
||||
// window, judge it torn, unlink a LIVE claim, and take the lock — two
|
||||
// live writers. The link claim leaves no empty window to misread.)
|
||||
const claimTmp = `${lockFile}.claim-${myPid}-${Date.now()}`
|
||||
try {
|
||||
await fs.promises.writeFile(lockFile, JSON.stringify(info, null, 2), { flag: 'wx' })
|
||||
await fs.promises.writeFile(claimTmp, JSON.stringify(info, null, 2))
|
||||
await fs.promises.link(claimTmp, lockFile)
|
||||
} catch (err: any) {
|
||||
if (err.code === 'EEXIST') {
|
||||
continue // someone else claimed between our read and create — re-evaluate
|
||||
}
|
||||
throw err
|
||||
} finally {
|
||||
await fs.promises.unlink(claimTmp).catch(() => {})
|
||||
}
|
||||
|
||||
this.installWriterLock(info)
|
||||
|
|
@ -1972,6 +1982,44 @@ export class FileSystemStorage extends BaseStorage {
|
|||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* THE FENCE: verify this instance still owns the writer lock before a
|
||||
* commit barrier proceeds. An evicted writer (an operator's
|
||||
* `{ force: true }` takeover, or an operator deleting the lock file) must
|
||||
* fail LOUDLY on its next flush instead of writing on unaware — the
|
||||
* unfenced evicted writer was half of a production split-brain (each
|
||||
* writer flushing its own internally-consistent id-mapper snapshot,
|
||||
* alternating the store between two truths). One small file read per
|
||||
* flush window, never per record. No-op when this instance holds no
|
||||
* writer lock (read-only opens, in-memory stores).
|
||||
*
|
||||
* @throws `BRAINY_WRITER_FENCED` when the lock is gone or held by another.
|
||||
*/
|
||||
public override async assertWriterFenceHeld(): Promise<void> {
|
||||
if (!this.writerLockInfo) return
|
||||
const current = await this.readWriterLock()
|
||||
if (
|
||||
current &&
|
||||
current.pid === this.writerLockInfo.pid &&
|
||||
current.hostname === this.writerLockInfo.hostname &&
|
||||
current.startedAt === this.writerLockInfo.startedAt
|
||||
) {
|
||||
return
|
||||
}
|
||||
const err = new Error(
|
||||
`Writer fence lost for ${this.rootDir}: this process (PID ${this.writerLockInfo.pid}) ` +
|
||||
`no longer holds the writer lock — ` +
|
||||
(current
|
||||
? `it is now held by PID ${current.pid} on ${current.hostname} (since ${current.startedAt}).`
|
||||
: `the lock file is gone (released or removed by an operator).`) +
|
||||
`\nThis instance refuses to commit further writes: a fenced-out writer continuing to ` +
|
||||
`flush is how split-brain stores are made. Close this instance; if the takeover was a ` +
|
||||
`mistake, close the successor and re-open.`
|
||||
) as Error & { code: string }
|
||||
err.code = 'BRAINY_WRITER_FENCED'
|
||||
throw err
|
||||
}
|
||||
|
||||
/** The consumer-facing BRAINY_WRITER_LOCKED error, holder details attached. */
|
||||
private writerLockedError(existing: WriterLockInfo): Error {
|
||||
const err = new Error(
|
||||
|
|
@ -2060,18 +2108,25 @@ export class FileSystemStorage extends BaseStorage {
|
|||
|
||||
/**
|
||||
* Determine whether an existing writer lock is stale (safe to overwrite).
|
||||
* Same hostname and (dead PID OR heartbeat older than threshold) → stale.
|
||||
* Different hostname → cannot prove stale, treat as live.
|
||||
* Same hostname and DEAD PID → stale. That is the whole rule: a LIVE
|
||||
* process is never auto-evicted, however old its heartbeat — a >60s
|
||||
* event-loop stall (debugger pause, GC, heavy sync work) is a slow writer,
|
||||
* not a dead one, and heartbeat-age eviction of live writers was the
|
||||
* dominant mechanism behind a production split-brain (two live unaware
|
||||
* writers alternating a store's id-mapper between two truths). A holder
|
||||
* that LOOKS alive but is truly wedged is the operator's call via
|
||||
* `{ force: true }` — and the fence check on every flush
|
||||
* ({@link assertWriterFenceHeld}) guarantees a forced-out holder fails
|
||||
* loudly instead of writing on. Different hostname → cannot prove
|
||||
* anything, treat as live. The heartbeat remains for OBSERVABILITY (the
|
||||
* lock error names it so an operator can judge staleness themselves).
|
||||
*/
|
||||
private async isWriterLockStale(lock: WriterLockInfo): Promise<boolean> {
|
||||
const os = await import('node:os')
|
||||
if (lock.hostname !== os.hostname()) {
|
||||
return false
|
||||
}
|
||||
const heartbeatAge = Date.now() - new Date(lock.lastHeartbeat).getTime()
|
||||
const pidAlive = this.isPidAlive(lock.pid)
|
||||
if (!pidAlive) return true
|
||||
return heartbeatAge > FileSystemStorage.WRITER_STALE_THRESHOLD_MS
|
||||
return !this.isPidAlive(lock.pid)
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue