diff --git a/integrationTests/fixtures/transaction-context-reads/resources.js b/integrationTests/fixtures/transaction-context-reads/resources.js index 5bd19a6c31..9fcd7a2aa6 100644 --- a/integrationTests/fixtures/transaction-context-reads/resources.js +++ b/integrationTests/fixtures/transaction-context-reads/resources.js @@ -97,3 +97,52 @@ export class DashWriteThenSearch extends Resource { return { variant: 'write-then-search', companyId, count: snapshots.length, snapshots }; } } + +// POST-COMMIT ATOMICITY — commit the per-request transaction mid-handler, then write two records +// and throw. Those writes belong to the request transaction's pending final commit, so the failure +// must leave neither of them behind. +export class DashCommitWriteThrow extends Resource { + static loadAsInstance = false; + async get(query) { + const suffix = paramId(query) ?? 'x'; + await tables.Company.get('c1'); + await transaction.commit(this); + await tables.Company.put({ id: `atomic-company-${suffix}`, name: 'should not survive' }); + await tables.ScoreSnapshot.put({ id: `atomic-snap-${suffix}`, companyId: 'atomic-co', score: 1 }); + throw new Error('deliberate failure after the mid-handler commit'); + } +} + +// The same shape that succeeds: both post-commit writes must be durable once the request completes. +export class DashCommitWriteOk extends Resource { + static loadAsInstance = false; + async get(query) { + const suffix = paramId(query) ?? 'x'; + await tables.Company.get('c1'); + await transaction.commit(this); + await tables.Company.put({ id: `ok-company-${suffix}`, name: 'kept' }); + await tables.ScoreSnapshot.put({ id: `ok-snap-${suffix}`, companyId: 'atomic-co', score: 2 }); + return { variant: 'commit-write-ok', suffix }; + } +} + +// CLOSED-SLOT READ GUARD — the original point of this fixture, preserved now that an ordinary +// mid-handler commit rotates the scope to a fresh open generation instead of leaving it closed. An +// undrained iterator holds the native handle, so the commit cannot rotate and the slot stays genuinely +// CLOSED; the search that follows must still read the latest committed state rather than empty. +export class DashUndrainedThenSearch extends Resource { + static loadAsInstance = false; + async get(query) { + const companyId = paramId(query); + const held = tables.ScoreSnapshot.search({ + conditions: [{ attribute: 'companyId', comparator: 'equals', value: companyId }], + }); + const iterator = held[Symbol.asyncIterator](); + await iterator.next(); // hold the handle open, do not drain + const ctx = this.getContext(); + await transaction.commit(this); + const txnOpenAfter = ctx?.transaction?.open; + const snapshots = await searchSnapshots(companyId); + return { variant: 'undrained-then-search', companyId, txnOpenAfter, count: snapshots.length, snapshots }; + } +} diff --git a/integrationTests/resources/transaction-context-reads.test.ts b/integrationTests/resources/transaction-context-reads.test.ts index 6989323d61..36e0aa37f9 100644 --- a/integrationTests/resources/transaction-context-reads.test.ts +++ b/integrationTests/resources/transaction-context-reads.test.ts @@ -104,13 +104,44 @@ suite('Transaction context: closed txn in ALS still reads latest', { skip: skipS strictEqual(body.count, SNAP_IDS.length); }); - test('commit-then-search (txn closed in ALS) still returns all rows', async () => { + test('commit-then-search still returns all rows, on the rotated generation', async () => { const body = await dashCount('/DashCommitThenSearch/'); - // Guards the misdiagnosis: the request txn is genuinely closed before the search - // (open: 1 -> 0), yet the closed slot reads latest committed state, not empty. + // The explicit commit closes the generation it committed and the scope rotates to a fresh + // OPEN, snapshot-free one, because the request scope still owes a final commit. Either way the + // search must read the latest committed state — which is what the original misdiagnosis + // (empty results from a closed slot) claimed it would not. strictEqual(body.txnOpenBefore, 1, 'txn was open before the explicit commit'); - strictEqual(body.txnOpenAfter, 0, 'txn is closed in ALS before the search'); - strictEqual(body.count, SNAP_IDS.length, 'closed-txn-in-ALS must still see all snapshots'); + strictEqual(body.txnOpenAfter, 1, 'the scope rotates to a fresh open generation after its own commit'); + strictEqual(body.count, SNAP_IDS.length, 'the rotated generation must still see all snapshots'); + }); + + test('a genuinely closed slot still returns all rows (original guard, via an undrained iterator)', async () => { + // The commit-then-search case above now rotates to a fresh OPEN generation, so it no longer + // exercises a CLOSED slot. An undrained iterator holds the handle, which blocks the rotation and + // keeps the slot closed — the shape this file was written to pin. + const body = await dashCount('/DashUndrainedThenSearch/'); + strictEqual(body.txnOpenAfter, 0, 'a retained handle must block the rotation, leaving the slot closed'); + strictEqual(body.count, SNAP_IDS.length, 'a closed slot must still read latest committed state, not empty'); + }); + + test('writes made after a mid-handler commit roll back when the request fails', async () => { + const r = await fetch(`${httpURL}/DashCommitWriteThrow/?company=rollback`, { + headers: { Authorization: auth }, + }); + ok(r.status >= 500, `expected the handler failure to surface, got ${r.status}`); + for (const path of ['/Company/atomic-company-rollback', '/ScoreSnapshot/atomic-snap-rollback']) { + const probe = await fetch(`${httpURL}${path}`, { headers: { Authorization: auth } }); + strictEqual(probe.status, 404, `${path} must not exist: it was written after the mid-handler commit`); + } + }); + + test('writes made after a mid-handler commit are durable when the request succeeds', async () => { + const r = await fetch(`${httpURL}/DashCommitWriteOk/?company=keep`, { headers: { Authorization: auth } }); + ok(r.status < 300, `DashCommitWriteOk expected 2xx, got ${r.status}`); + for (const path of ['/Company/ok-company-keep', '/ScoreSnapshot/ok-snap-keep']) { + const probe = await fetch(`${httpURL}${path}`, { headers: { Authorization: auth } }); + ok(probe.status < 300, `${path} must be durable once the request completes, got ${probe.status}`); + } }); test('lazily-returned search iterated during post-commit serialization returns all rows', async () => { diff --git a/resources/DatabaseTransaction.ts b/resources/DatabaseTransaction.ts index ac8a68e770..7c7d831e8a 100644 --- a/resources/DatabaseTransaction.ts +++ b/resources/DatabaseTransaction.ts @@ -16,6 +16,8 @@ import type { Entry } from './RecordEncoder.ts'; import { toBufferKey } from 'ordered-binary'; const trackedTxns = new Set(); +// Read options for a rotated generation's native transactions; shared because they never vary. +const SNAPSHOT_FREE = Object.freeze({ disableSnapshot: true }); // Logical transactions the monitor supervises for their WRITES, kept apart from trackedTxns because the // two have different units and different consumers: trackedTxns is per-link, bounds a read snapshot, and // is what the read-queue-depth metric counts, while this holds one entry per logical transaction — the @@ -325,6 +327,15 @@ type RocksTransactionWithRetry = RocksTransaction & { isRetry?: boolean }; export class DatabaseTransaction implements Transaction { #context: Context; + // Whether a resources/transaction.ts scope owns this instance — i.e. a final commit or abort is + // guaranteed to follow. Only such a transaction may be rotated to a new generation by a mid-scope + // commit (see rotateAfterMidScopeCommit); anything else must commit each later write immediately, + // because nothing would commit staged ones. Settable only at construction, so it cannot be turned on + // for a transaction that is already attached to a context and running. + #scopeOwned: boolean; + constructor(options?: { scopeOwned?: boolean }) { + this.#scopeOwned = options?.scopeOwned === true; + } writes: TransactionWrite[] = []; // the set of writes to commit if the conditions are met // the last staged write per store and key, used to chain repeat writes to the same key (linkWrite) declare writesByKey?: Map>; @@ -383,6 +394,11 @@ export class DatabaseTransaction implements Transaction { // Set once the retained read handle's write intents have been released (see commit()'s // outstanding-iterators branch), so a retry round cannot re-fire the release. declare writesAbandoned?: boolean; + // Set once a mid-scope commit has rotated this instance to a new generation: every native + // transaction it opens from then on reads WITHOUT a snapshot. Committing mid-scope is how a handler + // asks to stop reading a pinned snapshot, so re-pinning one for the rest of the scope would take + // back what it asked for. + snapshotFree = false; getReadTxn(disableSnapshot?: boolean): ReadTransaction { this.readTxnRefCount = (this.readTxnRefCount || 0) + 1; @@ -409,7 +425,12 @@ export class DatabaseTransaction implements Transaction { // snapshot that blocks compaction. Only applied when creating the transaction fresh; an // already-open transaction keeps whatever snapshot mode it was created with. // `coordinatedRetry` signals IsBusy write conflicts as RETRY_NOW rather than ERR_BUSY. - this.attachOwnedTransaction(new RocksTransaction(this.db.store, { coordinatedRetry: true, disableSnapshot })); + this.attachOwnedTransaction( + new RocksTransaction(this.db.store, { + coordinatedRetry: true, + disableSnapshot: disableSnapshot || this.snapshotFree, + }) + ); if (this.timestamp) { this.transaction.setTimestamp(this.timestamp); @@ -716,7 +737,10 @@ export class DatabaseTransaction implements Transaction { if (!transaction && this.open === TRANSACTION_STATE.OPEN) transaction = this.transaction; let immediateCommit = false; if (!transaction) { - transaction = new RocksTransaction(operation.store.store as RocksStore); + transaction = new RocksTransaction( + operation.store.store as RocksStore, + this.snapshotFree ? SNAPSHOT_FREE : undefined + ); if (operation.store.rootStore !== this.db.rootStore) { harperLogger.warn?.('Created new transaction in save, but the store does match existing store', transaction.id); } @@ -997,15 +1021,27 @@ export class DatabaseTransaction implements Transaction { // now reset transactions tracking; this transaction be reused and committed again this.retries = 0; // reset per-native-transaction retry counter so a reused DatabaseTransaction's next batch starts fresh this.clearWrites(); + if (options.doneWriting) this.endScopeOwnership(); this.releaseContext(!!options.doneWriting); - this.next = null; let txnTime = this.timestamp; this.timestamp = 0; // reset the timestamp as well - return Promise.all(completions).then(() => { - return { - txnTime, - }; - }); + return Promise.all(completions).then( + () => { + // Only once the chained store's commit has settled, as on the synchronous path: a + // partially failed mid-scope commit must not leave the scope resumable. + this.completeMidScopeCommit(options); + return { + txnTime, + }; + }, + (error) => { + // As on the synchronous path: a completion that failed (a chained store's commit, + // a replication confirmation) leaves this commit partly landed, so ownership goes + // with it rather than letting a later commit rotate on top. + this.endScopeOwnership(); + throw error; + } + ); }, (error) => { // Coordinated transactions surface conflicts as RETRY_NOW (handled in the @@ -1083,6 +1119,9 @@ export class DatabaseTransaction implements Transaction { // back-reference here too, or transaction.ts's onComplete() (which has no // rejection handler of its own) would leave a long-lived context pinning this // CLOSED wrapper forever. + // A failed commit must never be followed by a resumed segment: this generation is + // finished and its durability is unknown, so ownership goes with it. + this.endScopeOwnership(); this.releaseContext(!!options.doneWriting); throw error; } @@ -1094,6 +1133,7 @@ export class DatabaseTransaction implements Transaction { cleanupUnusedBlobs(write.savedBlobs, collectRetainedFileIds(write.store.getEntry(write.key)?.value)); } this.clearWrites(); + if (options.doneWriting) this.endScopeOwnership(); this.releaseContext(!!options.doneWriting); const txnResolution: CommitResolution = { txnTime: this.timestamp, @@ -1102,16 +1142,39 @@ export class DatabaseTransaction implements Transaction { // now run any other transactions options.timestamp = this.timestamp; // as above: the next store must not inherit this store's explicit native transaction - const nextResolution = this.next?.commit( - options.transaction ? { ...options, transaction: undefined } : options - ); + let nextResolution; + try { + nextResolution = this.next?.commit(options.transaction ? { ...options, transaction: undefined } : options); + } catch (error) { + // A synchronous throw reaches neither rejection handler below, and the head has already + // committed — surrender ownership here too, or the scope stays resumable on top of a + // half-landed multi-store commit. + this.endScopeOwnership(); + throw error; + } if ((nextResolution as any)?.then) - return (nextResolution as any)?.then((nextResolution) => ({ - txnTime: this.timestamp, - next: nextResolution, - })); + return (nextResolution as any)?.then( + (nextResolution) => { + // Only once the chained store's own commit has SETTLED: rotating first would leave the + // scope resumable after a partially failed mid-scope commit. + this.completeMidScopeCommit(options); + return { + txnTime: this.timestamp, + next: nextResolution, + }; + }, + (error) => { + // A chained store's commit failed, so this multi-store commit half-landed. Surrender + // ownership as the head's own failure branch does: a handler that catches this and + // commits again must not rotate on top of it, and must not have the failed link + // dropped from the chain before its abort can clean up its blobs. + this.endScopeOwnership(); + throw error; + } + ); txnResolution.next = nextResolution as any; } + this.completeMidScopeCommit(options); return txnResolution; }, (error) => { @@ -1120,6 +1183,44 @@ export class DatabaseTransaction implements Transaction { } ); } + /** + * A successful commit that is NOT the scope's final one leaves the scope still running and still + * responsible for a commit. Rotate to a fresh OPEN generation so the rest of the scope's writes + * stage into it and are committed — or rolled back — as one unit, instead of each committing itself + * the moment it is made. Every dispatch path keeps its plain `open === OPEN` check; CLOSED never + * gains a second meaning. + * + * Deliberately not rotated when: the scope is finished (`doneWriting`), nothing owns this instance, + * a timeout poisoned it, or a commit failed — a failed or uncertain commit must never be followed by + * a resumed segment that can commit on its own. Nor when read iterators still hold the native + * handle: that handle belongs to them until they drain, so there is nothing to rotate into and those + * writes keep today's immediate-commit path. + */ + /** + * Finish a commit: the chain goes with it, then the scope may rotate. A link left attached and CLOSED + * would be reused by txnForContext for the next write to that database and commit itself, surviving a + * rollback of the rotated head — the cross-store leftover this rotation exists to prevent. Every + * commit path must run this, and none may do one half without the other. + */ + /** Both scope flags leave together, so no exit can clear one and keep the other. */ + private endScopeOwnership(): void { + this.#scopeOwned = false; + this.snapshotFree = false; + } + + private completeMidScopeCommit(options: CommitOptions): void { + this.next = null; + this.rotateAfterMidScopeCommit(options); + } + + /** See completeMidScopeCommit, which is the only caller and carries the reasoning. */ + private rotateAfterMidScopeCommit(options: CommitOptions): void { + if (options.doneWriting || this.timedOut || this.transaction || !this.#scopeOwned) return; + this.open = TRANSACTION_STATE.OPEN; + this.snapshotFree = true; + this.writesAbandoned = false; + } + abort(): void { while (this.readTxnsUsed > 0) this.doneReadTxn(); // release the read snapshot when we abort, we assume we don't need it // Defensively release any native handle whose reference bookkeeping was already consumed. @@ -1132,6 +1233,7 @@ export class DatabaseTransaction implements Transaction { cleanupUnusedBlobs(write.savedBlobs, collectRetainedFileIds(write.store.getEntry(write.key)?.value)); } } finally { + this.endScopeOwnership(); // the scope is over; nothing may rotate this instance again this.clearWrites(); // A timeout-poisoned abort (abortDueToTimeout()) is the one abort that is NOT "reuse-free": // Resource.ts's dispatcher deliberately keeps joining a `timedOut` transaction (instead of diff --git a/resources/ResourceInterface.ts b/resources/ResourceInterface.ts index 057f6274d4..a97cc8b746 100644 --- a/resources/ResourceInterface.ts +++ b/resources/ResourceInterface.ts @@ -75,6 +75,21 @@ export interface Context { * so code that commits mid-handler can keep using its context. LMDBTransaction does not release, * so there the completed transaction itself stays in the slot — also safe to call, but retained. * `null` was the previous released marker and is still accepted defensively. + * + * A transaction its own handler commits mid-scope is rotated to a fresh open generation, so the rest + * of that scope's writes are committed — or rolled back — with the scope's final commit rather than + * each committing itself immediately. Two cases keep the older per-write behavior: a commit made + * while a read iterator still holds the transaction's handle (that handle is the iterator's until it + * drains), and a commit that failed. LMDB has always behaved this way; the RocksDB path now matches. + * + * The engines still differ on reads. A rotated RocksDB generation is snapshot-free, so the rest of + * the scope keeps seeing other writers' committed data — which is what committing mid-scope asks for. + * LMDB cannot open a snapshot-free read transaction, so there the scope keeps its snapshot. + * + * Because the writes after a mid-scope commit stage rather than commit one at a time, a single + * mid-scope commit does not bound how much a long handler holds in memory. A handler streaming a + * large volume should keep committing (a checkpoint every N records), which commits each batch and + * rotates again. */ transaction?: DatabaseTransaction | null; /** If the operation that will be performed with this context should check user authorization */ diff --git a/resources/Table.ts b/resources/Table.ts index dc4a2836e4..0e7c5f11b4 100644 --- a/resources/Table.ts +++ b/resources/Table.ts @@ -5632,6 +5632,9 @@ export function makeTable(options) { // commit is tracked with its own identity (DatabaseTransaction.ts's trackOutstandingCommit), // so a wedged second-store commit is named just as precisely as a wedged first one. transaction.next.startedFrom = transaction.startedFrom; + // A second database joined after a mid-scope commit belongs to the same snapshot-free + // generation as the head, or its reads would re-pin what the commit just unpinned. + transaction.next.snapshotFree = transaction.snapshotFree; if (transaction.open === TRANSACTION_STATE.CLOSED) { // if the current transaction is already closed, we need to retain that state on new databases we work with transaction.next.open = TRANSACTION_STATE.CLOSED; diff --git a/resources/transaction.ts b/resources/transaction.ts index b2dd2abcdf..8ad2e2600d 100644 --- a/resources/transaction.ts +++ b/resources/transaction.ts @@ -45,7 +45,9 @@ export function transaction( return callback(context.transaction); // nothing to be done, already in open transaction } - const transaction = new DatabaseTransaction(); + // scopeOwned: onComplete/onError below guarantee this instance a final commit or an abort, which is + // what lets a mid-scope commit rotate it instead of leaving later writes to commit themselves. + const transaction = new DatabaseTransaction({ scopeOwned: true }); context.transaction = transaction; if (context.timestamp) transaction.timestamp = context.timestamp; if (context.replicatedConfirmation) transaction.replicatedConfirmation = context.replicatedConfirmation; diff --git a/unitTests/resources/resumeAfterMidScopeCommit.test.js b/unitTests/resources/resumeAfterMidScopeCommit.test.js new file mode 100644 index 0000000000..f6173c6c60 --- /dev/null +++ b/unitTests/resources/resumeAfterMidScopeCommit.test.js @@ -0,0 +1,353 @@ +const assert = require('assert'); +const { setupTestDBPath } = require('../testUtils'); +const { table } = require('#src/resources/databases'); +const { setMainIsWorker } = require('#js/server/threads/manageThreads'); +const { transaction, contextStorage } = require('#src/resources/transaction'); +const { DatabaseTransaction, TRANSACTION_STATE } = require('#src/resources/DatabaseTransaction'); +const serverUtilities = require('#src/server/serverHelpers/serverUtilities'); + +// A handler that commits its own transaction mid-scope is a documented pattern, and its enclosing +// transaction() scope still has a final commit to make. The writes it makes after that commit +// therefore belong to that pending commit: they stage, and they roll back with it. Before this, each +// one committed immediately and individually, so a handler that failed halfway left the earlier half +// durable — which is how a failed cluster delete could mark a cluster TERMINATED with its instances +// still RUNNING. +// Runs on BOTH engines: LMDBTransaction already left a non-final commit OPEN (LMDBTransaction.ts's +// commit), so post-commit writes have always staged and rolled back with the scope there. This change +// brings the RocksDB path to that behavior. Four cases below are RocksDB-only, each for a stated +// engine reason — read-after-write, snapshot-free reads, and the two that reach into rocksdb-js. +const isLMDB = process.env.HARPER_STORAGE_ENGINE === 'lmdb'; +const rocksOnly = isLMDB ? it.skip : it; + +describe('Writes after a mid-scope commit rejoin the scope', () => { + let A, B, Other; + before(async function () { + setupTestDBPath(); + setMainIsWorker(true); + A = table({ table: 'ResumeA', attributes: [{ name: 'id', isPrimaryKey: true }, { name: 'v' }] }); + B = table({ table: 'ResumeB', attributes: [{ name: 'id', isPrimaryKey: true }, { name: 'v' }] }); + // A genuinely separate DATABASE, not just a second table: only this reaches txnForContext's + // `transaction.next` chain, which is where a link created after the mid-scope commit could + // otherwise be left committing on its own. + Other = table({ + database: 'resume_other', + table: 'ResumeOther', + attributes: [{ name: 'id', isPrimaryKey: true }, { name: 'v' }], + }); + }); + + async function scanValues(context) { + const out = []; + for await (const record of A.search([{ attribute: 'v', comparator: 'greater_than', value: -1 }], context)) + out.push(record.v); + return out.sort((x, y) => x - y); + } + + it('rolls post-commit writes back with the scope, across two stores', async function () { + const context = {}; + await assert.rejects( + transaction(context, async () => { + await A.put('pre', { v: 1 }, context); + await B.put('pre', { v: 1 }, context); + await context.transaction.commit(); + await A.put('post', { v: 2 }, context); + await B.put('post', { v: 2 }, context); + throw new Error('forced failure after the mid-scope commit'); + }), + /forced failure/ + ); + assert.ok(await A.get('pre'), 'what the explicit commit committed must stay committed'); + assert.ok(await B.get('pre')); + assert.equal(await A.get('post'), undefined, 'a write made after the commit must roll back with the scope'); + assert.equal(await B.get('post'), undefined, 'including on a chained second store'); + }); + + it('commits post-commit writes when the scope completes', async function () { + const context = {}; + await transaction(context, async () => { + await A.put('ok1', { v: 1 }, context); + await context.transaction.commit(); + await A.put('ok2', { v: 2 }, context); + await B.put('ok2', { v: 2 }, context); + assert.equal(context.transaction.open, TRANSACTION_STATE.OPEN, 'the write must have reopened the scope'); + }); + for (const [t, id] of [ + [A, 'ok1'], + [A, 'ok2'], + [B, 'ok2'], + ]) + assert.ok(await t.get(id), `${id} must be durable once the scope commits`); + }); + + it('rolls back a post-commit write to a second DATABASE, through the chained link', async function () { + const context = {}; + await assert.rejects( + transaction(context, async () => { + await A.put('chain-pre', { v: 1 }, context); + await context.transaction.commit(); + await A.put('chain-post', { v: 2 }, context); + await Other.put('chain-post', { v: 2 }, context); // creates transaction.next after the commit + assert.ok(context.transaction.next, 'premise: a chained link must have been created'); + throw new Error('forced failure'); + }), + /forced failure/ + ); + assert.ok(await A.get('chain-pre')); + assert.equal(await A.get('chain-post'), undefined); + assert.equal(await Other.get('chain-post'), undefined, 'the chained database must roll back too'); + }); + + it('commits a post-commit write to a second DATABASE when the scope completes', async function () { + const context = {}; + await transaction(context, async () => { + await context.transaction.commit(); + await A.put('chain-ok', { v: 1 }, context); + await Other.put('chain-ok', { v: 1 }, context); + }); + assert.ok(await A.get('chain-ok')); + assert.ok(await Other.get('chain-ok'), 'the chained database must be durable'); + }); + + // A commit that FAILED leaves this generation finished and its durability unknown. Rotating after it + // would let the rest of the scope stage into a generation whose predecessor may not have landed. + // RocksDB only: forces a terminal failure through rocksdb-js's Transaction. + rocksOnly('does not rotate after a failed mid-scope commit', async function () { + const { Transaction } = require('@harperfast/rocksdb-js'); + const originalCommit = Transaction.prototype.commit; + const targetDb = A.primaryStore.store.db; + let forcedFailures = 0; + const context = {}; + try { + await transaction(context, async () => { + await A.put('poison-seed', { v: 1 }, context); + Transaction.prototype.commit = function (...args) { + if (this.store?.db !== targetDb) return originalCommit.apply(this, args); + forcedFailures++; + Transaction.prototype.commit = originalCommit; // one forced failure only + return Promise.reject(Object.assign(new Error('forced terminal failure'), { code: 'ERR_CORRUPTION' })); + }; + await assert.rejects(context.transaction.commit(), /forced terminal failure/); + assert.notEqual( + context.transaction?.open, + TRANSACTION_STATE.OPEN, + 'a failed commit must not leave an open generation behind' + ); + await A.put('after-failed-commit', { v: 2 }, context); + }); + } finally { + Transaction.prototype.commit = originalCommit; + } + assert.ok(forcedFailures > 0, 'premise: the explicit commit must actually have failed'); + assert.equal(await A.get('poison-seed'), undefined, 'the failed commit committed nothing'); + assert.ok( + await A.get('after-failed-commit'), + 'a write after a failed commit must commit itself rather than stage into a generation nobody will commit' + ); + }); + + // RocksDB only: LMDB does not guarantee read-after-write within a transaction. + // A CHAINED store's commit can fail while the head's succeeded, leaving the multi-store commit half + // landed. A handler that catches that (the shape people write, because a mid-scope commit can + // conflict) must not get a rotated scope on its next commit: the failed link would be dropped from + // the chain before its abort could clean up, and the rest of the scope would stage on a partial commit. + // A rejected promise and a synchronous throw leave the chained commit through different code, so both + // have to surrender ownership. + for (const mode of ['rejects', 'throws synchronously']) { + rocksOnly(`surrenders ownership when a chained database’s commit ${mode}`, async function () { + const { Transaction } = require('@harperfast/rocksdb-js'); + const originalCommit = Transaction.prototype.commit; + const chainedDb = Other.primaryStore.store.db; + const tag = mode === 'rejects' ? 'reject' : 'throw'; + let forcedFailures = 0; + const context = {}; + try { + await transaction(context, async () => { + await A.put(`chain-${tag}-head`, { v: 1 }, context); + await Other.put(`chain-${tag}-link`, { v: 1 }, context); + Transaction.prototype.commit = function (...args) { + if (this.store?.db !== chainedDb) return originalCommit.apply(this, args); + forcedFailures++; + Transaction.prototype.commit = originalCommit; // one forced failure only + const error = Object.assign(new Error(`forced chained ${tag}`), { code: 'ERR_CORRUPTION' }); + if (mode === 'rejects') return Promise.reject(error); + throw error; + }; + await assert.rejects( + (async () => context.transaction.commit())(), + new RegExp(`forced chained ${tag}`), + 'the failure must reach the caller either way' + ); + assert.notEqual( + context.transaction?.open, + TRANSACTION_STATE.OPEN, + 'a half-landed multi-store commit must not leave the scope rotated' + ); + await A.put(`chain-${tag}-after`, { v: 2 }, context); + }); + } finally { + Transaction.prototype.commit = originalCommit; + } + assert.ok(forcedFailures > 0, 'premise: the chained commit must actually have failed'); + assert.ok( + await A.get(`chain-${tag}-after`), + 'a write after a half-landed commit must commit itself rather than stage into a rotated generation' + ); + }); + } + + rocksOnly('reads its own post-commit writes', async function () { + const context = {}; + await transaction(context, async () => { + await context.transaction.commit(); + await A.put('own', { v: 7 }, context); + assert.equal((await A.get('own', context))?.v, 7); + }); + }); + + // Committing mid-handler is documented as the way to stop reading a pinned snapshot, so every read + // shape has to keep seeing other writers for the rest of the scope — including a search() iterator, + // which holds its own handle. This is what the resumed transaction's snapshot-free mode buys. + // RocksDB only: only RocksDB can open a snapshot-free read transaction; LMDB keeps its snapshot. + rocksOnly('keeps reads seeing other writers for the rest of the scope', async function () { + await A.put('shared', { v: 1 }, {}); + const context = {}; + const pointReads = []; + await transaction(context, async () => { + await A.get('shared', context); + await context.transaction.commit(); + for (let v = 2; v <= 4; v++) { + await A.put('shared', { v }, {}); // another writer: its OWN context, not the ambient one + pointReads.push((await A.get('shared', context)).v); + pointReads.push((await transaction(() => A.get('shared'))).v); // the migration-guide form + } + await A.put('own-write', { v: 50 }, context); // resume the scope, then keep reading + const before = await scanValues(context); + await A.put('scan-visible', { v: 99 }, {}); // another writer again + const after = await scanValues(context); + assert.ok( + after.includes(99) && !before.includes(99), + 'a search() after the commit must still pick up another writer’s row' + ); + }); + assert.deepEqual(pointReads, [2, 2, 3, 3, 4, 4], 'every read after the commit must see the latest value'); + }); + + // The gate is ownership, not state — and the case it exists for is a DatabaseTransaction sitting OPEN + // in a context that no transaction() scope owns: crash-recovery replay (replayLogs.ts) commits in a + // loop at timestamp boundaries, and Table.ts builds one directly. If the gate regressed to "anything + // rotates", replay would stage its post-commit writes into a generation its loop never commits. The + // released-slot test below cannot see that: it goes through the placeholder, so txnForContext builds + // an ImmediateTransaction and never consults ownership at all. + rocksOnly('does not rotate a transaction no scope owns', async function () { + const context = { transaction: new DatabaseTransaction() }; + await A.put('unowned-1', { v: 1 }, context); + assert.ok(context.transaction, 'premise: the unowned transaction is still the context’s'); + await context.transaction.commit(); + assert.notEqual( + context.transaction.open, + TRANSACTION_STATE.OPEN, + 'an unowned transaction must stay closed after its own commit — nothing guarantees another commit' + ); + await A.put('unowned-2', { v: 2 }, context); + assert.ok(await A.get('unowned-1')); + assert.ok( + await A.get('unowned-2'), + 'a write after an unowned transaction’s commit must commit itself, not stage into a generation nobody commits' + ); + }); + + it('does not resume a transaction with no owning scope', async function () { + const context = {}; + await transaction(context, async () => { + await A.put('scoped', { v: 1 }, context); + }); + // The scope is over. These writes have no pending commit and must be durable on their own. + await A.put('after-scope-1', { v: 1 }, context); + await A.put('after-scope-2', { v: 2 }, context); + assert.ok(await A.get('after-scope-1'), 'a write after the scope ended must commit itself'); + assert.ok(await A.get('after-scope-2')); + }); + + // A read between the commit and the writes must not quietly cost the guarantee. Committing + // mid-handler in order to re-read is the documented reason to commit at all, so if an intervening + // read dropped the scope's transaction from the context the guarantee would be absent in exactly + // the shape people use. + it('keeps post-commit writes atomic even when a read comes between', async function () { + const context = {}; + await assert.rejects( + transaction(context, async () => { + await A.put('seed', { v: 0 }, context); + await context.transaction.commit(); + await A.get('seed', context); + await scanValues(context); + await A.put('after-read', { v: 1 }, context); + throw new Error('forced failure'); + }), + /forced failure/ + ); + assert.equal(await A.get('after-read'), undefined, 'the write after an intervening read must roll back too'); + }); + + // The one shape the guarantee does NOT cover, pinned so it cannot change silently: with an iterator + // still holding the native handle at commit time, that handle belongs to the iterator until it + // drains, so there is nothing to rotate into and the writes keep committing individually. This is + // the checkpoint-over-a-search loop, and it is stated on Context.transaction for that reason. + // RocksDB only: the retained-handle branch is the RocksDB commit path. + rocksOnly('does not rotate while an iterator still holds the handle, and says so', async function () { + await A.put('iter-seed', { v: 1 }, {}); + const context = {}; + let openState; + await transaction(context, async () => { + const results = await A.search([{ attribute: 'v', comparator: 'greater_than', value: -1 }], context); + const iterator = results[Symbol.asyncIterator](); + await iterator.next(); // hold the handle open + await A.put('iter-write', { v: 2 }, context); + await context.transaction.commit(); + openState = context.transaction.open; + await A.put('iter-post', { v: 3 }, context); + while (!(await iterator.next()).done); + }); + assert.notEqual(openState, TRANSACTION_STATE.OPEN, 'a retained handle must block the rotation'); + assert.ok( + await A.get('iter-post'), + 'and the write after that commit therefore commits on its own — the documented carve-out' + ); + }); + + it('still lets a checkpoint loop commit repeatedly inside one scope', async function () { + const context = {}; + await transaction(context, async () => { + for (let i = 0; i < 4; i++) { + await A.put(`ckpt-${i}`, { v: i }, context); + await transaction.commit(context); + } + }); + for (let i = 0; i < 4; i++) assert.ok(await A.get(`ckpt-${i}`), `ckpt-${i} must persist`); + }); + + // The ambient operation-handler context is the shape central-manager runs in, so the rotation has to + // hold there and not only on an explicit `transaction(context, …)` scope. + rocksOnly('rotates under an ambient operation-handler context', async function () { + await assert.rejects( + serverUtilities.processLocalTransaction( + { body: { operation: 'test_registered_op', hdb_user: { username: 'internal_bookkeeping' } } }, + async () => { + const context = contextStorage.getStore(); + await transaction(context, async () => { + await A.put('ambient-pre', { v: 1 }, context); + await context.transaction.commit(); + await A.put('ambient-post', { v: 2 }, context); + throw new Error('forced ambient failure'); + }); + } + ), + /forced ambient failure/ + ); + assert.ok(await A.get('ambient-pre'), 'what the mid-scope commit committed must stay committed'); + assert.equal( + await A.get('ambient-post'), + undefined, + 'a write after the mid-scope commit must roll back under an ambient context too' + ); + }); +});