diff --git a/src/db/generationStore.ts b/src/db/generationStore.ts index e440f230..28b6276c 100644 --- a/src/db/generationStore.ts +++ b/src/db/generationStore.ts @@ -686,12 +686,21 @@ export class GenerationStore { faultPoint('after-staging') // -- 4. Execute the planned batch ------------------------------------- + // Durability barrier: record every canonical write/delete the operations + // make, then fsync them BELOW (before the counter advances). Without + // this, a hard kill after commitTransaction returns could leave the + // fsync'd generation counter ahead of still-page-cached entity bytes — + // phantom progress for any generation-based consumer. + this.storage.beginWriteBarrier?.() this.inTransact = true try { await args.execute() } finally { this.inTransact = false } + // The transaction's entire canonical footprint is now durable, so the + // counter/manifest advance below can never outrun the entity bytes. + await this.storage.flushWriteBarrier?.() faultPoint('after-execute') // -- 5. Counter + manifest rename (COMMIT POINT) ---------------------- @@ -787,6 +796,20 @@ export class GenerationStore { * mid-flush is recovered by drop-without-restore * (see {@link GenerationDelta.groupCommit}). * + * **Durability contract (single-op vs transact).** A single-op write's live + * canonical bytes are written via tmp+rename but NOT individually fsync'd — + * they are durable at the next {@link flushPendingSingleOps} or `close()`, not + * the instant the write resolves. On a hard kill before that flush, a + * single-op write can be lost even though the call returned; the group-commit + * counter is buffered alongside, so counter and data are lost together (no + * counter-ahead-of-state torn store). {@link commitTransaction} is the + * stronger contract: it runs a write barrier that fsyncs the whole batch's + * canonical footprint BEFORE advancing the generation counter, so a committed + * transact is durable on return. Callers that need per-write durability should + * use `transact()` (or `flush()` after a single-op); Model-B group-commit + * deliberately trades single-op fsync latency (a 3-5x write regression) for + * throughput. + * * The per-write generation is read by graph-index ops through the same * `generation()` watermark `transact()` uses — because this method, like * {@link commitTransaction}, holds the mutex across the counter bump AND diff --git a/src/db/types.ts b/src/db/types.ts index 4030533a..4f932e76 100644 --- a/src/db/types.ts +++ b/src/db/types.ts @@ -391,6 +391,24 @@ export interface GenerationStorage { /** Durability barrier: fsync the given object paths (no-op in memory). */ syncRawObjects(paths: string[]): Promise + /** + * OPTIONAL transaction durability barrier. `commitTransaction` calls + * {@link beginWriteBarrier} immediately before running the planned operations + * and {@link flushWriteBarrier} immediately after — BEFORE the generation + * counter and manifest are advanced. An adapter whose canonical writes are + * not synchronously durable (the filesystem adapter's tmp+rename lands in the + * page cache) MUST implement these so a transaction reported "committed" is + * durable before its generation stamp: otherwise a hard kill can leave the + * fsync'd counter ahead of the still-buffered entity bytes (phantom progress + * for any generation-based consumer). Adapters whose writes are already + * durable per-call (cloud object PUT) may leave these undefined — the store + * treats them as no-ops. `beginWriteBarrier` also resets any tracking left by + * an aborted prior transaction. + */ + beginWriteBarrier?(): void + /** @see beginWriteBarrier — fsync every canonical write since begin. */ + flushWriteBarrier?(): Promise + /** Read an entity's raw stored metadata+vector objects. */ readNounRaw(id: string): Promise<{ metadata: any | null; vector: any | null }> /** Restore an entity's raw stored objects (`null` part ⇒ delete that file). */ diff --git a/src/storage/adapters/fileSystemStorage.ts b/src/storage/adapters/fileSystemStorage.ts index 87e73e4a..b0bc4ab7 100644 --- a/src/storage/adapters/fileSystemStorage.ts +++ b/src/storage/adapters/fileSystemStorage.ts @@ -119,6 +119,18 @@ export class FileSystemStorage extends BaseStorage { private compressionEnabled: boolean = true // Enable gzip compression by default for 60-80% disk savings private compressionLevel: number = 6 // zlib compression level (1-9, default: 6 = balanced) + // Transaction durability barrier (see GenerationStorage.beginWriteBarrier). + // Non-null ONLY between beginWriteBarrier() and flushWriteBarrier(). Because + // canonical writes are tmp+rename (durable only in the page cache until an + // fsync), the generation store fsyncs everything a transaction wrote before + // it advances the generation counter. `writeBarrierPaths` collects the + // root-relative object paths written; `writeBarrierDeleteDirs` the parent + // dirs of deleted objects (an unlink is durable only once its directory is + // fsync'd). Both are null outside a transaction, so the tracking `add`s below + // are free on the single-op and non-transactional write paths. + private writeBarrierPaths: Set | null = null + private writeBarrierDeleteDirs: Set | null = null + /** * Initialize the storage adapter * @param rootDirectory The root directory for storage @@ -373,6 +385,12 @@ export class FileSystemStorage extends BaseStorage { throw error } } + + // Transaction durability barrier: record the write so the generation store + // can fsync it before advancing the counter. Reached only on a successful + // rename; the logical (non-`.gz`) path is stored — flushWriteBarrier's + // syncRawObjects resolves the compressed variant. + this.writeBarrierPaths?.add(pathStr) } /** @@ -467,6 +485,11 @@ export class FileSystemStorage extends BaseStorage { if (deletedCount === 0) { // File doesn't exist - this is fine } + + // Transaction durability barrier: an unlink is durable only once its parent + // directory is fsync'd. Record the dir (root-relative; '.' for a top-level + // object) so flushWriteBarrier can sync it before the counter advances. + this.writeBarrierDeleteDirs?.add(path.dirname(pathStr)) } /** @@ -630,6 +653,61 @@ export class FileSystemStorage extends BaseStorage { } } + /** + * Begin a transaction durability barrier: start recording every canonical + * object write and delete so {@link flushWriteBarrier} can fsync them before + * the generation counter advances. Resets unconditionally, discarding any + * tracking left by a transaction that aborted without flushing. + * + * @see GenerationStorage.beginWriteBarrier + */ + public beginWriteBarrier(): void { + this.writeBarrierPaths = new Set() + this.writeBarrierDeleteDirs = new Set() + } + + /** + * Flush the transaction durability barrier: fsync every canonical write since + * {@link beginWriteBarrier} (file contents AND the rename directory entries, + * via {@link syncRawObjects}), then fsync the parent directory of every + * canonical delete so the unlinks are durable too. Clears the tracking. After + * this resolves, the transaction's entire canonical footprint is on disk, so + * the generation counter/manifest can be advanced without risking a + * counter-ahead-of-state torn store on a hard kill. + * + * @see GenerationStorage.flushWriteBarrier + */ + public async flushWriteBarrier(): Promise { + const paths = this.writeBarrierPaths + const deleteDirs = this.writeBarrierDeleteDirs + this.writeBarrierPaths = null + this.writeBarrierDeleteDirs = null + + if (paths && paths.size > 0) { + // syncRawObjects fsyncs each file and its parent directory. + await this.syncRawObjects([...paths]) + } + + if (deleteDirs && deleteDirs.size > 0) { + for (const relDir of deleteDirs) { + const dirPath = path.join(this.rootDir, relDir) + let handle: any + try { + handle = await fs.promises.open(dirPath, 'r') + } catch { + continue // directory vanished or platform disallows opening dirs + } + try { + await handle.sync() + } catch { + // Some platforms/filesystems reject directory fsync — best effort. + } finally { + await handle.close() + } + } + } + } + /** * Append one line to `_system/tx-log.jsonl`. Plain `appendFile` — the * tx-log is the one append-in-place file in the store (and is byte-copied, diff --git a/tests/integration/transact-durability-barrier.test.ts b/tests/integration/transact-durability-barrier.test.ts new file mode 100644 index 00000000..9311ce67 --- /dev/null +++ b/tests/integration/transact-durability-barrier.test.ts @@ -0,0 +1,151 @@ +/** + * @module tests/integration/transact-durability-barrier + * @description Regression for a consumer-reported durability gap: a committed + * `transact()` reported success while its canonical entity writes were still + * only in the page cache (tmp+rename, not yet fsync'd), whereas the generation + * counter/manifest WERE fsync'd — so a hard kill could leave the counter ahead + * of the persisted entity bytes ("phantom progress" for any generation-based + * consumer resuming from the counter). + * + * Fix: `commitTransaction` opens a write barrier before running the planned + * operations and flushes it (fsync of every canonical write + the parent dir of + * every canonical delete) BEFORE advancing the counter and writing the manifest. + * These tests assert the ordering property directly by recording the storage's + * `syncRawObjects` calls: the entity writes are fsync'd in an earlier call than + * the manifest, and a precommit-rejected batch opens no barrier and advances + * nothing. + */ +import { describe, it, expect, beforeEach, afterEach } from 'vitest' +import * as fs from 'node:fs' +import * as os from 'node:os' +import * as path from 'node:path' +import { Brainy } from '../../src/brainy.js' +import { NounType, VerbType } from '../../src/types/graphTypes.js' +import { MemoryStorage } from '../../src/storage/adapters/memoryStorage.js' + +const MANIFEST_REL = '_system/manifest.json' + +describe('transact durability barrier — entity writes fsync before the counter advances', () => { + let dir: string + let brain: any + let syncCalls: string[][] + let beginCount: number + let flushCount: number + + beforeEach(async () => { + process.env.BRAINY_DETERMINISTIC_EMBEDDINGS = 'true' + dir = fs.mkdtempSync(path.join(os.tmpdir(), 'brainy-durability-')) + brain = new Brainy({ + requireSubtype: false, + storage: { type: 'filesystem', path: dir }, + dimensions: 384, + silent: true + }) + await brain.init() + + // Instrument the real filesystem storage: record every fsync batch in order, + // and count barrier open/flush, delegating to the originals. + syncCalls = [] + beginCount = 0 + flushCount = 0 + const storage = brain['storage'] + const origSync = storage.syncRawObjects.bind(storage) + storage.syncRawObjects = async (paths: string[]) => { + syncCalls.push([...paths]) + return origSync(paths) + } + const origBegin = storage.beginWriteBarrier.bind(storage) + storage.beginWriteBarrier = () => { + beginCount++ + return origBegin() + } + const origFlush = storage.flushWriteBarrier.bind(storage) + storage.flushWriteBarrier = async () => { + flushCount++ + return origFlush() + } + }) + + afterEach(async () => { + await brain.close() + fs.rmSync(dir, { recursive: true, force: true }) + }) + + /** Index of the first sync call whose paths satisfy `pred` (or -1). */ + const findSync = (pred: (p: string) => boolean): number => + syncCalls.findIndex((paths) => paths.some(pred)) + + it('a committed transact fsyncs the entity write BEFORE the manifest', async () => { + const id = '00000000-0000-4000-8000-00000000ab01' + await brain.transact([{ op: 'add', id, data: 'durable entity', type: NounType.Thing }]) + + // The barrier opened and flushed exactly once. + expect(beginCount).toBe(1) + expect(flushCount).toBe(1) + + // The entity's canonical metadata write was fsync'd… + const entitySyncIdx = findSync((p) => p.includes(`/${id}/`) && p.includes('entities/nouns')) + expect(entitySyncIdx).toBeGreaterThanOrEqual(0) + // …and the manifest (the commit point) was fsync'd in a LATER call. + const manifestSyncIdx = findSync((p) => p === MANIFEST_REL) + expect(manifestSyncIdx).toBeGreaterThanOrEqual(0) + expect(entitySyncIdx).toBeLessThan(manifestSyncIdx) + }) + + it('a multi-op transact (add + relate) fsyncs both endpoints and the edge before the manifest', async () => { + const a = '00000000-0000-4000-8000-00000000ab02' + const b = '00000000-0000-4000-8000-00000000ab03' + await brain.transact([ + { op: 'add', id: a, data: 'A', type: NounType.Thing }, + { op: 'add', id: b, data: 'B', type: NounType.Thing }, + { op: 'relate', from: a, to: b, type: VerbType.Contains } + ]) + + expect(flushCount).toBe(1) + const aIdx = findSync((p) => p.includes(`/${a}/`)) + const bIdx = findSync((p) => p.includes(`/${b}/`)) + const manifestIdx = findSync((p) => p === MANIFEST_REL) + expect(aIdx).toBeGreaterThanOrEqual(0) + expect(bIdx).toBeGreaterThanOrEqual(0) + expect(manifestIdx).toBeGreaterThanOrEqual(0) + expect(Math.max(aIdx, bIdx)).toBeLessThan(manifestIdx) + + // Sanity: the data is actually there and traverses. + expect((await brain.related(a)).map((r: any) => r.to)).toContain(b) + }) + + it('a precommit-rejected batch opens no barrier and advances nothing', async () => { + const anchor = await brain.add({ id: '00000000-0000-4000-8000-00000000ab04', data: 'anchor', type: NounType.Thing }) + const genBefore = brain.generationStore.generation() + syncCalls.length = 0 + beginCount = 0 + flushCount = 0 + + const never = '00000000-0000-4000-8000-00000000ab05' + await expect( + brain.transact([ + { op: 'add', id: never, data: 'never', type: NounType.Thing }, + { op: 'update', id: anchor, metadata: { poke: 1 }, ifRev: 999 } + ]) + ).rejects.toMatchObject({ name: 'RevisionConflictError' }) + + // Precommit throws before the barrier opens (it lives just before execute): + // the barrier never opens, nothing is flushed, and the generation counter is + // unchanged. (A manifest sync CAN appear here from transact() flushing the + // anchor's previously-buffered single-op — that is the anchor's deferred + // durability, not this rejected batch committing; beginCount is the clean + // signal that this batch's commit path never reached execute.) + expect(beginCount).toBe(0) + expect(flushCount).toBe(0) + expect(brain.generationStore.generation()).toBe(genBefore) + expect(await brain.get(never)).toBeNull() + }) + + it('MemoryStorage does not implement the barrier (optional-chaining no-op)', () => { + const mem = new MemoryStorage() as any + // In-memory has no durability seam; the generation store treats the absent + // barrier methods as no-ops via optional chaining. + expect(mem.beginWriteBarrier).toBeUndefined() + expect(mem.flushWriteBarrier).toBeUndefined() + }) +})