From b99ae6b9c47e1550106ae937df5159764b30d41b Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Mon, 12 Oct 2026 16:47:27 -0600 Subject: [PATCH 1/4] Confirm a transaction whose only write is a losing relocation on the stored entry, not an unwritten log key _writeRelocate's commit handler returned early when a newer version already existed without marking the write skipped, so staging recorded appendedTo for it and confirmationTarget waited on a log key nothing was appended at. Reset and set write.skipped the way the other commit handlers do. Refs #3182 Dispatch-Task: harper-3182-losing-relocate-skipped Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01MquViG8a7UtvqFChsoL7qH --- resources/Table.ts | 6 +++++- unitTests/resources/residencyConfirmation.test.js | 12 ++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/resources/Table.ts b/resources/Table.ts index 12731ccc09..88ddb9f736 100644 --- a/resources/Table.ts +++ b/resources/Table.ts @@ -3981,7 +3981,11 @@ export function makeTable(options): TableResourceClass { const txnLogKey = isRocksDB ? (reservedLocalKey(transaction) ?? (context as any)?.timestamp ?? txnTime) : txnTime; - if (precedesExistingVersion(txnTime, existingEntry, options?.nodeId) < 0) return; + write.skipped = false; // reset on each retry; cleanup happens after commit if still true + if (precedesExistingVersion(txnTime, existingEntry, options?.nodeId) < 0) { + write.skipped = true; + return; + } const residency = TableResource.getResidencyRecord(options.residencyId); let metadata = 0; let newRecord = null; diff --git a/unitTests/resources/residencyConfirmation.test.js b/unitTests/resources/residencyConfirmation.test.js index 224b0b06fc..91399edf7b 100644 --- a/unitTests/resources/residencyConfirmation.test.js +++ b/unitTests/resources/residencyConfirmation.test.js @@ -120,6 +120,18 @@ describe('replicatedConfirmation residency (harper-pro#1031)', () => { assert.equal(calls[0].version, version); }); + it('confirms a commit whose only write is a relocation that lost to a newer entry on that entry (harper#3182)', async () => { + await Unplaced.put('relocated', { id: 'relocated', body: 'kept' }); + const { version } = Unplaced.primaryStore.getEntry('relocated'); + const context = { replicatedConfirmation: 1, timestamp: version - 60_000 }; + await confirmed(context, async () => { + const resource = await Unplaced.getResource('relocated', context); + resource._writeRelocate('relocated', {}); + }); + assert.equal(Unplaced.primaryStore.getEntry('relocated').version, version); + assert.equal(calls[0].version, version); + }); + it('keeps a count-based placement at its count when a record is updated', async () => { Records.setResidency(undefined); const previousNodes = global.server.nodes; From 2c76e12e7ca20ad493fe4ce8594b96d98641064e Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Mon, 12 Oct 2026 16:55:23 -0600 Subject: [PATCH 2/4] Cover the relocation's per-round skipped reset and a mixed commit A relocation that loses when first staged but applies when an open iterator forces the commit to re-stage it must report the append; a losing relocation next to an applied write still confirms on the log key. Refs #3182 Dispatch-Task: harper-3182-losing-relocate-skipped Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01MquViG8a7UtvqFChsoL7qH --- resources/Table.ts | 2 +- .../resources/residencyConfirmation.test.js | 38 +++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/resources/Table.ts b/resources/Table.ts index 88ddb9f736..11b7908c03 100644 --- a/resources/Table.ts +++ b/resources/Table.ts @@ -3981,7 +3981,7 @@ export function makeTable(options): TableResourceClass { const txnLogKey = isRocksDB ? (reservedLocalKey(transaction) ?? (context as any)?.timestamp ?? txnTime) : txnTime; - write.skipped = false; // reset on each retry; cleanup happens after commit if still true + write.skipped = false; if (precedesExistingVersion(txnTime, existingEntry, options?.nodeId) < 0) { write.skipped = true; return; diff --git a/unitTests/resources/residencyConfirmation.test.js b/unitTests/resources/residencyConfirmation.test.js index 91399edf7b..46ffb11811 100644 --- a/unitTests/resources/residencyConfirmation.test.js +++ b/unitTests/resources/residencyConfirmation.test.js @@ -56,6 +56,10 @@ describe('replicatedConfirmation residency (harper-pro#1031)', () => { const confirmed = (context, write) => transaction(context, write); const sorted = (lists) => lists.map((list) => [...list].sort()).sort(); + const logKeyOf = (Table, id, type) => { + for (const entry of Table.primaryStore.rootStore.auditStore.getRange({ start: 1, log: 0 })) + if (entry.tableId === Table.tableId && entry.recordId === id && entry.type === type) return entry.txnLogKey; + }; it('passes the other residents of a record placed by a residency list', async () => { await confirmed({ replicatedConfirmation: 1 }, () => @@ -128,10 +132,44 @@ describe('replicatedConfirmation residency (harper-pro#1031)', () => { const resource = await Unplaced.getResource('relocated', context); resource._writeRelocate('relocated', {}); }); + assert.equal((await Unplaced.get('relocated')).body, 'kept'); assert.equal(Unplaced.primaryStore.getEntry('relocated').version, version); + assert.equal(calls.length, 1); assert.equal(calls[0].version, version); }); + it('confirms on the log key when a losing relocation shares the commit with an applied write', async () => { + await Unplaced.put('relocated-mixed', { id: 'relocated-mixed', body: 'kept' }); + const { version } = Unplaced.primaryStore.getEntry('relocated-mixed'); + const context = { replicatedConfirmation: 1, timestamp: version - 60_000 }; + await confirmed(context, async () => { + const resource = await Unplaced.getResource('relocated-mixed', context); + resource._writeRelocate('relocated-mixed', {}); + await Unplaced.put('relocated-mixed-new', { id: 'relocated-mixed-new' }); + }); + assert.equal(calls.length, 1); + assert.equal(calls[0].version, logKeyOf(Unplaced, 'relocated-mixed-new', 'put')); + }); + + it('confirms a relocation that lost when staged but applied when re-staged past an open iterator', async () => { + await Unplaced.put('relocated-evicted', { id: 'relocated-evicted', body: 'kept' }); + const { version, value } = Unplaced.primaryStore.getEntry('relocated-evicted'); + const context = { replicatedConfirmation: 1, timestamp: version - 60_000 }; + let iterator; + await confirmed(context, async () => { + // the open iterator makes the commit re-stage every write on a fresh transaction + iterator = Unplaced.search({ conditions: [] })[Symbol.iterator](); + iterator.next(); + const resource = await Unplaced.getResource('relocated-evicted', context); + resource._writeRelocate('relocated-evicted', {}); + await Unplaced.evict('relocated-evicted', value, version); + }); + iterator.return?.(); + assert.equal(Unplaced.primaryStore.getEntry('relocated-evicted'), undefined); + assert.equal(calls.length, 1); + assert.equal(calls[0].version, logKeyOf(Unplaced, 'relocated-evicted', 'relocate')); + }); + it('keeps a count-based placement at its count when a record is updated', async () => { Records.setResidency(undefined); const previousNodes = global.server.nodes; From 0009ab32e005aafeb3b6dfe8156f67344a63f931 Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Mon, 12 Oct 2026 16:55:44 -0600 Subject: [PATCH 3/4] Note in resources/DESIGN.md that staging reads write.skipped as "appended nothing" Refs #3182 Dispatch-Task: harper-3182-losing-relocate-skipped Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01MquViG8a7UtvqFChsoL7qH --- resources/DESIGN.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/resources/DESIGN.md b/resources/DESIGN.md index 6cf880efe9..1b6b37f001 100644 --- a/resources/DESIGN.md +++ b/resources/DESIGN.md @@ -376,7 +376,7 @@ Blobs flagged with `saveBeforeCommit` (or `saveInRecord`) are written to disk in The mitigations live in three places: - `startPreCommitBlobsForRecord` (`blob.ts`) returns the blob list alongside its completion callback so each `TransactionWrite` can attach a `savedBlobs: Blob[]`. `cleanupUnusedBlobs(blobs)` (`blob.ts`) waits for each blob's `saving` promise to settle, then `deleteBlob`s the file — it clears the input list so it's idempotent across repeated calls (e.g. an early-return that also gets caught by the abort path). -- `Table.ts` commit handlers set `write.skipped = true` (and reset to `false` at the top of each invocation) on early-return paths that don't write the record/audit: duplicate-tie, superseded-by-put, no-audit-fullUpdate-loses, and cache-resolve version-changed. The transaction commit success paths (`DatabaseTransaction.commit` and `LMDBTransaction.commit`) walk writes and call `cleanupUnusedBlobs(write.savedBlobs)` for every still-skipped write. Cleanup is deferred (rather than run inline in the commit handler) because the commit handler runs again on optimistic-lock retries, and a retry can flip a previously-skipped write into a successful one (e.g. the existing record gets deleted between attempts so the older replicated update suddenly wins). Inline cleanup would race the deletion's `setTimeout` against the retry that referenced the blob. +- `Table.ts` commit handlers set `write.skipped = true` (and reset to `false` at the top of each invocation) on early-return paths that don't write the record/audit: duplicate-tie, superseded-by-put, no-audit-fullUpdate-loses, a losing invalidate, relocate or delete, and cache-resolve version-changed. Staging reads the same flag as "appended nothing" (`appendedTo`, below), so an unmarked early return also makes `replicatedConfirmation` wait on a log key nothing was written at (harper#3182). The transaction commit success paths (`DatabaseTransaction.commit` and `LMDBTransaction.commit`) walk writes and call `cleanupUnusedBlobs(write.savedBlobs)` for every still-skipped write. Cleanup is deferred (rather than run inline in the commit handler) because the commit handler runs again on optimistic-lock retries, and a retry can flip a previously-skipped write into a successful one (e.g. the existing record gets deleted between attempts so the older replicated update suddenly wins). Inline cleanup would race the deletion's `setTimeout` against the retry that referenced the blob. - `LMDBTransaction.abort` and `DatabaseTransaction.abort` walk all writes and run the same cleanup unconditionally (regardless of `skipped`), since nothing was committed. `DatabaseTransaction.commit` adds an explicit reject handler so a `Promise.all` failure on `completions` (e.g. a blob save errored) aborts the underlying transaction instead of leaking it _and_ the blob files. Both engines funnel synchronous throws and returned rejections from the whole commit chain through that cleanup once (`abortAfterCommitError`); LMDB captures `next` before its success path detaches the link, so a child's rejection still closes every uncommitted link. The cascade skips a link whose native commit is still outstanding — clearing its writes would unlink blobs that commit still references. **A blob instance outlives its file, so deletion tombstones it.** The `fileId` stays set on a `Blob` whose file has been unlinked, and `saveBlob` short-circuits on a set `fileId` — so a caller still holding the instance (the deploy recorder re-puts the same record object across a deploy) would re-encode a reference to a file that is gone. `cleanupUnusedBlobs` marks the blob's shared file state when transaction cleanup decides the file is no longer usable, before an in-flight save settles; slices share that state with their source. Ordinary supersession keeps its retention and re-reference window; reclamation marks every queued instance only after it claims the unlink. `saveBlob` throws on a discarded instance rather than minting a second reference (issue #2062). @@ -923,7 +923,7 @@ confirmable writes (`residentPeerLists`); the waiter needs the full count from e residency adds no list. The position is the committed transaction's reserved log key (`confirmationTarget`), not a record version, which can sit below the key (an explicit timestamp under the origin floor) or above it (a remote-origin version kept by a merge). Only when no confirmable write appended to the committed handle -(every write lost to a newer entry) does it fall back to the stored entry's version, as before. +(every write lost to a newer entry, which its commit handler marks `skipped`) does it fall back to the stored entry's version, as before. The commit handler stamps `write.residencyId` on each round that appends an audit entry and keeps it on a coordinated retry, which appends nothing, so the residents counted are the ones the replicated entry names. From db4669bcf43734b0df720d9b6150636b53d70d42 Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Mon, 12 Oct 2026 17:19:09 -0600 Subject: [PATCH 4/4] Release the replay test's iterator when the confirmed transaction rejects, and fail on a missing audit entry Refs #3182 Dispatch-Task: harper-3182-losing-relocate-skipped Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01MquViG8a7UtvqFChsoL7qH --- .../resources/residencyConfirmation.test.js | 22 +++++++++++-------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/unitTests/resources/residencyConfirmation.test.js b/unitTests/resources/residencyConfirmation.test.js index 46ffb11811..ac6d0b849b 100644 --- a/unitTests/resources/residencyConfirmation.test.js +++ b/unitTests/resources/residencyConfirmation.test.js @@ -59,6 +59,7 @@ describe('replicatedConfirmation residency (harper-pro#1031)', () => { const logKeyOf = (Table, id, type) => { for (const entry of Table.primaryStore.rootStore.auditStore.getRange({ start: 1, log: 0 })) if (entry.tableId === Table.tableId && entry.recordId === id && entry.type === type) return entry.txnLogKey; + assert.fail(`no ${type} audit entry for ${id}`); }; it('passes the other residents of a record placed by a residency list', async () => { @@ -156,15 +157,18 @@ describe('replicatedConfirmation residency (harper-pro#1031)', () => { const { version, value } = Unplaced.primaryStore.getEntry('relocated-evicted'); const context = { replicatedConfirmation: 1, timestamp: version - 60_000 }; let iterator; - await confirmed(context, async () => { - // the open iterator makes the commit re-stage every write on a fresh transaction - iterator = Unplaced.search({ conditions: [] })[Symbol.iterator](); - iterator.next(); - const resource = await Unplaced.getResource('relocated-evicted', context); - resource._writeRelocate('relocated-evicted', {}); - await Unplaced.evict('relocated-evicted', value, version); - }); - iterator.return?.(); + try { + await confirmed(context, async () => { + // the open iterator makes the commit re-stage every write on a fresh transaction + iterator = Unplaced.search({ conditions: [] })[Symbol.iterator](); + iterator.next(); + const resource = await Unplaced.getResource('relocated-evicted', context); + resource._writeRelocate('relocated-evicted', {}); + await Unplaced.evict('relocated-evicted', value, version); + }); + } finally { + iterator?.return?.(); + } assert.equal(Unplaced.primaryStore.getEntry('relocated-evicted'), undefined); assert.equal(calls.length, 1); assert.equal(calls[0].version, logKeyOf(Unplaced, 'relocated-evicted', 'relocate'));