fix(generation-store): commitTransaction refuses while single-ops are pending — the order invariant is enforced, not assumed
Some checks are pending
Some checks are pending
reservedGensAsc() documented but never enforced that pending single-op generations must sort above every committed one. A direct commitTransaction() call bypassing Brainy.transact()'s flush-first step could commit a fresh generation into committedRanges above lower, still-pending ones, unsorting the committed-then-pending walk resolveManyAt relies on and returning a wrong before-image for a point-in-time read — silently. commitTransaction() now refuses via a new PendingSingleOpsUnflushedError when the pending tier is non-empty, before any staging I/O. Behavior-neutral: both sanctioned callers (Brainy.transact(), Brainy.compactHistory()) already flush first.
This commit is contained in:
parent
da9519903a
commit
a79db434ac
4 changed files with 371 additions and 2 deletions
|
|
@ -32,7 +32,13 @@
|
|||
*/
|
||||
|
||||
import { prodLog } from '../utils/logger.js'
|
||||
import { GenerationCompactedError, GenerationConflictError, PendingFlushDurabilityError, StoreInconsistentError } from './errors.js'
|
||||
import {
|
||||
GenerationCompactedError,
|
||||
GenerationConflictError,
|
||||
PendingFlushDurabilityError,
|
||||
PendingSingleOpsUnflushedError,
|
||||
StoreInconsistentError
|
||||
} from './errors.js'
|
||||
import type { UnreconciledRecord } from './errors.js'
|
||||
import { TransactionRollbackError } from '../transaction/errors.js'
|
||||
import type {
|
||||
|
|
@ -1351,6 +1357,9 @@ export class GenerationStore {
|
|||
* @param args.execute - Runs the planned operation batch atomically.
|
||||
* @returns The committed generation and its commit timestamp.
|
||||
* @throws GenerationConflictError when the CAS expectation fails.
|
||||
* @throws PendingSingleOpsUnflushedError when the pending single-op tier is
|
||||
* non-empty — call `flushPendingSingleOps()` first (both `Brainy.transact()`
|
||||
* and `Brainy.compactHistory()` already do).
|
||||
*/
|
||||
/**
|
||||
* The generation fact log, or `null` when the storage layer cannot host one.
|
||||
|
|
@ -1425,6 +1434,13 @@ export class GenerationStore {
|
|||
execute: () => Promise<void>
|
||||
}): Promise<{ generation: number; timestamp: number }> {
|
||||
return this.withMutex(async () => {
|
||||
// The generation-order guard (see assertPendingSingleOpsFlushed): a
|
||||
// direct commitTransaction() call while single-ops are still pending
|
||||
// would commit above them, unsorting reservedGensAsc() and corrupting
|
||||
// point-in-time reads. Both sanctioned callers (Brainy.transact(),
|
||||
// Brainy.compactHistory()) already flush first, so this is
|
||||
// behavior-neutral on every real path.
|
||||
this.assertPendingSingleOpsFlushed()
|
||||
// A latched history-durability failure compromises the whole generation
|
||||
// chain — refuse a transact too (advancing the manifest past stuck,
|
||||
// un-durable single-op generations would be inconsistent). Same loud
|
||||
|
|
@ -2294,6 +2310,37 @@ export class GenerationStore {
|
|||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* @description Throw if the pending single-op tier is non-empty. Called at
|
||||
* the top of {@link commitTransaction} (the ONLY method that appends a
|
||||
* fresh commit directly into {@link committedRanges} outside recovery) so
|
||||
* the ordering invariant {@link reservedGensAsc}'s own doc comment states —
|
||||
* "pending generations are always greater than every committed one" — is
|
||||
* ENFORCED there rather than merely assumed.
|
||||
*
|
||||
* That invariant holds today only because both sanctioned callers flush the
|
||||
* pending tier before committing: `Brainy.transact()` (src/brainy.ts,
|
||||
* `await this.generationStore.flushPendingSingleOps()` immediately before
|
||||
* its `commitTransaction()` call) and `Brainy.compactHistory()`
|
||||
* (src/brainy.ts, the same flush immediately before its `compact()` call —
|
||||
* `compact()` itself only ever RECLAIMS an existing committed prefix, so it
|
||||
* cannot land a commit out of order and needs no guard of its own). A
|
||||
* caller that reaches `commitTransaction()` by any other path — bypassing
|
||||
* that flush — would commit a new generation into `committedRanges` ABOVE
|
||||
* generations still sitting in `pendingGens`, breaking `reservedGensAsc`'s
|
||||
* "committed-then-pending is already sorted" assumption and making
|
||||
* `resolveManyAt`'s single ascending pass (and `resolveAt`'s consumers)
|
||||
* return the WRONG before-image for a point-in-time read — silently, no
|
||||
* compensating error. Refusing here, before any staging I/O, keeps the
|
||||
* store untouched (nothing committed, nothing staged, the generation
|
||||
* counter reservation unaffected) on every path that already flushes.
|
||||
*/
|
||||
private assertPendingSingleOpsFlushed(): void {
|
||||
if (this.pendingGens.length > 0) {
|
||||
throw new PendingSingleOpsUnflushedError(this.pendingGens.length)
|
||||
}
|
||||
}
|
||||
|
||||
/** Schedule a coalesced pending-tier flush (size trigger fires immediately on
|
||||
* the next microtask; otherwise a {@link PENDING_FLUSH_DELAY_MS} timer). Both
|
||||
* defer outside the current mutex section so the flush can re-acquire it. A
|
||||
|
|
@ -2377,6 +2424,13 @@ export class GenerationStore {
|
|||
* committed-then-pending concatenation is already sorted — identical to the old
|
||||
* `[...committedGens, ...pendingGens]`. This is the union historical reads
|
||||
* resolve over so un-flushed single-ops are visible to pins/`asOf`.
|
||||
*
|
||||
* The "flush first" half of that invariant is ENFORCED, not just documented:
|
||||
* {@link commitTransaction} — the only method that lands a fresh commit into
|
||||
* {@link committedRanges} outside crash recovery — refuses via
|
||||
* {@link assertPendingSingleOpsFlushed} whenever {@link pendingGens} is
|
||||
* non-empty, so a committed generation can never land above a still-pending
|
||||
* one and break this ordering.
|
||||
*/
|
||||
private *reservedGensAsc(): IterableIterator<number> {
|
||||
yield* this.committedGensAsc()
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue