From 34a84b07545fa4949301f92a3a5909372af0fc3b Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Thu, 3 Sep 2026 15:38:17 -0600 Subject: [PATCH 01/16] Report the record version and the log key as separate clocks on RocksDB audit records MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On RocksDB an audit record now carries the two roles LMDB has always had: `version` is the record's own version (LWW ordering, @updatedTime, ETag) and `localTime` is that entry's key in the per-origin transaction log. The read surface used to overwrite `version` with the log key, so a consumer could not tell an ordering value from a resume position, and #2409's `recordVersion` alias is absorbed back into `version`. The two clocks hold the same value for every write whose record version is its own commit timestamp, so they only diverge on a source fill (#2065) — which is why confusing them stayed invisible until a cache table replicated. Three consequences carry the change: - A write applied from elsewhere carries its own record version (`TransactionWrite.recordVersion`), read in `save()` only when the transaction is `sourceApply` or `isReplay`. A replication receiver stores the origin's version while committing under the origin's log key; one frame can carry writes at different record versions, so this cannot be per-transaction. - Write identity is explicit as `(nodeId, log key)` in `isAuditEntryWrite`. The tombstone removal in `removeAuditEntry` and the audit pass of the blob orphan sweep both gate on it rather than on a legitimately non-unique version, and both retain rather than delete when identity is unknown. - Crash replay delimits transactions by the log key and replays each write at its stored version. Without the second half, a peer holding a fill at version V under log key L would have it restamped at L after an unclean restart, making a later legitimate write between V and L look stale (#2411). No record-format change, no wire-format change, no change to how LMDB stores anything. `additionalAuditRefs[].version` stays log-key addressable, because every consumer follows it straight into `auditStore.get`. Refs #2412 Refs #2411 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01WMMWWeFsqFuKduJHNMP6ZY --- dataLayer/harperBridge/ResourceBridge.ts | 6 +- resources/DESIGN.md | 37 +++ resources/DatabaseTransaction.ts | 15 +- resources/LMDBTransaction.ts | 7 +- resources/RocksTransactionLogStore.ts | 11 +- resources/Table.ts | 71 +++-- resources/auditStore.ts | 33 ++- resources/blob.ts | 7 +- resources/replayLogs.ts | 13 +- .../resources/auditEntryRecordFlags.test.js | 6 +- unitTests/resources/auditLog.test.js | 80 +++++- unitTests/resources/blob.test.js | 38 +++ .../resources/dualClockAuditRecord.test.js | 257 ++++++++++++++++++ .../transactionBroadcastGrouping.test.js | 11 +- 14 files changed, 530 insertions(+), 62 deletions(-) create mode 100644 unitTests/resources/dualClockAuditRecord.test.js diff --git a/dataLayer/harperBridge/ResourceBridge.ts b/dataLayer/harperBridge/ResourceBridge.ts index cf92aca4a5..6f0db4d804 100644 --- a/dataLayer/harperBridge/ResourceBridge.ts +++ b/dataLayer/harperBridge/ResourceBridge.ts @@ -579,7 +579,9 @@ export class ResourceBridge extends BridgeMethods { let operation = normalizeHistoryOperation(auditRecord.operation, auditRecord.type); return { operation, - timestamp: auditRecord.version, + // the transaction's position in the log, which is what this field has always + // reported on RocksDB and what groups an atomic transaction (harper#2412) + timestamp: auditRecord.localTime, user_name: auditRecord.user, ids: [id], records: [auditRecord.value], @@ -775,7 +777,7 @@ async function* groupRecordsInHistory(table, start?, end?, limit?) { let count = 0; for await (const entry of table.getHistory(start, end)) { let operation = normalizeHistoryOperation(entry.operation, entry.type); - const { id, version: timestamp, value } = entry; + const { id, localTime: timestamp, value } = entry; if (enqueued?.timestamp === timestamp) { enqueued.ids.push(id); enqueued.records.push(value); diff --git a/resources/DESIGN.md b/resources/DESIGN.md index 814b020861..35e5967768 100644 --- a/resources/DESIGN.md +++ b/resources/DESIGN.md @@ -101,6 +101,43 @@ One giant `makeTable()` factory that returns a `TableResource extends Resource` | How does post-ordering resolve vector distances safely? | Each comparator owns its `Sort`, passes it directly to the custom-index resolver, and caches distances by that immutable per-query sort object. | | How is application row filtering applied? | Authorization admission happens in the resource operation before query work. The legacy `allow*` hook, when armed by the protocol, is evaluated once with its historical receiver semantics; overriding it never changes its scope. An operation override may add indexed conditions and/or attach the JavaScript-only synchronous `target.rowFilter(record, context)`. `Table.search` composes it with query filters and rechecks the final materialized cache/source record. `SubscriptionRequest.rowFilter` covers full-row events; `eventFilter(event, context)` explicitly handles tombstones/messages/raw events. Prefer indexed conditions because an opaque predicate may inspect every admitted candidate and `limit` applies after filtering. | +**An audit record carries two clocks; never substitute one for the other (harper#2412 stage 0b).** +`AuditRecord.version` is the record's own version — LWW ordering in `precedesExistingVersion`, +`@updatedTime`, ETag/`Last-Modified`. It is legitimately non-unique. `AuditRecord.localTime` is that +entry's key in the per-origin transaction log: write identity, the record→log lookup +(`auditStore.get(logKey, tableId, id, nodeId)`), and every resume cursor. On LMDB these have always +been distinct fields; on RocksDB the read surface used to overwrite `version` with the log key, and +`#2409`'s `recordVersion` alias is now absorbed back into `version`. + +They hold the same value for every write whose record version is its own commit timestamp, which is +every ordinary local write, so a bug that confuses them stays invisible until a **source fill** +(`getFromSource`, core #2065): the record is stored at the source-reported version while its log +entry is keyed at the fill's commit. A record stores one word today, so it keeps its version and the +first-word == log-key invariant is restored in stage 2, not here. + +Consequences worth knowing: + +- **Identity is `(nodeId, log key)`, never a version.** `isAuditEntryWrite` (`auditStore.ts`) is the + single predicate; `removeAuditEntry`'s tombstone removal and `blob.ts`'s orphan sweep both gate on + it, and both retain rather than delete when identity is unknown. A version compare there would let + one write authorize destroying another's tombstone or blob. +- **An applied write carries its own record version.** `TransactionWrite.recordVersion`, set from + `options.version` by every `_write*` builder and read in `save()` only when the transaction is + `sourceApply` or `isReplay`, is how a replication receiver stores the origin's version while the + transaction commits under the origin's log key — so a peer's copy of an origin's log stays in the + origin's clock. One frame can carry writes at different record versions, so this cannot be a + per-transaction value. +- **`additionalAuditRefs[].version` is a log key, not a version.** Every consumer follows it straight + into `auditStore.get` (`Table.ts`'s `auditRefsToVisit`), so the out-of-order walk records the write's + `logTime` there. Under stage 2 the two clocks are separable and the field can be renamed; until then, + "fixing" it to the record version silently unaddresses the entry it points at. +- **Crash replay uses both.** `replayLogs` delimits transactions by `localTime` (which is also what + `CorruptFrameStop.truncatedVersions` records) and replays each write at its stored `version`. + Stamping a replayed record at its log key would move its version forward and make a later + legitimate write look stale. +- **`getHistory`/`read_audit_log` report `localTime` as the transaction timestamp.** That is what the + field meant on RocksDB all along; on LMDB it corrects a slot that was reporting the record version. + **QUERY admission uses the body projection.** `Resource.transactional` resolves an asynchronous HTTP QUERY body before resource resolution, recursively clones away client-supplied `checkPermission`, and copies the body's `select` onto the operation admission target before `allowRead`. After authorization, `Resource.query` transfers only the narrowed projection to the body target. This ensures a body-only relationship select is checked before `Table.search`; permission-control fields from QUERY data must never reach a search target. **Async false-mode read gates preserve the streaming contract.** `Table.search` returns an `ExtendedIterable` carrying the internal `SEARCH_AUTHORIZATION` promise. Static `Resource.search` and `query` await that verdict before returning a response; on success the wrapper initializes the real search before the transaction settles so its normal read snapshot stays reserved until iteration completes. The marker follows supported iterable transforms and retains `selectApplied`/`getColumns`, so async or mapped delegation cannot turn a denial into a truncated successful response. diff --git a/resources/DatabaseTransaction.ts b/resources/DatabaseTransaction.ts index f0cea66fd4..5b27520047 100644 --- a/resources/DatabaseTransaction.ts +++ b/resources/DatabaseTransaction.ts @@ -312,6 +312,10 @@ export type TransactionWrite = { // overload accounting, the replay marker and a no-op write's removal all belong to the committer. validate?: (txnTime: number, committedBy: DatabaseTransaction) => void; fullUpdate?: boolean; + // The record version this write stores, when it is not the transaction's own timestamp: an applied + // write (replication receive, crash replay) keeps the origin's version while the transaction commits + // under the origin's log key. Read only on those paths — see save(). + recordVersion?: number; saved?: boolean; deferSave?: boolean; skipReplicationConfirmation?: boolean; @@ -1026,13 +1030,18 @@ export class DatabaseTransaction implements Transaction { (transaction as RocksTransactionWithRetry).isRetry = true; } if (!txnTime) txnTime = this.timestamp = transaction.getTimestamp(); + // `txnTime` is this transaction's timestamp — the key its entries take in the per-origin log. + // A write applied from elsewhere carries the origin's record version too, and that is what the + // record is stored at; the two coincide for every locally-originated write. Gated on the apply + // flags so an ordinary write never reads the property (harper#2412). + const writeVersion = this.sourceApply || this.isReplay ? (operation.recordVersion ?? txnTime) : txnTime; if (reloadEntry || operation.entry === undefined) { operation.entry = operation.store.getEntry(operation.key, { transaction }); } if (!operation.saved) { operation.saved = true; // immediately execute in this transaction - if ((operation.validate?.(txnTime, this) as any) === false) { + if ((operation.validate?.(writeVersion, this) as any) === false) { operation.commit = () => {}; // noop if we try again return; } @@ -1043,9 +1052,9 @@ export class DatabaseTransaction implements Transaction { } if (lockHandle || this.recordLocks) operation.trackRecordVersion = true; if (operation.trackRecordVersion) operation.recordVersionApplied = false; - const completion = operation.commit(txnTime, operation.entry, this.retries > 0, transaction) as Promise; + const completion = operation.commit(writeVersion, operation.entry, this.retries > 0, transaction) as Promise; if (operation.trackRecordVersion) - operation.appliedRecordVersion = operation.recordVersionApplied ? txnTime : undefined; + operation.appliedRecordVersion = operation.recordVersionApplied ? writeVersion : undefined; if (typeof completion?.then === 'function') this.stageCompletion(completion); // Sticky record that THIS write staged with its audit entry appended (log entries batch on the // native transaction and are durably written by its commit attempt — even a failed one — so diff --git a/resources/LMDBTransaction.ts b/resources/LMDBTransaction.ts index 198ee877df..e774e70191 100644 --- a/resources/LMDBTransaction.ts +++ b/resources/LMDBTransaction.ts @@ -203,7 +203,12 @@ export class LMDBTransaction extends DatabaseTransaction { let writeIndex = 0; this.writes = this.writes.filter((write) => write); // filter out removed entries const doWrite = (write) => { - const completion = write.commit(txnTime, write.entry, retries); + // see DatabaseTransaction.save(): an applied write carries the origin's record version + const completion = write.commit( + this.sourceApply || this.isReplay ? (write.recordVersion ?? txnTime) : txnTime, + write.entry, + retries + ); if (typeof completion?.then === 'function') { // the aggregating Promise.all is attached a turn or more later (after the conditional batch // or the exclusive transaction resolves), so handle rejection here to keep the gap from diff --git a/resources/RocksTransactionLogStore.ts b/resources/RocksTransactionLogStore.ts index 6f8ccd25f7..075d1d6500 100644 --- a/resources/RocksTransactionLogStore.ts +++ b/resources/RocksTransactionLogStore.ts @@ -133,7 +133,6 @@ export class RocksTransactionLogStore extends EventEmitter { if (!(auditRecord instanceof Uint8Array)) { const txnTimestamp = options.transaction.getTimestamp?.(); if (txnTimestamp != null) auditRecord.localTime = txnTimestamp; - auditRecord.recordVersion = auditRecord.version; } (options.transaction.logEntries ??= []).push(auditRecord); } @@ -184,7 +183,7 @@ export class RocksTransactionLogStore extends EventEmitter { if (entry.recordId === recordId && entry.tableId === tableId) { return entry; } - if (entry.version !== key) return; // no longer in this transaction + if (entry.localTime !== key) return; // no longer in this transaction } } else { // Harper puts some metadata in the database, we will just put this in the root store instead @@ -479,9 +478,9 @@ export class RocksTransactionLogStore extends EventEmitter { position += 8; } const auditRecord = readAuditEntry(data, position, undefined); - // version stays the log key (replication resume relies on version === log key); - // the record's own version survives as recordVersion - auditRecord.version = timestamp; + // `version` is the record's own version, decoded from the entry; `localTime` is this + // entry's key in the per-origin log. They are the same value for a write whose record + // version is its commit timestamp, and differ for a source fill (harper#2412). auditRecord.localTime = timestamp; auditRecord.endTxn = endTxn; auditRecord.previousResidencyId = previousResidencyId; @@ -494,7 +493,9 @@ export class RocksTransactionLogStore extends EventEmitter { byteLength: data?.byteLength, }); return { + // the log key is all this entry still yields; its record version is undecodable version: timestamp, + localTime: timestamp, endTxn, type: undefined, tableId: undefined, diff --git a/resources/Table.ts b/resources/Table.ts index c121e2bf7a..52a43f9f13 100644 --- a/resources/Table.ts +++ b/resources/Table.ts @@ -852,6 +852,9 @@ export function makeTable(options) { ensureLoaded: false, nodeId: event.nodeId, viaNodeId: event.viaNodeId, + // the origin's record version, stored as-is so every replica holds the version the + // origin holds; the transaction's own timestamp stays the origin's log key + version: event.version, // use per-event expiresAt: batched txn context only holds the first event's expiration expiresAt: event.expiresAt, // bulk base-copy snapshot frame: apply current-state directly, without an audit/transaction-log @@ -1088,7 +1091,9 @@ export function makeTable(options) { continue; } } - // use the version as the transaction timestamp + // A source that reports no log position of its own (no `timestamp`) has only one clock, + // so its record version doubles as the apply transaction's timestamp. A replication + // receiver always sets `timestamp` from the origin's log key and never reaches this. if (!event.timestamp && event.version) event.timestamp = event.version; const commitResolution = transaction(event, () => { if (event.type === 'transaction') { @@ -2220,6 +2225,7 @@ export function makeTable(options) { store: primaryStore, invalidated: true, entry: this.#entry, + recordVersion: options?.version, lockHandle: this.#lockHandle && this.#lockHandle.keyId === writeKeyId(id) ? this.#lockHandle : undefined, commit: (txnTime, existingEntry, _retry, transaction: any) => { write.skipped = false; // reset on each retry; cleanup happens after commit if still true @@ -2270,6 +2276,7 @@ export function makeTable(options) { store: primaryStore, invalidated: true, entry: this.#entry, + recordVersion: options?.version, lockHandle: this.#lockHandle && this.#lockHandle.keyId === writeKeyId(id) ? this.#lockHandle : undefined, before: (this.constructor as any).source?.relocate && !(context as any)?.source @@ -2857,6 +2864,8 @@ export function makeTable(options) { nodeName: (context as any)?.nodeName, fullUpdate, deferSave: true, + // the origin's record version on an applied write; absent for a locally-originated one + recordVersion: options?.version, // Include the lock handle (if any) so the expired-handle guard in // DatabaseTransaction.save() can throw 409 when the lease has lapsed. // Only attach the hold handle when it covers exactly this key; off-key writes @@ -3031,6 +3040,12 @@ export function makeTable(options) { // of the updates to the record to ensure consistency across the cluster // TODO: can the previous version be older, but even more previous version be newer? if (audit) { + // This write's key in the per-origin transaction log: the transaction's own timestamp, + // which a replication apply or a replay adopts from the origin. It is what the keyed + // dedup below looks up — never the record version, which a source fill sets from the + // source and which is legitimately non-unique. Resolved here rather than at the top of + // the commit so an in-order write, the overwhelming majority, never pays the call. + const logTime = transaction?.getTimestamp?.() ?? txnTime; // A re-delivered out-of-order write (full-copy audit-replay re-delivers writes) must not have // its commutative ops re-folded. additionalAuditRefs is the record's own list of folded // out-of-order versions, read with read-your-writes consistency, so this skips the duplicate up @@ -3046,10 +3061,10 @@ export function makeTable(options) { if ( existingEntry.additionalAuditRefs?.some( (ref) => - ref.version === txnTime && + ref.version === logTime && precedesExistingVersion( txnTime, - { version: txnTime, localTime: txnTime, key: id, nodeId: ref.nodeId }, + { version: txnTime, localTime: logTime, key: id, nodeId: ref.nodeId }, options?.nodeId ) === 0 ) @@ -3096,23 +3111,27 @@ export function makeTable(options) { // depth-cap block. This is the same keyed lookup that block performs, hoisted ahead of the walk. // It is what catches transitive/proxied re-deliveries: they arrive buried below the record head // (so replication's head-tie fast-skip can't see them) yet are exact duplicates. Keyed by nodeId, - // so it is correct across multiple source nodes. RocksDB-only: LMDB audit entries are keyed by - // local audit time, not version, so this version-keyed lookup doesn't apply there (LMDB keeps the - // exact unbounded walk). A miss (the keyed lookup can lag a back-to-back re-delivery — #1137) + // so it is correct across multiple source nodes. The lookup key is this write's LOG key, not its + // record version — a replication apply commits under the origin's log key while storing the + // origin's version, and only the log key addresses the entry (harper#2412). The synthetic entry + // below still carries `txnTime` as its version: an audit-only commit records the surviving + // (newer) record version in its body, so `priorAudit.version` is not this write's version. + // RocksDB-only: LMDB audit entries are keyed by local audit time, so this lookup doesn't apply + // there (LMDB keeps the exact unbounded walk). A miss (the keyed lookup can lag a back-to-back re-delivery — #1137) // simply falls through to the walk, so this never changes correctness; the additionalAuditRefs // check above remains the read-your-writes guard. Never when this write staged in a prior // failed attempt: that attempt already appended this write's own audit entry, so the lookup // would find it and skip the write as "already applied" when the record was never committed. // A recommit of the same transaction survived that skip only because the old write batch // still carried the put; a fresh-transaction replay (ERR_TRY_AGAIN) would drop the write. - if (isRocksDB && !stagedOwnAuditEntry && dedupVersionCouldBeRetained(txnTime)) { - const priorAudit = auditStore.get(txnTime, tableId, id, options?.nodeId); + if (isRocksDB && !stagedOwnAuditEntry && dedupVersionCouldBeRetained(logTime)) { + const priorAudit = auditStore.get(logTime, tableId, id, options?.nodeId); if ( priorAudit && - priorAudit.version === txnTime && + priorAudit.localTime === logTime && precedesExistingVersion( txnTime, - { version: txnTime, localTime: txnTime, key: id, nodeId: priorAudit.nodeId }, + { version: txnTime, localTime: logTime, key: id, nodeId: priorAudit.nodeId }, options?.nodeId ) === 0 ) { @@ -3169,14 +3188,14 @@ export function makeTable(options) { // never committed (see the up-front keyed dedup above). const isReDeliveredDuplicate = () => { if (stagedOwnAuditEntry) return false; - if (!dedupVersionCouldBeRetained(txnTime)) return false; // pre-retention version — skip the end-of-log scan (best-effort; see above) - const duplicate = auditStore.get(txnTime, tableId, id, options?.nodeId); + if (!dedupVersionCouldBeRetained(logTime)) return false; // pre-retention log key — skip the end-of-log scan (best-effort; see above) + const duplicate = auditStore.get(logTime, tableId, id, options?.nodeId); return ( duplicate && - duplicate.version === txnTime && + duplicate.localTime === logTime && precedesExistingVersion( txnTime, - { version: txnTime, localTime: txnTime, key: id, nodeId: duplicate.nodeId }, + { version: txnTime, localTime: logTime, key: id, nodeId: duplicate.nodeId }, options?.nodeId ) === 0 ); @@ -3249,10 +3268,13 @@ export function makeTable(options) { } if (!addedAuditRef && isRocksDB) { addedAuditRef = true; - // Add a reference to this older audit record if we had out-of-order writes - additionalAuditRefs.push({ version: txnTime, nodeId: options?.nodeId }); + // Add a reference to this older audit record if we had out-of-order writes. The stored + // value is a LOG key, not a record version: every consumer follows it straight into + // `auditStore.get` (see the `auditRefsToVisit` mapping above and below), and on an + // applied write those two clocks differ. + additionalAuditRefs.push({ version: logTime, nodeId: options?.nodeId }); logger.debug?.('Adding additional audit ref for out-of-order write', { - version: txnTime, + logTime, nodeId: options?.nodeId, }); } @@ -3631,6 +3653,7 @@ export function makeTable(options) { entry, chainsStagedState: true, nodeName: (context as any)?.nodeName, + recordVersion: options?.version, lockHandle: this.#lockHandle && this.#lockHandle.keyId === writeKeyId(id) ? this.#lockHandle : undefined, before: (this.constructor as any).source?.delete && !(context as any)?.source @@ -4756,8 +4779,7 @@ export function makeTable(options) { // been written, so are fresh in memory. const entry: Entry = primaryStore.getEntry(id); if (entry) { - // staleness is a record-version comparison; auditRecord.version is the log key on RocksDB - if (entry.version !== (auditRecord.recordVersion ?? auditRecord.version)) return; // out of order event, with old update, don't send anything + if (entry.version !== auditRecord.version) return; // out of order event, with old update, don't send anything value = entry.value; type = entry.metadataFlags & INVALIDATED ? 'invalidate' : value ? 'put' : 'delete'; } else { @@ -5192,6 +5214,7 @@ export function makeTable(options) { store: primaryStore, entry: this.#entry, nodeName: (context as any)?.nodeName, + recordVersion: options?.version, validate: () => { if (!(context as any)?.source) { transaction.checkOverloaded(); @@ -5977,10 +6000,10 @@ export function makeTable(options) { if (auditRecord.tableId !== tableId) continue; yield { id: auditRecord.recordId, - localTime: auditRecord.version, + localTime: auditRecord.localTime, version: auditRecord.version, type: auditRecord.type, - value: auditRecord.getValue(primaryStore, true, auditRecord.version), + value: auditRecord.getValue(primaryStore, true, auditRecord.localTime), user: auditRecord.user, operation: auditRecord.originatingOperation, }; @@ -6004,12 +6027,12 @@ export function makeTable(options) { if (auditRecord.tableId === tableId && compareKeys(auditRecord.recordId, id) === 0) { history.splice(insertionPoint, 0, { id: auditRecord.recordId, - localTime: auditRecord.version, + localTime: auditRecord.localTime, version: auditRecord.version, type: auditRecord.type, - // reconstruct each entry's record image as of its own version, not the audit + // reconstruct each entry's record image as of its own log position, not the audit // window boundary (nextVersion), matching getHistory (issue #1330) - value: auditRecord.getValue(primaryStore, true, auditRecord.version), + value: auditRecord.getValue(primaryStore, true, auditRecord.localTime), user: auditRecord.user, operation: auditRecord.originatingOperation, }); diff --git a/resources/auditStore.ts b/resources/auditStore.ts index 658a12f411..060d39ff59 100644 --- a/resources/auditStore.ts +++ b/resources/auditStore.ts @@ -33,9 +33,8 @@ import { isReadOnlyMode } from './databases.ts'; initSync(); export type AuditRecord = { - version: number; - recordVersion?: number; // the record's own version; on RocksDB reads `version` becomes the log key, so record identity uses this - localTime: number; // log position: LMDB audit-store key; RocksDB transaction-log timestamp + version: number; // the record's own version: LWW ordering, @updatedTime, ETag + localTime: number; // log position: LMDB audit-store key; RocksDB transaction-log key type: string; encodedRecord?: Buffer; extendedType?: number; @@ -375,18 +374,39 @@ export function openAuditStore(rootStore) { return auditStore; } +/** + * Whether `entry` is the very write `auditRecord` describes. Write identity is (origin node, log key), + * never the record version: a version is legitimately non-unique, so a version match can name a + * different write and authorize destroying live state (a tombstone, a still-referenced blob). + * + * On LMDB this is exact — the audit-store key IS the record's `localTime`. On RocksDB a record stores + * one word, its version, which equals its log key for every write except a source fill (harper#2065); + * a fill therefore answers false here, as the pre-normalization version-versus-log-key compare + * already did. Absent identity answers false too. The direction is deliberate: uncertainty retains. + */ +export function isAuditEntryWrite(entry: any, auditRecord: AuditRecord): boolean { + return ( + entry != null && + auditRecord.localTime != null && + entry.localTime === auditRecord.localTime && + (entry.nodeId ?? 0) === (auditRecord.nodeId ?? 0) + ); +} + export function removeAuditEntry(auditStore: any, auditRecord: AuditRecord): Promise { let tombstoneRemoval: Promise | undefined; if (auditRecord.type === 'delete') { // if this is a delete, we remove the delete entry from the primary table - // at the same time so the audit table the primary table are in sync, assuming the entry matches this audit record version + // at the same time so the audit table the primary table are in sync, assuming the entry is still + // the record state this audit record wrote const tableId = auditRecord.tableId; const primaryStore = auditStore.tableStores[auditRecord.tableId]; - if (primaryStore?.getEntry(auditRecord.recordId)?.version === auditRecord.version) + const tombstone = primaryStore?.getEntry(auditRecord.recordId); + if (isAuditEntryWrite(tombstone, auditRecord)) // a failed tombstone removal doesn't mean the audit entry removal failed — only // auditStore.remove() below decides this function's outcome tombstoneRemoval = new Promise((resolve) => { - resolve(auditStore.deleteCallbacks?.[tableId]?.(auditRecord.recordId, auditRecord.version)); + resolve(auditStore.deleteCallbacks?.[tableId]?.(auditRecord.recordId, tombstone.version)); }).catch((error) => { harperLogger.warn('Error removing deleted record while removing its audit entry', error); }); @@ -729,7 +749,6 @@ export function readAuditEntry(buffer: Uint8Array, start = 0, end = undefined): return buffer.subarray(recordIdStart, recordIdEnd); }, version, - recordVersion: version, previousVersion, get user() { try { diff --git a/resources/blob.ts b/resources/blob.ts index 4749b9fc26..6a54093c63 100644 --- a/resources/blob.ts +++ b/resources/blob.ts @@ -3301,7 +3301,7 @@ function polyfillBlob() { * @param database */ export async function cleanupOrphans(database: any, databaseName?: string) { - const { HAS_BLOBS } = await import('./auditStore.ts'); + const { HAS_BLOBS, isAuditEntryWrite } = await import('./auditStore.ts'); let store: RootDatabase; let auditStore: RootDatabase; let orphansDeleted = 0; @@ -3383,7 +3383,10 @@ export async function cleanupOrphans(database: any, databaseName?: string) { const primaryStore = (auditStore as any).tableStores[(auditRecord as any).tableId]; if (!primaryStore) continue; const entry = primaryStore?.getEntry((auditRecord as any).recordId); - if (!entry || entry.version !== auditRecord.version || !entry.value) { + // Only the write this audit record describes had its blobs scanned by the table loop above. + // Identity is the log key, not the version: a version match can name a different write and + // would skip an audit value whose blobs are still referenced. + if (!entry?.value || !isAuditEntryWrite(entry, auditRecord as any)) { checkObjectForReferences((auditRecord as any).getValue(primaryStore)); } // slow this down a bit to reduce excessive load, this runs approximately at 10k per second diff --git a/resources/replayLogs.ts b/resources/replayLogs.ts index 37f85e05b2..fbe77bd850 100644 --- a/resources/replayLogs.ts +++ b/resources/replayLogs.ts @@ -152,6 +152,7 @@ export function replayLogs(rootStore: RocksDatabase, tables: any, electedReplaye nodeId, recordId, version, + localTime, residencyId, expiresAt, originatingOperation, @@ -212,9 +213,13 @@ export function replayLogs(rootStore: RocksDatabase, tables: any, electedReplaye warnedReplayHappening = true; console.warn('Harper was not properly shutdown, replaying transaction logs to synchronize database'); } - if (lastTimestamp !== version) { + // Transactions are delimited by the log key — which is also what `truncatedVersions` records — + // and each write is replayed at its own stored record version. The two differ for a source + // fill, and stamping such a record at the log key would move its version forward and make a + // later legitimate write in between look stale (harper#2411). + if (lastTimestamp !== localTime) { const torn = entries.corruptFrameStop.truncatedVersions.has(lastTimestamp); - lastTimestamp = version; + lastTimestamp = localTime; try { // commit the last transaction since we are starting a new one, unless a corrupt // frame swallowed the rest of it — half of a source transaction must never become @@ -256,7 +261,7 @@ export function replayLogs(rootStore: RocksDatabase, tables: any, electedReplaye } transaction = new DatabaseTransaction(); transaction.db = primaryStore; - transaction.timestamp = version; + transaction.timestamp = localTime; // retries=1 routes operation.commit() through its retry path (no duplicate audit staging) transaction.retries = 1; // Explicit replay marker: skips schema validation (harper#1316) and makes save() stamp @@ -265,7 +270,7 @@ export function replayLogs(rootStore: RocksDatabase, tables: any, electedReplaye transaction.isReplay = true; } context.transaction = transaction; - const options = { context, residencyId, nodeId, originatingOperation }; + const options = { context, residencyId, nodeId, originatingOperation, version }; writes++; stagedWrites++; switch (type) { diff --git a/unitTests/resources/auditEntryRecordFlags.test.js b/unitTests/resources/auditEntryRecordFlags.test.js index dc002782b1..92717f8b43 100644 --- a/unitTests/resources/auditEntryRecordFlags.test.js +++ b/unitTests/resources/auditEntryRecordFlags.test.js @@ -128,10 +128,12 @@ describe('Audit entry record flags match the body (#2153)', () => { let loser; for (const entry of T.auditStore.getRange({ start: 1 })) { - if (entry.version === loserVersion) loser = entry; + // keyed by log position: an audit-only commit records the surviving (newer) record version + // in its body, so only the log key identifies the superseded write's entry + if (entry.localTime === loserVersion) loser = entry; // every minted entry must be internally consistent: record flags imply a body if (entry.extendedType & (HAS_RECORD | HAS_PARTIAL_RECORD)) { - assert(entry.getBinaryValue().length > 0, `entry ${entry.version} advertises a record but has no body`); + assert(entry.getBinaryValue().length > 0, `entry ${entry.localTime} advertises a record but has no body`); } } assert(loser, 'audit entry for the superseded write should exist'); diff --git a/unitTests/resources/auditLog.test.js b/unitTests/resources/auditLog.test.js index cca732fa15..be2433b021 100644 --- a/unitTests/resources/auditLog.test.js +++ b/unitTests/resources/auditLog.test.js @@ -754,14 +754,22 @@ describe('Audit log', () => { it(`removeAuditEntry does not let a delete-callback that ${label} block or escape the audit-store removal`, async () => { const auditRemoveCalls = []; const fakeAuditStore = { - tableStores: { 7: { getEntry: () => ({ version: 42 }) } }, + tableStores: { 7: { getEntry: () => ({ version: 42, localTime: 42, nodeId: 0 }) } }, deleteCallbacks: { 7: failingCallback }, remove(key) { auditRemoveCalls.push(key); return Promise.resolve(); }, }; - const deleteAuditRecord = { type: 'delete', tableId: 7, recordId: 'orphan', version: 42, key: 'audit-key' }; + const deleteAuditRecord = { + type: 'delete', + tableId: 7, + recordId: 'orphan', + version: 42, + localTime: 42, + nodeId: 0, + key: 'audit-key', + }; let unhandledRejection; const onUnhandledRejection = (reason) => { @@ -786,6 +794,71 @@ describe('Audit log', () => { ); }); } + // harper#2412: a delete's audit entry authorizes removing the record's tombstone, so it has to name + // the very write that left it. The record version cannot: it is legitimately non-unique, and a + // version match on a different write would destroy live state. + describe('removeAuditEntry tombstone identity', () => { + function fakeStore(tombstone) { + const removals = []; + return { + removals, + store: { + tableStores: { 7: { getEntry: () => tombstone } }, + deleteCallbacks: { 7: (id, version) => removals.push({ id, version }) }, + remove: () => Promise.resolve(), + }, + }; + } + + it('does not remove a tombstone that merely shares the audit record version', async () => { + const { store, removals } = fakeStore({ version: 42, localTime: 900, nodeId: 0 }); + await removeAuditEntry(store, { + type: 'delete', + tableId: 7, + recordId: 'r', + version: 42, + localTime: 100, + nodeId: 0, + key: 'audit-key', + }); + assert.deepEqual(removals, [], 'a version match at a different log position is a different write'); + }); + + it('removes the tombstone the audit record actually wrote', async () => { + const { store, removals } = fakeStore({ version: 42, localTime: 100, nodeId: 0 }); + await removeAuditEntry(store, { + type: 'delete', + tableId: 7, + recordId: 'r', + version: 7, + localTime: 100, + nodeId: 0, + key: 'audit-key', + }); + assert.deepEqual(removals, [{ id: 'r', version: 42 }]); + }); + + it('does not remove a tombstone written by a different origin at the same log position', async () => { + const { store, removals } = fakeStore({ version: 42, localTime: 100, nodeId: 3 }); + await removeAuditEntry(store, { + type: 'delete', + tableId: 7, + recordId: 'r', + version: 42, + localTime: 100, + nodeId: 5, + key: 'audit-key', + }); + assert.deepEqual(removals, [], "identity is (origin, log key); one origin cannot claim another's write"); + }); + + it('retains the tombstone when the audit record carries no log position', async () => { + const { store, removals } = fakeStore({ version: 42, nodeId: 0 }); + await removeAuditEntry(store, { type: 'delete', tableId: 7, recordId: 'r', version: 42, key: 'audit-key' }); + assert.deepEqual(removals, [], 'unknown identity must retain, never delete'); + }); + }); + it('check log after operations and prune', async () => { await AuditedTable.operation({ operation: 'upsert', @@ -1307,7 +1380,8 @@ describe('Audit log', () => { const timestamps = []; assert.doesNotThrow(() => { for (const record of store.getRange({})) { - timestamps.push(record.version); + // localTime is the log key: these synthetic entries carry no decodable record version + timestamps.push(record.localTime); } }, 'aggregate iteration must not propagate the corrupt-entry RangeError'); diff --git a/unitTests/resources/blob.test.js b/unitTests/resources/blob.test.js index 35824d8c1f..72b431b72d 100644 --- a/unitTests/resources/blob.test.js +++ b/unitTests/resources/blob.test.js @@ -1218,6 +1218,44 @@ describe('Blob test', () => { assert.equal(orphansDeleted, 0); }); + // harper#2412: the orphan sweep skips scanning an audit entry's value only when the primary record + // IS that entry's write, because the table pass already scanned it. Identifying that write by record + // version instead of log position deletes blobs a retained audit entry still references, since a + // version is legitimately non-unique. + it('scans an audit value whose record version matches a different write', async () => { + // A file on disk that only an audit entry references, and a live record for the same id whose + // version happens to equal that entry's. Identifying the entry's write by version would treat + // the live record as its output, skip scanning the entry, and unlink a referenced blob. + const { blob, filePath } = await makeDiskBackedBlob(4096); + const id = 'orphan-identity'; + await BlobTest.put(id, { id, other: 'no blob here' }); + const entry = BlobTest.primaryStore.getEntry(id); + const auditStore = BlobTest.primaryStore.rootStore.auditStore; + const originalGetRange = auditStore.getRange.bind(auditStore); + auditStore.getRange = function (options) { + if (options?.start !== 1) return originalGetRange(options); + return [ + { + tableId: BlobTest.tableId, + recordId: id, + type: 'put', + version: entry.version, + localTime: entry.localTime + 1, + nodeId: entry.nodeId ?? 0, + getValue: () => ({ blob }), + }, + ]; + }; + try { + await cleanupOrphans(getDatabases().test); + assert(existsSync(filePath), 'a blob referenced by a retained audit entry must survive the sweep'); + } finally { + auditStore.getRange = originalGetRange; + if (existsSync(filePath)) unlinkSync(filePath); + await BlobTest.delete(id).catch(() => {}); + } + }); + // Helper: produce a blob backed ONLY by its on-disk file (no in-memory contentBuffer), the way a // node reads a blob it didn't write itself — a fresh full-copy replica or a read after the record // fell out of the in-memory cache. We save a blob to disk, encode it to its storage reference, then diff --git a/unitTests/resources/dualClockAuditRecord.test.js b/unitTests/resources/dualClockAuditRecord.test.js new file mode 100644 index 0000000000..bf48f57537 --- /dev/null +++ b/unitTests/resources/dualClockAuditRecord.test.js @@ -0,0 +1,257 @@ +// harper#2412 stage 0b: an audit record carries two clocks, and they must not be confused. +// +// version — the record's own version: LWW ordering, @updatedTime, ETag. Legitimately non-unique. +// localTime — this entry's key in the per-origin transaction log. Write identity, resume cursor. +// +// They hold the same value for a write whose record version is its own commit timestamp, which is +// every ordinary local write — so a test that only writes ordinary records cannot tell the two +// apart, and every case below deliberately drives them apart. +require('../testUtils'); +const assert = require('node:assert'); +const { setupTestDBPath } = require('../testUtils'); +const { table } = require('#src/resources/databases'); +const { Resource } = require('#src/resources/Resource'); +const { setMainIsWorker } = require('#js/server/threads/manageThreads'); +const { transaction } = require('#src/resources/transaction'); +const { waitFor } = require('../waitFor.js'); + +const isLMDB = process.env.HARPER_STORAGE_ENGINE === 'lmdb'; + +describe('Dual-clock audit records (harper#2412)', () => { + let Plain, Filled, auditStore; + let reportedVersion; + + // Every audit entry this suite's tables produced, newest last. + function auditEntriesFor(TableClass, id) { + const entries = []; + for (const auditRecord of auditStore.getRange({ start: 1 })) { + if (auditRecord.tableId !== TableClass.tableId) continue; + if (auditRecord.recordId !== id) continue; + entries.push({ + type: auditRecord.type, + version: auditRecord.version, + localTime: auditRecord.localTime, + }); + } + return entries; + } + + // The receive path: the apply transaction commits under the origin's log key while each write + // stores the origin's record version (Table.ts's apply dispatcher -> options.version). + function applyFromOrigin(TableClass, id, record, { logKey, version, nodeId = 1 }) { + const context = { source: {}, sourceApply: true, timestamp: logKey }; + return transaction(context, async () => { + const resource = await TableClass.getResource(id, context); + return resource._writeUpdate(id, record, true, { isNotification: true, nodeId, version }); + }); + } + + before(async function () { + if (isLMDB) return; + setupTestDBPath(); + setMainIsWorker(true); + Plain = table({ + table: 'DualClockPlain', + database: 'test', + attributes: [{ name: 'id', isPrimaryKey: true }, { name: 'name' }], + audit: true, + }); + Filled = table({ + table: 'DualClockFilled', + database: 'test', + attributes: [{ name: 'id', isPrimaryKey: true }, { name: 'name' }], + audit: true, + }); + Filled.sourcedFrom( + class extends Resource { + get() { + // a source that reports a version older than the fill's own commit: core #2065 + reportedVersion = Date.now() - 60_000; + this.getContext().lastModified = reportedVersion; + return { id: this.getId(), name: 'from-source' }; + } + } + ); + auditStore = Plain.primaryStore.rootStore.auditStore; + }); + + it('a plain write records its version as both the record version and the log key', async function () { + if (isLMDB) return this.skip(); + const id = 'plain-1'; + await Plain.put(id, { id, name: 'local' }); + const [entry] = auditEntriesFor(Plain, id); + assert.ok(entry, 'the write must have produced an audit entry'); + assert.equal(entry.version, Plain.primaryStore.getEntry(id).version, 'version is the record version'); + assert.equal(entry.localTime, entry.version, 'a locally-originated write commits at its own version'); + }); + + it('a source fill records the source version and the fill commit as separate clocks', async function () { + if (isLMDB) return this.skip(); + const id = 'fill-1'; + await Filled.get(id); + await waitFor(() => !Filled.primaryStore.hasLock(id), { message: 'the fill should finish committing' }); + const storedVersion = Filled.primaryStore.getEntry(id).version; + assert.equal(storedVersion, reportedVersion, 'the record keeps the version the source reported'); + const [entry] = auditEntriesFor(Filled, id); + assert.ok(entry, 'the fill must have produced an audit entry'); + assert.equal(entry.version, reportedVersion, 'the audit record carries the record version'); + assert.ok( + entry.localTime > reportedVersion, + `the log key is the fill's commit, not its version (localTime ${entry.localTime}, version ${entry.version})` + ); + }); + + it('an applied write keeps the origin version and takes the origin log key', async function () { + if (isLMDB) return this.skip(); + const id = 'applied-1'; + const logKey = Date.now(); + const version = logKey - 30_000; + await applyFromOrigin(Plain, id, { id, name: 'from-origin' }, { logKey, version }); + assert.equal(Plain.primaryStore.getEntry(id).version, version, 'the peer stores the origin record version'); + const [entry] = auditEntriesFor(Plain, id); + assert.equal(entry.version, version, 'the audit record carries the origin record version'); + assert.equal(entry.localTime, logKey, "the peer's log key for this write is the origin's log key"); + }); + + it('one applied transaction can carry writes at different record versions', async function () { + if (isLMDB) return this.skip(); + // A sender frames by log key, so entries committed together — a fill and an ordinary write — + // reach the receiver in one transaction with different record versions. Each has to keep its own. + const logKey = Date.now() + 1; + const olderVersion = logKey - 45_000; + const context = { source: {}, sourceApply: true, timestamp: logKey }; + await transaction(context, async () => { + const older = await Plain.getResource('batched-old', context); + await older._writeUpdate('batched-old', { id: 'batched-old', name: 'a' }, true, { + isNotification: true, + nodeId: 1, + version: olderVersion, + }); + const current = await Plain.getResource('batched-new', context); + await current._writeUpdate('batched-new', { id: 'batched-new', name: 'b' }, true, { + isNotification: true, + nodeId: 1, + version: logKey, + }); + }); + assert.equal(Plain.primaryStore.getEntry('batched-old').version, olderVersion); + assert.equal(Plain.primaryStore.getEntry('batched-new').version, logKey); + const [oldEntry] = auditEntriesFor(Plain, 'batched-old'); + const [newEntry] = auditEntriesFor(Plain, 'batched-new'); + assert.equal(oldEntry.version, olderVersion); + assert.equal(newEntry.version, logKey); + assert.equal(oldEntry.localTime, logKey, 'both share the transaction log key'); + assert.equal(newEntry.localTime, logKey); + }); + + it('an ordinary local write ignores a record version it was not given', async function () { + if (isLMDB) return this.skip(); + // The per-write clock is read only on an apply path; a plain put must be unaffected by it. + const id = 'plain-2'; + await Plain.put(id, { id, name: 'still-local' }); + const [entry] = auditEntriesFor(Plain, id); + assert.equal(entry.localTime, entry.version); + }); + + it('delivers a subscriber event whose version is the record version and localTime the log position', async function () { + if (isLMDB) return this.skip(); + const id = 'transport-1'; + const subscription = await Plain.subscribe({}); + const events = []; + subscription.on('data', (event) => events.push(event)); + try { + const logKey = Date.now() + 2; + const version = logKey - 90_000; + await applyFromOrigin(Plain, id, { id, name: 'delivered' }, { logKey, version }); + await waitFor(() => events.some((event) => event.id === id), { + message: 'the applied write should reach the subscription', + }); + const event = events.find((candidate) => candidate.id === id); + assert.equal(event.version, version, 'event.version is the record version'); + assert.equal(event.localTime, logKey, 'event.localTime is the log position'); + } finally { + subscription.close(); + } + }); + + it('still removes a real tombstone through deleteHistory on the live store', async function () { + if (isLMDB) return this.skip(); + // The identity predicate reads `localTime` off the PRIMARY entry. On RocksDB that is the record's + // stored word, written by the encoder's metadata prefix — including on a tombstone, which is a + // stored null rather than an absent key. A mocked entry could hide that; this one cannot. + const id = 'tombstone-1'; + await Plain.put(id, { id, name: 'to-be-deleted' }); + await Plain.delete(id); + const tombstone = Plain.primaryStore.getEntry(id); + assert.equal(tombstone.value, null, 'the delete must leave a tombstone, not an absent key'); + assert.equal(tombstone.localTime, tombstone.version, 'a locally-written record stores one word'); + const [, deleteEntry] = auditEntriesFor(Plain, id); + assert.equal(deleteEntry.type, 'delete'); + assert.equal(deleteEntry.localTime, tombstone.localTime, "the audit entry names the tombstone's write"); + await Plain.deleteHistory(Date.now() + 60_000); + assert.equal(Plain.primaryStore.getEntry(id), undefined, 'the tombstone must be removed with its entry'); + }); + + it('looks a record up in the log by its log key, not by its version', async function () { + if (isLMDB) return this.skip(); + // auditStore.get(key, ...) walks the entries at one log key; a fill's entry sits at its commit + // key while the record itself stores the source version, so keying by version must not find it. + const id = 'lookup-1'; + const logKey = Date.now() + 3; + const version = logKey - 120_000; + // nodeId 0 so the entry lands in — and is read back from — the one log a single-node test has + await applyFromOrigin(Plain, id, { id, name: 'lookup' }, { logKey, version, nodeId: 0 }); + const found = auditStore.get(logKey, Plain.tableId, id, 0); + assert.ok(found, 'the entry is addressable by its log key'); + assert.equal(found.version, version); + assert.equal(auditStore.get(version, Plain.tableId, id, 0), undefined, 'and not by its record version'); + }); +}); + +// LMDB has carried the two clocks in separate fields all along, so nothing here is normalized — but the +// per-write record version added for the receive path runs through LMDBTransaction's own commit loop, +// and that branch would otherwise ship untested. +describe('Dual-clock audit records on LMDB (harper#2412)', () => { + let Applied; + + before(async function () { + if (!isLMDB) return; + setupTestDBPath(); + setMainIsWorker(true); + Applied = table({ + table: 'DualClockLmdb', + database: 'test', + attributes: [{ name: 'id', isPrimaryKey: true }, { name: 'name' }], + audit: true, + }); + }); + + it('stores the origin record version on an applied write and keys the entry by its own audit time', async function () { + if (!isLMDB) return this.skip(); + const id = 'lmdb-applied-1'; + const originLogKey = Date.now(); + const version = originLogKey - 30_000; + const context = { source: {}, sourceApply: true, timestamp: originLogKey }; + await transaction(context, async () => { + const resource = await Applied.getResource(id, context); + return resource._writeUpdate(id, { id, name: 'from-origin' }, true, { + isNotification: true, + nodeId: 1, + version, + }); + }); + assert.equal(Applied.primaryStore.getEntry(id).version, version, 'the peer stores the origin record version'); + const auditStore = Applied.primaryStore.rootStore.auditStore; + let entry; + for (const auditRecord of auditStore.getRange({ start: 1 })) { + if (auditRecord.tableId === Applied.tableId && auditRecord.recordId === id) entry = auditRecord; + } + assert.ok(entry, 'the applied write must have produced an audit entry'); + assert.equal(entry.version, version, 'the audit entry carries the origin record version'); + assert.equal( + entry.localTime, + Applied.primaryStore.getEntry(id).localTime, + "on LMDB the audit key is the receiver's own local time for the record, not the origin's log key" + ); + }); +}); diff --git a/unitTests/resources/transactionBroadcastGrouping.test.js b/unitTests/resources/transactionBroadcastGrouping.test.js index 03eb99aa19..a0d0bccbf0 100644 --- a/unitTests/resources/transactionBroadcastGrouping.test.js +++ b/unitTests/resources/transactionBroadcastGrouping.test.js @@ -39,16 +39,9 @@ describe('transactionBroadcast transaction grouping', () => { try { const logKey = Date.now(); table.auditStore.emit('aftercommit', [ - { type: 'put', tableId: 1, recordId: 'a', version: logKey, recordVersion: logKey, localTime: logKey }, + { type: 'put', tableId: 1, recordId: 'a', version: logKey, localTime: logKey }, // a source fill in the same commit: backdated record version, same log key - { - type: 'put', - tableId: 1, - recordId: 'b', - version: logKey - 5000, - recordVersion: logKey - 5000, - localTime: logKey, - }, + { type: 'put', tableId: 1, recordId: 'b', version: logKey - 5000, localTime: logKey }, ]); await waitFor(() => events.length === 3, { message: 'both entries and the end_txn should be delivered' }); assert.deepEqual(events, [ From d8d0805962c92c40f741ab65c46e96191a2bb471 Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Thu, 3 Sep 2026 23:08:31 -0600 Subject: [PATCH 02/16] Rename audit log position to txnLogKey --- resources/DESIGN.md | 18 +++--- resources/DatabaseTransaction.ts | 12 ++-- resources/LMDBTransaction.ts | 11 ++-- resources/RocksTransactionLogStore.ts | 13 ++--- resources/Table.ts | 56 +++++++++---------- resources/auditStore.ts | 10 ++-- resources/replayLogs.ts | 8 +-- resources/transactionBroadcast.ts | 6 +- .../resources/auditEntryRecordFlags.test.js | 4 +- unitTests/resources/auditLog.test.js | 16 +++--- unitTests/resources/blob.test.js | 2 +- .../resources/dualClockAuditRecord.test.js | 36 ++++++++---- .../transactionBroadcastGrouping.test.js | 10 ++-- 13 files changed, 110 insertions(+), 92 deletions(-) diff --git a/resources/DESIGN.md b/resources/DESIGN.md index 35e5967768..746ca9740f 100644 --- a/resources/DESIGN.md +++ b/resources/DESIGN.md @@ -103,7 +103,8 @@ One giant `makeTable()` factory that returns a `TableResource extends Resource` **An audit record carries two clocks; never substitute one for the other (harper#2412 stage 0b).** `AuditRecord.version` is the record's own version — LWW ordering in `precedesExistingVersion`, -`@updatedTime`, ETag/`Last-Modified`. It is legitimately non-unique. `AuditRecord.localTime` is that +`@updatedTime`, ETag/`Last-Modified`. For audit-only out-of-order entries the body may carry the +surviving record version rather than the originating write's version. `AuditRecord.txnLogKey` is that entry's key in the per-origin transaction log: write identity, the record→log lookup (`auditStore.get(logKey, tableId, id, nodeId)`), and every resume cursor. On LMDB these have always been distinct fields; on RocksDB the read surface used to overwrite `version` with the log key, and @@ -125,18 +126,21 @@ Consequences worth knowing: `options.version` by every `_write*` builder and read in `save()` only when the transaction is `sourceApply` or `isReplay`, is how a replication receiver stores the origin's version while the transaction commits under the origin's log key — so a peer's copy of an origin's log stays in the - origin's clock. One frame can carry writes at different record versions, so this cannot be a - per-transaction value. + origin's clock. `getAppliedWriteVersion` bounds the body value by `txnLogKey`: a source fill keeps + its earlier source version, while an out-of-order audit body cannot restamp its originating write + at the later surviving version. One frame can carry writes at different record versions, so this + cannot be a per-transaction value. - **`additionalAuditRefs[].version` is a log key, not a version.** Every consumer follows it straight into `auditStore.get` (`Table.ts`'s `auditRefsToVisit`), so the out-of-order walk records the write's - `logTime` there. Under stage 2 the two clocks are separable and the field can be renamed; until then, + `txnLogKey` there. Under stage 2 the stored reference field can be renamed; until then, "fixing" it to the record version silently unaddresses the entry it points at. -- **Crash replay uses both.** `replayLogs` delimits transactions by `localTime` (which is also what +- **Crash replay uses both.** `replayLogs` delimits transactions by `txnLogKey` (which is also what `CorruptFrameStop.truncatedVersions` records) and replays each write at its stored `version`. Stamping a replayed record at its log key would move its version forward and make a later legitimate write look stale. -- **`getHistory`/`read_audit_log` report `localTime` as the transaction timestamp.** That is what the - field meant on RocksDB all along; on LMDB it corrects a slot that was reporting the record version. +- **Compatibility surfaces still report `localTime` as the transaction timestamp.** Subscription, + history, and pro-to-core replication event shapes predate this internal name and remain unchanged; + no on-disk or on-wire identifier changes in this stage. **QUERY admission uses the body projection.** `Resource.transactional` resolves an asynchronous HTTP QUERY body before resource resolution, recursively clones away client-supplied `checkPermission`, and copies the body's `select` onto the operation admission target before `allowRead`. After authorization, `Resource.query` transfers only the narrowed projection to the body target. This ensures a body-only relationship select is checked before `Table.search`; permission-control fields from QUERY data must never reach a search target. diff --git a/resources/DatabaseTransaction.ts b/resources/DatabaseTransaction.ts index 5b27520047..e7849cff82 100644 --- a/resources/DatabaseTransaction.ts +++ b/resources/DatabaseTransaction.ts @@ -312,9 +312,8 @@ export type TransactionWrite = { // overload accounting, the replay marker and a no-op write's removal all belong to the committer. validate?: (txnTime: number, committedBy: DatabaseTransaction) => void; fullUpdate?: boolean; - // The record version this write stores, when it is not the transaction's own timestamp: an applied - // write (replication receive, crash replay) keeps the origin's version while the transaction commits - // under the origin's log key. Read only on those paths — see save(). + // The audit body's candidate record version. Applied writes bound it by the origin's transaction-log + // key because an out-of-order audit-only entry can carry the later surviving record version. recordVersion?: number; saved?: boolean; deferSave?: boolean; @@ -375,6 +374,10 @@ export type TransactionWrite = { innerCommit?: MaybePromise; }; +export function getAppliedWriteVersion(recordVersion: number | undefined, txnLogKey: number): number { + return recordVersion == null ? txnLogKey : Math.min(recordVersion, txnLogKey); +} + /** * The state a preceding write in this transaction left for `operation`'s key, or undefined if this * is the first write to it. Within a transaction the writes are ordered by program order, and @@ -1034,7 +1037,8 @@ export class DatabaseTransaction implements Transaction { // A write applied from elsewhere carries the origin's record version too, and that is what the // record is stored at; the two coincide for every locally-originated write. Gated on the apply // flags so an ordinary write never reads the property (harper#2412). - const writeVersion = this.sourceApply || this.isReplay ? (operation.recordVersion ?? txnTime) : txnTime; + const writeVersion = + this.sourceApply || this.isReplay ? getAppliedWriteVersion(operation.recordVersion, txnTime) : txnTime; if (reloadEntry || operation.entry === undefined) { operation.entry = operation.store.getEntry(operation.key, { transaction }); } diff --git a/resources/LMDBTransaction.ts b/resources/LMDBTransaction.ts index e774e70191..e006ec9a1d 100644 --- a/resources/LMDBTransaction.ts +++ b/resources/LMDBTransaction.ts @@ -1,5 +1,6 @@ import { DatabaseTransaction, + getAppliedWriteVersion, shouldSpareCommitPhase, transactionOpenTooLongError, type CommitOptions, @@ -131,6 +132,8 @@ export class LMDBTransaction extends DatabaseTransaction { if (!txnTime) txnTime = this.timestamp = options.timestamp || getNextMonotonicTime(); if (!options.timestamp) options.timestamp = txnTime; const retries = options.retries || 0; + const writeVersion = (write: TransactionWrite) => + this.sourceApply || this.isReplay ? getAppliedWriteVersion(write.recordVersion, txnTime) : txnTime; // now validate if (this.validated < this.writes.length) { try { @@ -140,7 +143,7 @@ export class LMDBTransaction extends DatabaseTransaction { this.validated = this.writes.length; for (let i = start; i < this.validated; i++) { const write = this.writes[i]; - write?.validate?.(this.timestamp, this); + write?.validate?.(writeVersion(write), this); } let hasBefore; for (let i = start; i < this.validated; i++) { @@ -204,11 +207,7 @@ export class LMDBTransaction extends DatabaseTransaction { this.writes = this.writes.filter((write) => write); // filter out removed entries const doWrite = (write) => { // see DatabaseTransaction.save(): an applied write carries the origin's record version - const completion = write.commit( - this.sourceApply || this.isReplay ? (write.recordVersion ?? txnTime) : txnTime, - write.entry, - retries - ); + const completion = write.commit(writeVersion(write), write.entry, retries); if (typeof completion?.then === 'function') { // the aggregating Promise.all is attached a turn or more later (after the conditional batch // or the exclusive transaction resolves), so handle rejection here to keep the gap from diff --git a/resources/RocksTransactionLogStore.ts b/resources/RocksTransactionLogStore.ts index 075d1d6500..a692687377 100644 --- a/resources/RocksTransactionLogStore.ts +++ b/resources/RocksTransactionLogStore.ts @@ -131,8 +131,8 @@ export class RocksTransactionLogStore extends EventEmitter { } if (this.listenerCount('aftercommit')) { if (!(auditRecord instanceof Uint8Array)) { - const txnTimestamp = options.transaction.getTimestamp?.(); - if (txnTimestamp != null) auditRecord.localTime = txnTimestamp; + const txnLogKey = options.transaction.getTimestamp?.(); + if (txnLogKey != null) auditRecord.txnLogKey = txnLogKey; } (options.transaction.logEntries ??= []).push(auditRecord); } @@ -183,7 +183,7 @@ export class RocksTransactionLogStore extends EventEmitter { if (entry.recordId === recordId && entry.tableId === tableId) { return entry; } - if (entry.localTime !== key) return; // no longer in this transaction + if (entry.txnLogKey !== key) return; // no longer in this transaction } } else { // Harper puts some metadata in the database, we will just put this in the root store instead @@ -478,10 +478,7 @@ export class RocksTransactionLogStore extends EventEmitter { position += 8; } const auditRecord = readAuditEntry(data, position, undefined); - // `version` is the record's own version, decoded from the entry; `localTime` is this - // entry's key in the per-origin log. They are the same value for a write whose record - // version is its commit timestamp, and differ for a source fill (harper#2412). - auditRecord.localTime = timestamp; + auditRecord.txnLogKey = timestamp; auditRecord.endTxn = endTxn; auditRecord.previousResidencyId = previousResidencyId; auditRecord.previousVersion = previousVersion; @@ -495,7 +492,7 @@ export class RocksTransactionLogStore extends EventEmitter { return { // the log key is all this entry still yields; its record version is undecodable version: timestamp, - localTime: timestamp, + txnLogKey: timestamp, endTxn, type: undefined, tableId: undefined, diff --git a/resources/Table.ts b/resources/Table.ts index 52a43f9f13..29abb38daf 100644 --- a/resources/Table.ts +++ b/resources/Table.ts @@ -3045,7 +3045,7 @@ export function makeTable(options) { // dedup below looks up — never the record version, which a source fill sets from the // source and which is legitimately non-unique. Resolved here rather than at the top of // the commit so an in-order write, the overwhelming majority, never pays the call. - const logTime = transaction?.getTimestamp?.() ?? txnTime; + const txnLogKey = transaction?.getTimestamp?.() ?? txnTime; // A re-delivered out-of-order write (full-copy audit-replay re-delivers writes) must not have // its commutative ops re-folded. additionalAuditRefs is the record's own list of folded // out-of-order versions, read with read-your-writes consistency, so this skips the duplicate up @@ -3061,10 +3061,10 @@ export function makeTable(options) { if ( existingEntry.additionalAuditRefs?.some( (ref) => - ref.version === logTime && + ref.version === txnLogKey && precedesExistingVersion( txnTime, - { version: txnTime, localTime: logTime, key: id, nodeId: ref.nodeId }, + { version: txnTime, localTime: txnLogKey, key: id, nodeId: ref.nodeId }, options?.nodeId ) === 0 ) @@ -3095,10 +3095,10 @@ export function makeTable(options) { if (!oldestRetainedAuditTimeResolved) { oldestRetainedAuditTimeResolved = true; // getRange yields ascending by audit-log key, so the first entry is the oldest retained. - // Mirror replicationConnection's retention check and the cleanup key basis (localTime ?? - // version). Fall back to the nominal time-based purge floor when the log is empty/unavailable. + // Mirror replicationConnection's retention check and the cleanup key basis (`txnLogKey`). + // Fall back to the nominal time-based purge floor when the log is empty/unavailable. for (const entry of auditStore.getRange({ start: 1, log: options?.nodeId })) { - oldestRetainedAuditTime = entry.localTime ?? entry.version; + oldestRetainedAuditTime = entry.txnLogKey; break; } oldestRetainedAuditTime ??= Date.now() - auditRetention; @@ -3124,14 +3124,14 @@ export function makeTable(options) { // would find it and skip the write as "already applied" when the record was never committed. // A recommit of the same transaction survived that skip only because the old write batch // still carried the put; a fresh-transaction replay (ERR_TRY_AGAIN) would drop the write. - if (isRocksDB && !stagedOwnAuditEntry && dedupVersionCouldBeRetained(logTime)) { - const priorAudit = auditStore.get(logTime, tableId, id, options?.nodeId); + if (isRocksDB && !stagedOwnAuditEntry && dedupVersionCouldBeRetained(txnLogKey)) { + const priorAudit = auditStore.get(txnLogKey, tableId, id, options?.nodeId); if ( priorAudit && - priorAudit.localTime === logTime && + priorAudit.txnLogKey === txnLogKey && precedesExistingVersion( txnTime, - { version: txnTime, localTime: logTime, key: id, nodeId: priorAudit.nodeId }, + { version: txnTime, localTime: txnLogKey, key: id, nodeId: priorAudit.nodeId }, options?.nodeId ) === 0 ) { @@ -3188,14 +3188,14 @@ export function makeTable(options) { // never committed (see the up-front keyed dedup above). const isReDeliveredDuplicate = () => { if (stagedOwnAuditEntry) return false; - if (!dedupVersionCouldBeRetained(logTime)) return false; // pre-retention log key — skip the end-of-log scan (best-effort; see above) - const duplicate = auditStore.get(logTime, tableId, id, options?.nodeId); + if (!dedupVersionCouldBeRetained(txnLogKey)) return false; // pre-retention log key — skip the end-of-log scan (best-effort; see above) + const duplicate = auditStore.get(txnLogKey, tableId, id, options?.nodeId); return ( duplicate && - duplicate.localTime === logTime && + duplicate.txnLogKey === txnLogKey && precedesExistingVersion( txnTime, - { version: txnTime, localTime: logTime, key: id, nodeId: duplicate.nodeId }, + { version: txnTime, localTime: txnLogKey, key: id, nodeId: duplicate.nodeId }, options?.nodeId ) === 0 ); @@ -3272,9 +3272,9 @@ export function makeTable(options) { // value is a LOG key, not a record version: every consumer follows it straight into // `auditStore.get` (see the `auditRefsToVisit` mapping above and below), and on an // applied write those two clocks differ. - additionalAuditRefs.push({ version: logTime, nodeId: options?.nodeId }); + additionalAuditRefs.push({ version: txnLogKey, nodeId: options?.nodeId }); logger.debug?.('Adding additional audit ref for out-of-order write', { - logTime, + txnLogKey, nodeId: options?.nodeId, }); } @@ -4756,7 +4756,7 @@ export function makeTable(options) { const subscription = addSubscription( TableResource, thisId, - function (id: Id, auditRecord?: any, localTime?: any, beginTxn?: any) { + function (id: Id, auditRecord?: any, txnLogKey?: any, beginTxn?: any) { if (dropDuringReplay) return; try { let type = auditRecord.type; @@ -4788,7 +4788,7 @@ export function makeTable(options) { } const event = { id, - localTime, + localTime: txnLogKey, value, version: auditRecord.version, type, @@ -4853,11 +4853,11 @@ export function makeTable(options) { if (auditRecord.tableId !== tableId) continue; const id = auditRecord.recordId; if (thisId == null || isDescendantId(thisId, id)) { - const value = auditRecord.getValue(primaryStore, getFullRecord, auditRecord.localTime); + const value = auditRecord.getValue(primaryStore, getFullRecord, auditRecord.txnLogKey); if ( !send({ id, - localTime: auditRecord.localTime, + localTime: auditRecord.txnLogKey, value, version: auditRecord.version, type: auditRecord.type, @@ -4870,7 +4870,7 @@ export function makeTable(options) { if ((await subscription.waitForDrain()) === false) return; } } - subscription!.startTime = auditRecord.localTime ?? auditRecord.version; // update so we don't double send + subscription!.startTime = auditRecord.txnLogKey; // update so we don't double send } } finally { // replay is done, we can start sending real-time messages again @@ -4902,10 +4902,10 @@ export function makeTable(options) { ); break; } - const value = auditRecord.getValue(primaryStore, getFullRecord, auditRecord.localTime); + const value = auditRecord.getValue(primaryStore, getFullRecord, auditRecord.txnLogKey); const historyEntry = { id, - localTime: auditRecord.localTime, + localTime: auditRecord.txnLogKey, value, version: auditRecord.version, type: auditRecord.type, @@ -4919,7 +4919,7 @@ export function makeTable(options) { if (--count <= 0) break; } } catch (error) { - logger.error?.('Error getting history entry', auditRecord.localTime, error); + logger.error?.('Error getting history entry', auditRecord.txnLogKey, error); } } for (let i = history.length; i > 0;) { @@ -6000,10 +6000,10 @@ export function makeTable(options) { if (auditRecord.tableId !== tableId) continue; yield { id: auditRecord.recordId, - localTime: auditRecord.localTime, + localTime: auditRecord.txnLogKey, version: auditRecord.version, type: auditRecord.type, - value: auditRecord.getValue(primaryStore, true, auditRecord.localTime), + value: auditRecord.getValue(primaryStore, true, auditRecord.txnLogKey), user: auditRecord.user, operation: auditRecord.originatingOperation, }; @@ -6027,12 +6027,12 @@ export function makeTable(options) { if (auditRecord.tableId === tableId && compareKeys(auditRecord.recordId, id) === 0) { history.splice(insertionPoint, 0, { id: auditRecord.recordId, - localTime: auditRecord.localTime, + localTime: auditRecord.txnLogKey, version: auditRecord.version, type: auditRecord.type, // reconstruct each entry's record image as of its own log position, not the audit // window boundary (nextVersion), matching getHistory (issue #1330) - value: auditRecord.getValue(primaryStore, true, auditRecord.localTime), + value: auditRecord.getValue(primaryStore, true, auditRecord.txnLogKey), user: auditRecord.user, operation: auditRecord.originatingOperation, }); diff --git a/resources/auditStore.ts b/resources/auditStore.ts index 060d39ff59..2974576311 100644 --- a/resources/auditStore.ts +++ b/resources/auditStore.ts @@ -34,7 +34,7 @@ initSync(); export type AuditRecord = { version: number; // the record's own version: LWW ordering, @updatedTime, ETag - localTime: number; // log position: LMDB audit-store key; RocksDB transaction-log key + txnLogKey: number; // position in the origin's transaction log type: string; encodedRecord?: Buffer; extendedType?: number; @@ -148,7 +148,7 @@ export function openAuditStore(rootStore) { auditStore.getRange = function (options) { if (options.values === false) return superGetRange(options); // getKeys shouldn't be modified return superGetRange(options).map(({ key, value }) => { - value.key = value.localTime = key; + value.key = value.txnLogKey = key; return value; }); }; @@ -379,7 +379,7 @@ export function openAuditStore(rootStore) { * never the record version: a version is legitimately non-unique, so a version match can name a * different write and authorize destroying live state (a tombstone, a still-referenced blob). * - * On LMDB this is exact — the audit-store key IS the record's `localTime`. On RocksDB a record stores + * On LMDB this is exact — the audit-store key IS the record's `txnLogKey`. On RocksDB a record stores * one word, its version, which equals its log key for every write except a source fill (harper#2065); * a fill therefore answers false here, as the pre-normalization version-versus-log-key compare * already did. Absent identity answers false too. The direction is deliberate: uncertainty retains. @@ -387,8 +387,8 @@ export function openAuditStore(rootStore) { export function isAuditEntryWrite(entry: any, auditRecord: AuditRecord): boolean { return ( entry != null && - auditRecord.localTime != null && - entry.localTime === auditRecord.localTime && + auditRecord.txnLogKey != null && + entry.localTime === auditRecord.txnLogKey && (entry.nodeId ?? 0) === (auditRecord.nodeId ?? 0) ); } diff --git a/resources/replayLogs.ts b/resources/replayLogs.ts index fbe77bd850..7830d98318 100644 --- a/resources/replayLogs.ts +++ b/resources/replayLogs.ts @@ -152,7 +152,7 @@ export function replayLogs(rootStore: RocksDatabase, tables: any, electedReplaye nodeId, recordId, version, - localTime, + txnLogKey, residencyId, expiresAt, originatingOperation, @@ -217,9 +217,9 @@ export function replayLogs(rootStore: RocksDatabase, tables: any, electedReplaye // and each write is replayed at its own stored record version. The two differ for a source // fill, and stamping such a record at the log key would move its version forward and make a // later legitimate write in between look stale (harper#2411). - if (lastTimestamp !== localTime) { + if (lastTimestamp !== txnLogKey) { const torn = entries.corruptFrameStop.truncatedVersions.has(lastTimestamp); - lastTimestamp = localTime; + lastTimestamp = txnLogKey; try { // commit the last transaction since we are starting a new one, unless a corrupt // frame swallowed the rest of it — half of a source transaction must never become @@ -261,7 +261,7 @@ export function replayLogs(rootStore: RocksDatabase, tables: any, electedReplaye } transaction = new DatabaseTransaction(); transaction.db = primaryStore; - transaction.timestamp = localTime; + transaction.timestamp = txnLogKey; // retries=1 routes operation.commit() through its retry path (no duplicate audit staging) transaction.retries = 1; // Explicit replay marker: skips schema validation (harper#1316) and makes save() stamp diff --git a/resources/transactionBroadcast.ts b/resources/transactionBroadcast.ts index dab4b9a201..b5bff1fbf7 100644 --- a/resources/transactionBroadcast.ts +++ b/resources/transactionBroadcast.ts @@ -95,7 +95,7 @@ export function addSubscription(table, key, listener?: (key) => any, startTime?: * subscription and get the initial state. */ class Subscription extends IterableEventQueue { - listener: (recordId: Id, auditEntry: any, localTime: number, beginTxn: boolean) => void; + listener: (recordId: Id, auditEntry: any, txnLogKey: number, beginTxn: boolean) => void; subscriptions: any; startTime?: number; includeDescendants?: boolean; @@ -178,10 +178,10 @@ function notifyFromTransactionData(subscriptions, auditLogIterable?, allowYield } if (result.done) break; const auditRecord = result.value; - const timestamp: number = auditRecord.localTime ?? auditRecord.version; + const timestamp: number = auditRecord.txnLogKey; subscriptions.lastTxnTime = timestamp; // the transaction extent: RocksDB entries committed together share the log key (record - // versions may differ); LMDB's localTime is a per-entry audit key, so version delimits there + // versions may differ); LMDB's transaction-log key is per-entry, so version delimits there const txnKey = auditStore.reusableIterable ? timestamp : auditRecord.version; if (ACTIONS_OF_INTEREST.includes(auditRecord.type)) { const tableSubscriptions = subscriptions[auditRecord.tableId]; diff --git a/unitTests/resources/auditEntryRecordFlags.test.js b/unitTests/resources/auditEntryRecordFlags.test.js index 92717f8b43..568840720d 100644 --- a/unitTests/resources/auditEntryRecordFlags.test.js +++ b/unitTests/resources/auditEntryRecordFlags.test.js @@ -130,10 +130,10 @@ describe('Audit entry record flags match the body (#2153)', () => { for (const entry of T.auditStore.getRange({ start: 1 })) { // keyed by log position: an audit-only commit records the surviving (newer) record version // in its body, so only the log key identifies the superseded write's entry - if (entry.localTime === loserVersion) loser = entry; + if (entry.txnLogKey === loserVersion) loser = entry; // every minted entry must be internally consistent: record flags imply a body if (entry.extendedType & (HAS_RECORD | HAS_PARTIAL_RECORD)) { - assert(entry.getBinaryValue().length > 0, `entry ${entry.localTime} advertises a record but has no body`); + assert(entry.getBinaryValue().length > 0, `entry ${entry.txnLogKey} advertises a record but has no body`); } } assert(loser, 'audit entry for the superseded write should exist'); diff --git a/unitTests/resources/auditLog.test.js b/unitTests/resources/auditLog.test.js index be2433b021..ecdb992edd 100644 --- a/unitTests/resources/auditLog.test.js +++ b/unitTests/resources/auditLog.test.js @@ -766,7 +766,7 @@ describe('Audit log', () => { tableId: 7, recordId: 'orphan', version: 42, - localTime: 42, + txnLogKey: 42, nodeId: 0, key: 'audit-key', }; @@ -817,7 +817,7 @@ describe('Audit log', () => { tableId: 7, recordId: 'r', version: 42, - localTime: 100, + txnLogKey: 100, nodeId: 0, key: 'audit-key', }); @@ -831,7 +831,7 @@ describe('Audit log', () => { tableId: 7, recordId: 'r', version: 7, - localTime: 100, + txnLogKey: 100, nodeId: 0, key: 'audit-key', }); @@ -845,7 +845,7 @@ describe('Audit log', () => { tableId: 7, recordId: 'r', version: 42, - localTime: 100, + txnLogKey: 100, nodeId: 5, key: 'audit-key', }); @@ -912,8 +912,8 @@ describe('Audit log', () => { // key (localTime on LMDB; RocksDB's version field already *is* its key), not a 0/1 placeholder // flag substituted from a shared per-environment register. Resolving it through the audit // store proves the chain is walkable, not just numerically similar. - assert.equal(entries[1].previousVersion, entries[0].localTime ?? entries[0].version); - assert.equal(entries[2].previousVersion, entries[1].localTime ?? entries[1].version); + assert.equal(entries[1].previousVersion, entries[0].txnLogKey); + assert.equal(entries[2].previousVersion, entries[1].txnLogKey); assert( AuditedTable.auditStore.get(entries[1].previousVersion, AuditedTable.tableId, id), 'previousVersion must resolve back to the actual prior audit entry' @@ -1380,8 +1380,8 @@ describe('Audit log', () => { const timestamps = []; assert.doesNotThrow(() => { for (const record of store.getRange({})) { - // localTime is the log key: these synthetic entries carry no decodable record version - timestamps.push(record.localTime); + // txnLogKey is the log key: these synthetic entries carry no decodable record version + timestamps.push(record.txnLogKey); } }, 'aggregate iteration must not propagate the corrupt-entry RangeError'); diff --git a/unitTests/resources/blob.test.js b/unitTests/resources/blob.test.js index 72b431b72d..41629b0306 100644 --- a/unitTests/resources/blob.test.js +++ b/unitTests/resources/blob.test.js @@ -1240,7 +1240,7 @@ describe('Blob test', () => { recordId: id, type: 'put', version: entry.version, - localTime: entry.localTime + 1, + txnLogKey: entry.localTime + 1, nodeId: entry.nodeId ?? 0, getValue: () => ({ blob }), }, diff --git a/unitTests/resources/dualClockAuditRecord.test.js b/unitTests/resources/dualClockAuditRecord.test.js index bf48f57537..4d3a35ea15 100644 --- a/unitTests/resources/dualClockAuditRecord.test.js +++ b/unitTests/resources/dualClockAuditRecord.test.js @@ -1,7 +1,7 @@ // harper#2412 stage 0b: an audit record carries two clocks, and they must not be confused. // // version — the record's own version: LWW ordering, @updatedTime, ETag. Legitimately non-unique. -// localTime — this entry's key in the per-origin transaction log. Write identity, resume cursor. +// txnLogKey — this entry's key in the per-origin transaction log. Write identity, resume cursor. // // They hold the same value for a write whose record version is its own commit timestamp, which is // every ordinary local write — so a test that only writes ordinary records cannot tell the two @@ -30,7 +30,7 @@ describe('Dual-clock audit records (harper#2412)', () => { entries.push({ type: auditRecord.type, version: auditRecord.version, - localTime: auditRecord.localTime, + txnLogKey: auditRecord.txnLogKey, }); } return entries; @@ -82,7 +82,7 @@ describe('Dual-clock audit records (harper#2412)', () => { const [entry] = auditEntriesFor(Plain, id); assert.ok(entry, 'the write must have produced an audit entry'); assert.equal(entry.version, Plain.primaryStore.getEntry(id).version, 'version is the record version'); - assert.equal(entry.localTime, entry.version, 'a locally-originated write commits at its own version'); + assert.equal(entry.txnLogKey, entry.version, 'a locally-originated write commits at its own version'); }); it('a source fill records the source version and the fill commit as separate clocks', async function () { @@ -96,8 +96,8 @@ describe('Dual-clock audit records (harper#2412)', () => { assert.ok(entry, 'the fill must have produced an audit entry'); assert.equal(entry.version, reportedVersion, 'the audit record carries the record version'); assert.ok( - entry.localTime > reportedVersion, - `the log key is the fill's commit, not its version (localTime ${entry.localTime}, version ${entry.version})` + entry.txnLogKey > reportedVersion, + `the log key is the fill's commit, not its version (txnLogKey ${entry.txnLogKey}, version ${entry.version})` ); }); @@ -110,7 +110,19 @@ describe('Dual-clock audit records (harper#2412)', () => { assert.equal(Plain.primaryStore.getEntry(id).version, version, 'the peer stores the origin record version'); const [entry] = auditEntriesFor(Plain, id); assert.equal(entry.version, version, 'the audit record carries the origin record version'); - assert.equal(entry.localTime, logKey, "the peer's log key for this write is the origin's log key"); + assert.equal(entry.txnLogKey, logKey, "the peer's log key for this write is the origin's log key"); + }); + + it('bounds an overloaded audit-body version by the originating transaction-log key', async function () { + if (isLMDB) return this.skip(); + const id = 'applied-out-of-order-1'; + const logKey = Date.now(); + const survivingVersion = logKey + 30_000; + await applyFromOrigin(Plain, id, { id, name: 'from-origin' }, { logKey, version: survivingVersion }); + assert.equal(Plain.primaryStore.getEntry(id).version, logKey); + const [entry] = auditEntriesFor(Plain, id); + assert.equal(entry.version, logKey); + assert.equal(entry.txnLogKey, logKey); }); it('one applied transaction can carry writes at different record versions', async function () { @@ -140,8 +152,10 @@ describe('Dual-clock audit records (harper#2412)', () => { const [newEntry] = auditEntriesFor(Plain, 'batched-new'); assert.equal(oldEntry.version, olderVersion); assert.equal(newEntry.version, logKey); - assert.equal(oldEntry.localTime, logKey, 'both share the transaction log key'); - assert.equal(newEntry.localTime, logKey); + assert.equal(oldEntry.txnLogKey, logKey, 'both share the transaction log key'); + assert.equal(newEntry.txnLogKey, logKey); + const foundSecond = auditStore.get(logKey, Plain.tableId, 'batched-new', 0); + assert.equal(foundSecond?.recordId, 'batched-new', 'lookup scans every entry sharing the transaction log key'); }); it('an ordinary local write ignores a record version it was not given', async function () { @@ -150,7 +164,7 @@ describe('Dual-clock audit records (harper#2412)', () => { const id = 'plain-2'; await Plain.put(id, { id, name: 'still-local' }); const [entry] = auditEntriesFor(Plain, id); - assert.equal(entry.localTime, entry.version); + assert.equal(entry.txnLogKey, entry.version); }); it('delivers a subscriber event whose version is the record version and localTime the log position', async function () { @@ -187,7 +201,7 @@ describe('Dual-clock audit records (harper#2412)', () => { assert.equal(tombstone.localTime, tombstone.version, 'a locally-written record stores one word'); const [, deleteEntry] = auditEntriesFor(Plain, id); assert.equal(deleteEntry.type, 'delete'); - assert.equal(deleteEntry.localTime, tombstone.localTime, "the audit entry names the tombstone's write"); + assert.equal(deleteEntry.txnLogKey, tombstone.localTime, "the audit entry names the tombstone's write"); await Plain.deleteHistory(Date.now() + 60_000); assert.equal(Plain.primaryStore.getEntry(id), undefined, 'the tombstone must be removed with its entry'); }); @@ -249,7 +263,7 @@ describe('Dual-clock audit records on LMDB (harper#2412)', () => { assert.ok(entry, 'the applied write must have produced an audit entry'); assert.equal(entry.version, version, 'the audit entry carries the origin record version'); assert.equal( - entry.localTime, + entry.txnLogKey, Applied.primaryStore.getEntry(id).localTime, "on LMDB the audit key is the receiver's own local time for the record, not the origin's log key" ); diff --git a/unitTests/resources/transactionBroadcastGrouping.test.js b/unitTests/resources/transactionBroadcastGrouping.test.js index a0d0bccbf0..ec71ff2197 100644 --- a/unitTests/resources/transactionBroadcastGrouping.test.js +++ b/unitTests/resources/transactionBroadcastGrouping.test.js @@ -1,6 +1,6 @@ // Transaction delimiting in the broadcaster is engine-aware: RocksDB entries committed together // share the log key while their record versions may differ (a source fill's version can be a -// source-reported lastModified); LMDB's localTime is a per-entry audit key, so there the shared +// source-reported lastModified); LMDB's transaction-log key is per-entry, so there the shared // version delimits the transaction. These drive the real same-thread aftercommit path. require('../testUtils'); const assert = require('node:assert'); @@ -39,9 +39,9 @@ describe('transactionBroadcast transaction grouping', () => { try { const logKey = Date.now(); table.auditStore.emit('aftercommit', [ - { type: 'put', tableId: 1, recordId: 'a', version: logKey, localTime: logKey }, + { type: 'put', tableId: 1, recordId: 'a', version: logKey, txnLogKey: logKey }, // a source fill in the same commit: backdated record version, same log key - { type: 'put', tableId: 1, recordId: 'b', version: logKey - 5000, localTime: logKey }, + { type: 'put', tableId: 1, recordId: 'b', version: logKey - 5000, txnLogKey: logKey }, ]); await waitFor(() => events.length === 3, { message: 'both entries and the end_txn should be delivered' }); assert.deepEqual(events, [ @@ -60,8 +60,8 @@ describe('transactionBroadcast transaction grouping', () => { try { const version = Date.now(); table.auditStore.emit('aftercommit', [ - { type: 'put', tableId: 1, recordId: 'c', version, localTime: version + 1 }, - { type: 'put', tableId: 1, recordId: 'd', version, localTime: version + 2 }, + { type: 'put', tableId: 1, recordId: 'c', version, txnLogKey: version + 1 }, + { type: 'put', tableId: 1, recordId: 'd', version, txnLogKey: version + 2 }, ]); await waitFor(() => events.length === 3, { message: 'both entries and the end_txn should be delivered' }); assert.deepEqual(events, [ From 09e0008272e3d77080971a64fb9dac7feeb5c2f0 Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Thu, 3 Sep 2026 23:45:13 -0600 Subject: [PATCH 03/16] Fix audit-walk duplicate identity --- resources/Table.ts | 12 ++++++++++ unitTests/resources/transaction.test.js | 31 +++++++++++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/resources/Table.ts b/resources/Table.ts index 29abb38daf..beadb7486c 100644 --- a/resources/Table.ts +++ b/resources/Table.ts @@ -3212,6 +3212,18 @@ export function makeTable(options) { } const auditRecord = auditStore.get(localTime, tableId, id, nodeId); if (!auditRecord) break; + if ( + !stagedOwnAuditEntry && + localTime === txnLogKey && + precedesExistingVersion( + txnTime, + { version: txnTime, localTime: txnLogKey, key: id, nodeId: auditRecord.nodeId }, + options?.nodeId + ) === 0 + ) { + write.skipped = true; + return; + } auditedVersion = auditRecord.version; if (auditedVersion >= txnTime) { if (auditedVersion === txnTime) { diff --git a/unitTests/resources/transaction.test.js b/unitTests/resources/transaction.test.js index 38dd5024ca..4958dfc5ef 100644 --- a/unitTests/resources/transaction.test.js +++ b/unitTests/resources/transaction.test.js @@ -675,6 +675,37 @@ describe('Transactions', () => { assert.equal(capWarned, false, 're-delivery should be deduped up front, not via the deep walk'); }); + it('deduplicates an audit-only write in the walk when the keyed lookup misses', async function () { + if (isLMDB) return; + const id = 1114005; + const base = Date.now() + 250_000_000; + const duplicateKey = base + 50; + await TxnTest.put(id, { count: 0 }, { timestamp: base }); + await TxnTest.patch(id, { seq: 1 }, { timestamp: base + 100 }); + const duplicate = { count: { __op__: 'add', value: 1 } }; + await TxnTest.patch(id, duplicate, { timestamp: duplicateKey }); + await TxnTest.patch(id, { seq: 2 }, { timestamp: base + 200 }); + assert.equal((await TxnTest.get(id)).count, 1); + + const auditStore = TxnTest.auditStore; + const originalGet = auditStore.get; + let missedKeyedLookup = false; + auditStore.get = (key, ...args) => { + if (key === duplicateKey && !missedKeyedLookup) { + missedKeyedLookup = true; + return; + } + return originalGet.call(auditStore, key, ...args); + }; + try { + await TxnTest.patch(id, duplicate, { timestamp: duplicateKey }); + } finally { + auditStore.get = originalGet; + } + assert(missedKeyedLookup, 'the up-front keyed lookup must miss so the audit walk owns dedup'); + assert.equal((await TxnTest.get(id)).count, 1, 'the walk must not reapply the commutative operation'); + }); + // #1114/#1316: an out-of-order write whose every field is overwritten by newer in-order patches is // fully superseded. The fold at the end of the walk already drops it (writeCommit(false)) — but only // after walking the whole chain. The early-out folds as it walks and escapes the moment the residual From 07c0688803c681ae7691719a852bfd8afad60e28 Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Fri, 4 Sep 2026 00:04:33 -0600 Subject: [PATCH 04/16] Preserve the dual-clock audit head --- resources/DESIGN.md | 17 +++-- resources/LMDBTransaction.ts | 8 +-- resources/Table.ts | 72 +++++++++++++++---- .../resources/dualClockAuditRecord.test.js | 71 +++++++++++++++--- 4 files changed, 132 insertions(+), 36 deletions(-) diff --git a/resources/DESIGN.md b/resources/DESIGN.md index 746ca9740f..c3342a043c 100644 --- a/resources/DESIGN.md +++ b/resources/DESIGN.md @@ -113,8 +113,9 @@ been distinct fields; on RocksDB the read surface used to overwrite `version` wi They hold the same value for every write whose record version is its own commit timestamp, which is every ordinary local write, so a bug that confuses them stays invisible until a **source fill** (`getFromSource`, core #2065): the record is stored at the source-reported version while its log -entry is keyed at the fill's commit. A record stores one word today, so it keeps its version and the -first-word == log-key invariant is restored in stage 2, not here. +entry is keyed at the fill's commit. A RocksDB record stores its version in the existing word and, +when the clocks diverge, keeps its audit head in `additionalAuditRefs`; stage 2 can replace that +compatibility pointer with a dedicated log-key word. Consequences worth knowing: @@ -122,18 +123,20 @@ Consequences worth knowing: single predicate; `removeAuditEntry`'s tombstone removal and `blob.ts`'s orphan sweep both gate on it, and both retain rather than delete when identity is unknown. A version compare there would let one write authorize destroying another's tombstone or blob. -- **An applied write carries its own record version.** `TransactionWrite.recordVersion`, set from +- **A RocksDB applied write carries its own record version.** `TransactionWrite.recordVersion`, set from `options.version` by every `_write*` builder and read in `save()` only when the transaction is `sourceApply` or `isReplay`, is how a replication receiver stores the origin's version while the transaction commits under the origin's log key — so a peer's copy of an origin's log stays in the origin's clock. `getAppliedWriteVersion` bounds the body value by `txnLogKey`: a source fill keeps its earlier source version, while an out-of-order audit body cannot restamp its originating write at the later surviving version. One frame can carry writes at different record versions, so this - cannot be a per-transaction value. + cannot be a per-transaction value. Deprecated LMDB keeps its legacy transaction-version apply + behavior. - **`additionalAuditRefs[].version` is a log key, not a version.** Every consumer follows it straight - into `auditStore.get` (`Table.ts`'s `auditRefsToVisit`), so the out-of-order walk records the write's - `txnLogKey` there. Under stage 2 the stored reference field can be renamed; until then, - "fixing" it to the record version silently unaddresses the entry it points at. + into `auditStore.get` (`Table.ts`'s `auditRefsToVisit`). The list carries folded out-of-order + branches and, when a RocksDB record version differs from its log key, the record's own audit head. + Under stage 2 the stored reference field can be renamed; until then, "fixing" it to the record + version silently unaddresses the entry it points at. - **Crash replay uses both.** `replayLogs` delimits transactions by `txnLogKey` (which is also what `CorruptFrameStop.truncatedVersions` records) and replays each write at its stored `version`. Stamping a replayed record at its log key would move its version forward and make a later diff --git a/resources/LMDBTransaction.ts b/resources/LMDBTransaction.ts index e006ec9a1d..198ee877df 100644 --- a/resources/LMDBTransaction.ts +++ b/resources/LMDBTransaction.ts @@ -1,6 +1,5 @@ import { DatabaseTransaction, - getAppliedWriteVersion, shouldSpareCommitPhase, transactionOpenTooLongError, type CommitOptions, @@ -132,8 +131,6 @@ export class LMDBTransaction extends DatabaseTransaction { if (!txnTime) txnTime = this.timestamp = options.timestamp || getNextMonotonicTime(); if (!options.timestamp) options.timestamp = txnTime; const retries = options.retries || 0; - const writeVersion = (write: TransactionWrite) => - this.sourceApply || this.isReplay ? getAppliedWriteVersion(write.recordVersion, txnTime) : txnTime; // now validate if (this.validated < this.writes.length) { try { @@ -143,7 +140,7 @@ export class LMDBTransaction extends DatabaseTransaction { this.validated = this.writes.length; for (let i = start; i < this.validated; i++) { const write = this.writes[i]; - write?.validate?.(writeVersion(write), this); + write?.validate?.(this.timestamp, this); } let hasBefore; for (let i = start; i < this.validated; i++) { @@ -206,8 +203,7 @@ export class LMDBTransaction extends DatabaseTransaction { let writeIndex = 0; this.writes = this.writes.filter((write) => write); // filter out removed entries const doWrite = (write) => { - // see DatabaseTransaction.save(): an applied write carries the origin's record version - const completion = write.commit(writeVersion(write), write.entry, retries); + const completion = write.commit(txnTime, write.entry, retries); if (typeof completion?.then === 'function') { // the aggregating Promise.all is attached a turn or more later (after the conditional batch // or the exclusive transaction resolves), so handle rejection here to keep the gap from diff --git a/resources/Table.ts b/resources/Table.ts index beadb7486c..a022388078 100644 --- a/resources/Table.ts +++ b/resources/Table.ts @@ -711,6 +711,20 @@ export function makeTable(options) { }, }); } + function resolveAuditHead( + id: Id, + version: number | undefined, + nodeId: number | undefined, + refs?: Array<{ version: number; nodeId: number }> + ) { + if (refs) { + for (const ref of refs) { + const auditRecord = auditStore.getSync(ref.version, tableId, id, ref.nodeId); + if (auditRecord?.version === version) return { txnLogKey: ref.version, nodeId: ref.nodeId }; + } + } + return { txnLogKey: version, nodeId }; + } class TableResource extends Resource { #record: any; // the stored/frozen record from the database and stored in the cache (should not be modified directly) #changes: any; // the changes to the record that have been made (should not be modified directly) @@ -2993,6 +3007,8 @@ export function makeTable(options) { this.#savingOperation = null; write.stagedIn = undefined; // nothing may pin this write's transaction past its commit let omitLocalRecord = false; + const txnLogKey = + isRocksDB && options?.version != null ? (transaction?.getTimestamp?.() ?? txnTime) : txnTime; // we use optimistic locking to only commit if the existing record state still holds true. // this is superior to using an async transaction since it doesn't require JS execution // during the write transaction. @@ -3040,12 +3056,6 @@ export function makeTable(options) { // of the updates to the record to ensure consistency across the cluster // TODO: can the previous version be older, but even more previous version be newer? if (audit) { - // This write's key in the per-origin transaction log: the transaction's own timestamp, - // which a replication apply or a replay adopts from the origin. It is what the keyed - // dedup below looks up — never the record version, which a source fill sets from the - // source and which is legitimately non-unique. Resolved here rather than at the top of - // the commit so an in-order write, the overwhelming majority, never pays the call. - const txnLogKey = transaction?.getTimestamp?.() ?? txnTime; // A re-delivered out-of-order write (full-copy audit-replay re-delivers writes) must not have // its commutative ops re-folded. additionalAuditRefs is the record's own list of folded // out-of-order versions, read with read-your-writes consistency, so this skips the duplicate up @@ -3213,6 +3223,7 @@ export function makeTable(options) { const auditRecord = auditStore.get(localTime, tableId, id, nodeId); if (!auditRecord) break; if ( + isRocksDB && !stagedOwnAuditEntry && localTime === txnLogKey && precedesExistingVersion( @@ -3507,6 +3518,14 @@ export function makeTable(options) { ); updateIndices(id, existingRecord, recordToStore, transaction && { transaction }); + // Preserve an addressable audit head when the record and log clocks diverge. + if (isRocksDB && audit && !isCopyApply && txnLogKey !== txnTime) { + const headIndex = additionalAuditRefs.findIndex( + (ref) => ref.version === txnLogKey && (ref.nodeId ?? 0) === (options?.nodeId ?? 0) + ); + if (headIndex > 0) additionalAuditRefs.unshift(additionalAuditRefs.splice(headIndex, 1)[0]); + else if (headIndex < 0) additionalAuditRefs.unshift({ version: txnLogKey, nodeId: options?.nodeId }); + } writeCommit(true); if (write.trackRecordVersion) write.recordVersionApplied = true; if (expiresAt >= 0) { @@ -5011,6 +5030,12 @@ export function makeTable(options) { logger.trace?.('re-retrieved record', localTime, this.#entry?.localTime); localTime = entry?.localTime; } + let nodeId = entry?.nodeId; + if (isRocksDB && entry) { + const head = resolveAuditHead(thisId, entry.version, nodeId, entry.additionalAuditRefs); + localTime = head.txnLogKey; + nodeId = head.nodeId; + } logger.trace?.('Subscription from', startTime, 'from', thisId, localTime); if (startTime < localTime) { // start time specified, get the audit history for this record. Set startTime up @@ -5021,7 +5046,6 @@ export function makeTable(options) { const history = []; let inspected = 0; let nextTime = localTime; - let nodeId = entry?.nodeId; do { if (++recordsSinceYield >= REPLAY_YIELD_INTERVAL) { recordsSinceYield = 0; @@ -5046,8 +5070,16 @@ export function makeTable(options) { if (count) count--; } else if (!isActive()) return; } - nextTime = auditRecord.previousVersion; - nodeId = auditRecord.previousNodeId; + const previousHead = isRocksDB + ? resolveAuditHead( + thisId, + auditRecord.previousVersion, + auditRecord.previousNodeId, + auditRecord.previousAdditionalAuditRefs + ) + : { txnLogKey: auditRecord.previousVersion, nodeId: auditRecord.previousNodeId }; + nextTime = previousHead.txnLogKey; + nodeId = previousHead.nodeId; } else break; } while (nextTime > startTime && count !== 0); for (let i = history.length; i > 0;) { @@ -6026,7 +6058,9 @@ export function makeTable(options) { if (id == undefined) throw new Error('An id is required'); const entry = primaryStore.getEntry(id); if (!entry) return history; - let nextVersion = entry.localTime; + let nextVersion = isRocksDB + ? resolveAuditHead(id, entry.version, entry.nodeId, entry.additionalAuditRefs).txnLogKey + : entry.localTime; if (!nextVersion) throw new Error('The entry does not have a local audit time'); const count = 0; const auditWindow = 100; @@ -6048,8 +6082,16 @@ export function makeTable(options) { user: auditRecord.user, operation: auditRecord.originatingOperation, }); - if (auditRecord.previousVersion > highestPreviousVersion && auditRecord.previousVersion < start) { - highestPreviousVersion = auditRecord.previousVersion; + const previousVersion = isRocksDB + ? resolveAuditHead( + id, + auditRecord.previousVersion, + auditRecord.previousNodeId, + auditRecord.previousAdditionalAuditRefs + ).txnLogKey + : auditRecord.previousVersion; + if (previousVersion > highestPreviousVersion && previousVersion < start) { + highestPreviousVersion = previousVersion; } } } @@ -6928,6 +6970,7 @@ export function makeTable(options) { const currentRecord = existingEntry?.value; const recordVersion = isRocksDB && racedVersion != null ? Math.max(sourceVersion, racedVersion) : sourceVersion; + const txnLogKey = isRocksDB ? transaction?.getTimestamp?.() : recordVersion; updateIndices(id, currentRecord, updatedRecord, transaction && { transaction }); if (updatedRecord) { if (existingEntry) { @@ -6989,19 +7032,22 @@ export function makeTable(options) { `Writing resolved record from source with id: ${id}, timestamp: ${new Date(recordVersion).toISOString()}` ); // TODO: We are doing a double check for ifVersion that should probably be cleaned out + const writeAudit = (audit && (hasChanges || omitLocalRecord)) || null; updateRecord( id, updatedRecord, existingEntry, recordVersion, omitLocalRecord ? INVALIDATED : 0, - (audit && (hasChanges || omitLocalRecord)) || null, + writeAudit, { user: (sourceContext as any)?.user, expiresAt: sourceContext.expiresAt, residencyId, transaction, tableToTrack: tableName, + additionalAuditRefs: + writeAudit && txnLogKey !== recordVersion ? [{ version: txnLogKey, nodeId: 0 }] : undefined, }, 'put', Boolean(invalidated), diff --git a/unitTests/resources/dualClockAuditRecord.test.js b/unitTests/resources/dualClockAuditRecord.test.js index 4d3a35ea15..1add698561 100644 --- a/unitTests/resources/dualClockAuditRecord.test.js +++ b/unitTests/resources/dualClockAuditRecord.test.js @@ -38,11 +38,21 @@ describe('Dual-clock audit records (harper#2412)', () => { // The receive path: the apply transaction commits under the origin's log key while each write // stores the origin's record version (Table.ts's apply dispatcher -> options.version). - function applyFromOrigin(TableClass, id, record, { logKey, version, nodeId = 1 }) { + function applyFromOrigin( + TableClass, + id, + record, + { logKey, version, nodeId = 1, fullUpdate = true, isCopyApply = false } + ) { const context = { source: {}, sourceApply: true, timestamp: logKey }; return transaction(context, async () => { const resource = await TableClass.getResource(id, context); - return resource._writeUpdate(id, record, true, { isNotification: true, nodeId, version }); + return resource._writeUpdate(id, record, fullUpdate, { + isNotification: true, + isCopyApply, + nodeId, + version, + }); }); } @@ -99,6 +109,12 @@ describe('Dual-clock audit records (harper#2412)', () => { entry.txnLogKey > reportedVersion, `the log key is the fill's commit, not its version (txnLogKey ${entry.txnLogKey}, version ${entry.version})` ); + assert( + Filled.primaryStore + .getEntry(id) + .additionalAuditRefs?.some((ref) => ref.version === entry.txnLogKey && ref.nodeId === 0), + 'the stored record keeps an addressable pointer to its audit head' + ); }); it('an applied write keeps the origin version and takes the origin log key', async function () { @@ -113,6 +129,44 @@ describe('Dual-clock audit records (harper#2412)', () => { assert.equal(entry.txnLogKey, logKey, "the peer's log key for this write is the origin's log key"); }); + it('keeps a log-key pointer to the audit head when the stored version differs', async function () { + if (isLMDB) return this.skip(); + const id = 'applied-head-1'; + const logKey = Date.now() + 10; + const version = logKey - 30_000; + await applyFromOrigin(Plain, id, { id, name: 'newer' }, { logKey, version, nodeId: 0 }); + const head = Plain.primaryStore.getEntry(id); + assert(head.additionalAuditRefs?.some((ref) => ref.version === logKey && ref.nodeId === 0)); + assert((await Plain.getHistoryOfRecord(id)).some((entry) => entry.localTime === logKey)); + + await applyFromOrigin( + Plain, + id, + { name: 'older', count: { __op__: 'add', value: 1 } }, + { logKey: logKey + 1, version: version - 1, nodeId: 0, fullUpdate: false } + ); + assert.deepEqual(await Plain.get(id), { id, name: 'newer' }); + }); + + it('does not point a copy-applied record at an audit entry that was never written', async function () { + if (isLMDB) return this.skip(); + const id = 'copy-head-1'; + const logKey = Date.now() + 11; + await applyFromOrigin( + Plain, + id, + { id, name: 'copied' }, + { + logKey, + version: logKey - 30_000, + nodeId: 0, + isCopyApply: true, + } + ); + assert.equal(Plain.primaryStore.getEntry(id).additionalAuditRefs, undefined); + assert.equal(auditStore.get(logKey, Plain.tableId, id, 0), undefined); + }); + it('bounds an overloaded audit-body version by the originating transaction-log key', async function () { if (isLMDB) return this.skip(); const id = 'applied-out-of-order-1'; @@ -136,13 +190,13 @@ describe('Dual-clock audit records (harper#2412)', () => { const older = await Plain.getResource('batched-old', context); await older._writeUpdate('batched-old', { id: 'batched-old', name: 'a' }, true, { isNotification: true, - nodeId: 1, + nodeId: 0, version: olderVersion, }); const current = await Plain.getResource('batched-new', context); await current._writeUpdate('batched-new', { id: 'batched-new', name: 'b' }, true, { isNotification: true, - nodeId: 1, + nodeId: 0, version: logKey, }); }); @@ -222,9 +276,6 @@ describe('Dual-clock audit records (harper#2412)', () => { }); }); -// LMDB has carried the two clocks in separate fields all along, so nothing here is normalized — but the -// per-write record version added for the receive path runs through LMDBTransaction's own commit loop, -// and that branch would otherwise ship untested. describe('Dual-clock audit records on LMDB (harper#2412)', () => { let Applied; @@ -240,7 +291,7 @@ describe('Dual-clock audit records on LMDB (harper#2412)', () => { }); }); - it('stores the origin record version on an applied write and keys the entry by its own audit time', async function () { + it('keeps legacy transaction-version semantics while exposing the audit key as txnLogKey', async function () { if (!isLMDB) return this.skip(); const id = 'lmdb-applied-1'; const originLogKey = Date.now(); @@ -254,14 +305,14 @@ describe('Dual-clock audit records on LMDB (harper#2412)', () => { version, }); }); - assert.equal(Applied.primaryStore.getEntry(id).version, version, 'the peer stores the origin record version'); + assert.equal(Applied.primaryStore.getEntry(id).version, originLogKey); const auditStore = Applied.primaryStore.rootStore.auditStore; let entry; for (const auditRecord of auditStore.getRange({ start: 1 })) { if (auditRecord.tableId === Applied.tableId && auditRecord.recordId === id) entry = auditRecord; } assert.ok(entry, 'the applied write must have produced an audit entry'); - assert.equal(entry.version, version, 'the audit entry carries the origin record version'); + assert.equal(entry.version, originLogKey); assert.equal( entry.txnLogKey, Applied.primaryStore.getEntry(id).localTime, From 4c808358af45c20209d004c3ec9d1f5b1ff14a1e Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Fri, 4 Sep 2026 08:56:04 -0600 Subject: [PATCH 05/16] Preserve audit head and LMDB history semantics --- resources/Table.ts | 23 +++++++++++++++---- .../resources/dualClockAuditRecord.test.js | 8 ++++++- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/resources/Table.ts b/resources/Table.ts index a022388078..f7928e56ee 100644 --- a/resources/Table.ts +++ b/resources/Table.ts @@ -717,12 +717,24 @@ export function makeTable(options) { nodeId: number | undefined, refs?: Array<{ version: number; nodeId: number }> ) { - if (refs) { - for (const ref of refs) { + const visited = new Set(); + function findHead(candidateRefs?: Array<{ version: number; nodeId: number }>) { + if (!candidateRefs) return; + for (const ref of candidateRefs) { + const identity = `${ref.nodeId ?? 0}:${ref.version}`; + if (visited.has(identity)) continue; + visited.add(identity); const auditRecord = auditStore.getSync(ref.version, tableId, id, ref.nodeId); - if (auditRecord?.version === version) return { txnLogKey: ref.version, nodeId: ref.nodeId }; + if (!auditRecord) continue; + // An audit-only out-of-order entry carries the surviving record version in its body. + // Its previous refs still identify the real head, so prefer them over the fold entry. + const previousHead = findHead(auditRecord.previousAdditionalAuditRefs); + if (previousHead) return previousHead; + if (auditRecord.version === version) return { txnLogKey: ref.version, nodeId: ref.nodeId }; } } + const referencedHead = findHead(refs); + if (referencedHead) return referencedHead; return { txnLogKey: version, nodeId }; } class TableResource extends Resource { @@ -6044,7 +6056,8 @@ export function makeTable(options) { if (auditRecord.tableId !== tableId) continue; yield { id: auditRecord.recordId, - localTime: auditRecord.txnLogKey, + // Compatibility-facing LMDB history has always reported/grouped by record version. + localTime: isRocksDB ? auditRecord.txnLogKey : auditRecord.version, version: auditRecord.version, type: auditRecord.type, value: auditRecord.getValue(primaryStore, true, auditRecord.txnLogKey), @@ -6073,7 +6086,7 @@ export function makeTable(options) { if (auditRecord.tableId === tableId && compareKeys(auditRecord.recordId, id) === 0) { history.splice(insertionPoint, 0, { id: auditRecord.recordId, - localTime: auditRecord.txnLogKey, + localTime: isRocksDB ? auditRecord.txnLogKey : auditRecord.version, version: auditRecord.version, type: auditRecord.type, // reconstruct each entry's record image as of its own log position, not the audit diff --git a/unitTests/resources/dualClockAuditRecord.test.js b/unitTests/resources/dualClockAuditRecord.test.js index 1add698561..81bdc2c97c 100644 --- a/unitTests/resources/dualClockAuditRecord.test.js +++ b/unitTests/resources/dualClockAuditRecord.test.js @@ -143,9 +143,13 @@ describe('Dual-clock audit records (harper#2412)', () => { Plain, id, { name: 'older', count: { __op__: 'add', value: 1 } }, - { logKey: logKey + 1, version: version - 1, nodeId: 0, fullUpdate: false } + { logKey: logKey + 1_000, version: version - 1, nodeId: 0, fullUpdate: false } ); assert.deepEqual(await Plain.get(id), { id, name: 'newer' }); + assert( + (await Plain.getHistoryOfRecord(id)).some((entry) => entry.localTime === logKey), + 'an audit-only fold must not displace the real head of the surviving record' + ); }); it('does not point a copy-applied record at an audit entry that was never written', async function () { @@ -313,6 +317,8 @@ describe('Dual-clock audit records on LMDB (harper#2412)', () => { } assert.ok(entry, 'the applied write must have produced an audit entry'); assert.equal(entry.version, originLogKey); + const [historyEntry] = await Applied.getHistoryOfRecord(id); + assert.equal(historyEntry.localTime, originLogKey, 'legacy history continues to report the record version'); assert.equal( entry.txnLogKey, Applied.primaryStore.getEntry(id).localTime, From 72cef98f30bc334f0a811bc7b3d72679e2a36ef6 Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Fri, 4 Sep 2026 09:19:00 -0600 Subject: [PATCH 06/16] Resolve ordinary dual-clock audit heads --- resources/Table.ts | 4 ++++ .../resources/dualClockAuditRecord.test.js | 19 ++++++++++++++++++- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/resources/Table.ts b/resources/Table.ts index f7928e56ee..f30104b97e 100644 --- a/resources/Table.ts +++ b/resources/Table.ts @@ -717,6 +717,10 @@ export function makeTable(options) { nodeId: number | undefined, refs?: Array<{ version: number; nodeId: number }> ) { + if (version != null) { + const directHead = auditStore.getSync(version, tableId, id, nodeId); + if (directHead?.version === version) return { txnLogKey: version, nodeId }; + } const visited = new Set(); function findHead(candidateRefs?: Array<{ version: number; nodeId: number }>) { if (!candidateRefs) return; diff --git a/unitTests/resources/dualClockAuditRecord.test.js b/unitTests/resources/dualClockAuditRecord.test.js index 81bdc2c97c..68b6905981 100644 --- a/unitTests/resources/dualClockAuditRecord.test.js +++ b/unitTests/resources/dualClockAuditRecord.test.js @@ -132,7 +132,7 @@ describe('Dual-clock audit records (harper#2412)', () => { it('keeps a log-key pointer to the audit head when the stored version differs', async function () { if (isLMDB) return this.skip(); const id = 'applied-head-1'; - const logKey = Date.now() + 10; + const logKey = Date.now() - 10_000; const version = logKey - 30_000; await applyFromOrigin(Plain, id, { id, name: 'newer' }, { logKey, version, nodeId: 0 }); const head = Plain.primaryStore.getEntry(id); @@ -171,6 +171,23 @@ describe('Dual-clock audit records (harper#2412)', () => { assert.equal(auditStore.get(logKey, Plain.tableId, id, 0), undefined); }); + it('does not let an audit-only fold displace an ordinary audit head', async function () { + if (isLMDB) return this.skip(); + const id = 'ordinary-fold-head-1'; + const head = Date.now() - 10_000; + await applyFromOrigin(Plain, id, { id, name: 'newer' }, { logKey: head, version: head, nodeId: 0 }); + await applyFromOrigin( + Plain, + id, + { name: 'older', count: { __op__: 'add', value: 1 } }, + { logKey: head + 1_000, version: head - 1, nodeId: 0, fullUpdate: false } + ); + assert( + (await Plain.getHistoryOfRecord(id)).some((entry) => entry.localTime === head), + 'an audit-only fold must not displace a directly addressable head' + ); + }); + it('bounds an overloaded audit-body version by the originating transaction-log key', async function () { if (isLMDB) return this.skip(); const id = 'applied-out-of-order-1'; From 782e1703573adaa24b5cb7ffebe2f43f9a9f18ac Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Fri, 4 Sep 2026 10:17:00 -0600 Subject: [PATCH 07/16] Preserve audit refs in the log-key domain --- resources/DESIGN.md | 12 ++-- resources/DatabaseTransaction.ts | 4 +- resources/RecordEncoder.ts | 2 +- resources/Table.ts | 46 +++++++++----- .../resources/dualClockAuditRecord.test.js | 62 ++++++++++++++++--- 5 files changed, 94 insertions(+), 32 deletions(-) diff --git a/resources/DESIGN.md b/resources/DESIGN.md index c3342a043c..654a14c99a 100644 --- a/resources/DESIGN.md +++ b/resources/DESIGN.md @@ -102,9 +102,9 @@ One giant `makeTable()` factory that returns a `TableResource extends Resource` | How is application row filtering applied? | Authorization admission happens in the resource operation before query work. The legacy `allow*` hook, when armed by the protocol, is evaluated once with its historical receiver semantics; overriding it never changes its scope. An operation override may add indexed conditions and/or attach the JavaScript-only synchronous `target.rowFilter(record, context)`. `Table.search` composes it with query filters and rechecks the final materialized cache/source record. `SubscriptionRequest.rowFilter` covers full-row events; `eventFilter(event, context)` explicitly handles tombstones/messages/raw events. Prefer indexed conditions because an opaque predicate may inspect every admitted candidate and `limit` applies after filtering. | **An audit record carries two clocks; never substitute one for the other (harper#2412 stage 0b).** -`AuditRecord.version` is the record's own version — LWW ordering in `precedesExistingVersion`, -`@updatedTime`, ETag/`Last-Modified`. For audit-only out-of-order entries the body may carry the -surviving record version rather than the originating write's version. `AuditRecord.txnLogKey` is that +`AuditRecord.version` is the originating write's record version — LWW ordering in +`precedesExistingVersion`, `@updatedTime`, ETag/`Last-Modified`. Historical audit-only entries may +carry the surviving version instead. `AuditRecord.txnLogKey` is that entry's key in the per-origin transaction log: write identity, the record→log lookup (`auditStore.get(logKey, tableId, id, nodeId)`), and every resume cursor. On LMDB these have always been distinct fields; on RocksDB the read surface used to overwrite `version` with the log key, and @@ -127,9 +127,9 @@ Consequences worth knowing: `options.version` by every `_write*` builder and read in `save()` only when the transaction is `sourceApply` or `isReplay`, is how a replication receiver stores the origin's version while the transaction commits under the origin's log key — so a peer's copy of an origin's log stays in the - origin's clock. `getAppliedWriteVersion` bounds the body value by `txnLogKey`: a source fill keeps - its earlier source version, while an out-of-order audit body cannot restamp its originating write - at the later surviving version. One frame can carry writes at different record versions, so this + origin's clock. New audit entries persist that write version even when an out-of-order merge leaves + a newer record version in the primary store. `getAppliedWriteVersion` also bounds historical + overloaded values by `txnLogKey`. One frame can carry writes at different record versions, so this cannot be a per-transaction value. Deprecated LMDB keeps its legacy transaction-version apply behavior. - **`additionalAuditRefs[].version` is a log key, not a version.** Every consumer follows it straight diff --git a/resources/DatabaseTransaction.ts b/resources/DatabaseTransaction.ts index e7849cff82..4264d60a9a 100644 --- a/resources/DatabaseTransaction.ts +++ b/resources/DatabaseTransaction.ts @@ -312,8 +312,8 @@ export type TransactionWrite = { // overload accounting, the replay marker and a no-op write's removal all belong to the committer. validate?: (txnTime: number, committedBy: DatabaseTransaction) => void; fullUpdate?: boolean; - // The audit body's candidate record version. Applied writes bound it by the origin's transaction-log - // key because an out-of-order audit-only entry can carry the later surviving record version. + // The origin record version carried by an applied or replayed write. Bound it by the origin's + // transaction-log key so malformed or historical overloaded values cannot move ordering past the write. recordVersion?: number; saved?: boolean; deferSave?: boolean; diff --git a/resources/RecordEncoder.ts b/resources/RecordEncoder.ts index 2794c4d9d7..b1c15e32f7 100644 --- a/resources/RecordEncoder.ts +++ b/resources/RecordEncoder.ts @@ -1013,7 +1013,7 @@ export function recordUpdater(store, tableId, auditStore) { result = auditStore[isRocksDB ? 'putSync' : 'put']( record === undefined ? NEW_TIMESTAMP_PLACEHOLDER : LAST_TIMESTAMP_PLACEHOLDER, { - version: newVersion, + version: options?.recordVersion ?? newVersion, tableId, recordId: id, previousVersion: isRocksDB ? existingEntry?.version : existingEntry?.localTime, diff --git a/resources/Table.ts b/resources/Table.ts index f30104b97e..08db66c1b0 100644 --- a/resources/Table.ts +++ b/resources/Table.ts @@ -724,17 +724,29 @@ export function makeTable(options) { const visited = new Set(); function findHead(candidateRefs?: Array<{ version: number; nodeId: number }>) { if (!candidateRefs) return; - for (const ref of candidateRefs) { + const pending: Array<{ ref: { version: number; nodeId: number }; auditRecord?: any }> = candidateRefs + .slice() + .reverse() + .map((ref) => ({ ref })); + while (pending.length > 0) { + const candidate: any = pending.pop(); + const { ref, auditRecord } = candidate; + if (auditRecord) { + if (auditRecord.version === version) return { txnLogKey: ref.version, nodeId: ref.nodeId }; + continue; + } const identity = `${ref.nodeId ?? 0}:${ref.version}`; if (visited.has(identity)) continue; visited.add(identity); - const auditRecord = auditStore.getSync(ref.version, tableId, id, ref.nodeId); - if (!auditRecord) continue; - // An audit-only out-of-order entry carries the surviving record version in its body. - // Its previous refs still identify the real head, so prefer them over the fold entry. - const previousHead = findHead(auditRecord.previousAdditionalAuditRefs); - if (previousHead) return previousHead; - if (auditRecord.version === version) return { txnLogKey: ref.version, nodeId: ref.nodeId }; + const entry = auditStore.getSync(ref.version, tableId, id, ref.nodeId); + if (!entry) continue; + // Historical audit-only entries can carry the surviving record version in their body. + // Their previous refs still identify the real head, so prefer them over the fold entry. + pending.push({ ref, auditRecord: entry }); + const previousRefs = entry.previousAdditionalAuditRefs; + if (previousRefs) { + for (let index = previousRefs.length - 1; index >= 0; index--) pending.push({ ref: previousRefs[index] }); + } } } const referencedHead = findHead(refs); @@ -3139,9 +3151,7 @@ export function makeTable(options) { // (so replication's head-tie fast-skip can't see them) yet are exact duplicates. Keyed by nodeId, // so it is correct across multiple source nodes. The lookup key is this write's LOG key, not its // record version — a replication apply commits under the origin's log key while storing the - // origin's version, and only the log key addresses the entry (harper#2412). The synthetic entry - // below still carries `txnTime` as its version: an audit-only commit records the surviving - // (newer) record version in its body, so `priorAudit.version` is not this write's version. + // origin's version, and only the log key addresses the entry (harper#2412). // RocksDB-only: LMDB audit entries are keyed by local audit time, so this lookup doesn't apply // there (LMDB keeps the exact unbounded walk). A miss (the keyed lookup can lag a back-to-back re-delivery — #1137) // simply falls through to the walk, so this never changes correctness; the additionalAuditRefs @@ -3185,12 +3195,10 @@ export function makeTable(options) { ? existingEntry.additionalAuditRefs.map((ref) => ({ localTime: ref.version, nodeId: ref.nodeId })) : []; - // Collect any existing audit refs that should be preserved (those older than current transaction) + // Out-of-order merges retain every existing branch head; per-origin log keys are not globally ordered. if (existingEntry.additionalAuditRefs) { for (const ref of existingEntry.additionalAuditRefs) { - if (ref.version <= txnTime) { - additionalAuditRefs.push(ref); - } + additionalAuditRefs.push(ref); } } let addedAuditRef = false; @@ -3578,6 +3586,7 @@ export function makeTable(options) { user: (context as any)?.user, residencyId, expiresAt, + recordVersion: txnTime, nodeId: options?.nodeId, viaNodeId: options?.viaNodeId, originatingOperation: (context as any)?.originatingOperation, @@ -3714,6 +3723,8 @@ export function makeTable(options) { const priorStagedOp = priorStagedWrite(write); const priorStaged = priorStagedOp?.stagedEntry; const existingRecord = priorStaged ? priorStaged.value : existingEntry?.value; + const txnLogKey = + isRocksDB && options?.version != null ? (transaction?.getTimestamp?.() ?? txnTime) : txnTime; if (retry) { if (context && existingEntry?.version > (context.lastModified || 0)) context.lastModified = existingEntry.version; @@ -3742,6 +3753,11 @@ export function makeTable(options) { viaNodeId: options?.viaNodeId, transaction, tableToTrack: tableName, + recordVersion: txnTime, + additionalAuditRefs: + isRocksDB && audit && txnLogKey !== txnTime + ? [{ version: txnLogKey, nodeId: options?.nodeId }] + : undefined, }, 'delete' ); diff --git a/unitTests/resources/dualClockAuditRecord.test.js b/unitTests/resources/dualClockAuditRecord.test.js index 68b6905981..591be3877b 100644 --- a/unitTests/resources/dualClockAuditRecord.test.js +++ b/unitTests/resources/dualClockAuditRecord.test.js @@ -56,6 +56,14 @@ describe('Dual-clock audit records (harper#2412)', () => { }); } + function deleteFromOrigin(TableClass, id, { logKey, version, nodeId = 1 }) { + const context = { source: {}, sourceApply: true, timestamp: logKey }; + return transaction(context, async () => { + const resource = await TableClass.getResource(id, context); + return resource._writeDelete(id, { nodeId, version }); + }); + } + before(async function () { if (isLMDB) return; setupTestDBPath(); @@ -63,7 +71,7 @@ describe('Dual-clock audit records (harper#2412)', () => { Plain = table({ table: 'DualClockPlain', database: 'test', - attributes: [{ name: 'id', isPrimaryKey: true }, { name: 'name' }], + attributes: [{ name: 'id', isPrimaryKey: true }, { name: 'name' }, { name: 'count' }], audit: true, }); Filled = table({ @@ -132,24 +140,62 @@ describe('Dual-clock audit records (harper#2412)', () => { it('keeps a log-key pointer to the audit head when the stored version differs', async function () { if (isLMDB) return this.skip(); const id = 'applied-head-1'; - const logKey = Date.now() - 10_000; - const version = logKey - 30_000; - await applyFromOrigin(Plain, id, { id, name: 'newer' }, { logKey, version, nodeId: 0 }); + const baseVersion = Date.now() - 60_000; + await applyFromOrigin( + Plain, + id, + { id, name: 'base', count: 0 }, + { logKey: baseVersion, version: baseVersion, nodeId: 0, isCopyApply: true } + ); + const logKey = baseVersion + 5_000; + const version = baseVersion + 2_000; + await applyFromOrigin(Plain, id, { name: 'newer' }, { logKey, version, nodeId: 0, fullUpdate: false }); const head = Plain.primaryStore.getEntry(id); assert(head.additionalAuditRefs?.some((ref) => ref.version === logKey && ref.nodeId === 0)); assert((await Plain.getHistoryOfRecord(id)).some((entry) => entry.localTime === logKey)); + const olderLogKey = baseVersion + 4_000; + const olderVersion = baseVersion + 1_000; await applyFromOrigin( Plain, id, - { name: 'older', count: { __op__: 'add', value: 1 } }, - { logKey: logKey + 1_000, version: version - 1, nodeId: 0, fullUpdate: false } + { count: { __op__: 'add', value: 1 } }, + { logKey: olderLogKey, version: olderVersion, nodeId: 2, fullUpdate: false } + ); + assert.deepEqual(await Plain.get(id), { id, name: 'newer', count: 1 }); + assert( + Plain.primaryStore.getEntry(id).additionalAuditRefs?.some((ref) => ref.version === logKey && ref.nodeId === 0), + 'an out-of-order merge must retain the surviving head in the log-key domain' ); - assert.deepEqual(await Plain.get(id), { id, name: 'newer' }); assert( (await Plain.getHistoryOfRecord(id)).some((entry) => entry.localTime === logKey), 'an audit-only fold must not displace the real head of the surviving record' ); + const olderAudit = auditEntriesFor(Plain, id).find((entry) => entry.txnLogKey === olderLogKey); + assert.equal(olderAudit.version, olderVersion, "crash replay must see the folded write's original version"); + }); + + it('keeps a log-key pointer to an applied delete whose version differs', async function () { + if (isLMDB) return this.skip(); + const id = 'applied-delete-head-1'; + const writeLogKey = Date.now() - 10_000; + const writeVersion = writeLogKey - 30_000; + await applyFromOrigin( + Plain, + id, + { id, name: 'present' }, + { logKey: writeLogKey, version: writeVersion, nodeId: 0 } + ); + const deleteLogKey = writeLogKey + 1_000; + const deleteVersion = writeVersion + 1; + await deleteFromOrigin(Plain, id, { logKey: deleteLogKey, version: deleteVersion, nodeId: 0 }); + const tombstone = Plain.primaryStore.getEntry(id); + assert.equal(tombstone.value, null); + assert.equal(tombstone.version, deleteVersion); + assert(tombstone.additionalAuditRefs?.some((ref) => ref.version === deleteLogKey && ref.nodeId === 0)); + const deleteAudit = auditEntriesFor(Plain, id).find((entry) => entry.txnLogKey === deleteLogKey); + assert.equal(deleteAudit.type, 'delete'); + assert.equal(deleteAudit.version, deleteVersion); }); it('does not point a copy-applied record at an audit entry that was never written', async function () { @@ -188,7 +234,7 @@ describe('Dual-clock audit records (harper#2412)', () => { ); }); - it('bounds an overloaded audit-body version by the originating transaction-log key', async function () { + it('bounds an applied record version by the originating transaction-log key', async function () { if (isLMDB) return this.skip(); const id = 'applied-out-of-order-1'; const logKey = Date.now(); From d397b4896acf4f5d111be5a4f29e2464918e5bb4 Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Fri, 4 Sep 2026 10:48:15 -0600 Subject: [PATCH 08/16] Keep audit chains in the log-key domain --- resources/RecordEncoder.ts | 4 +- resources/Table.ts | 70 ++++++++++++++----- resources/crdt.ts | 19 ++++- unitTests/resources/crdt.test.js | 28 +++++++- .../resources/dualClockAuditRecord.test.js | 2 + 5 files changed, 98 insertions(+), 25 deletions(-) diff --git a/resources/RecordEncoder.ts b/resources/RecordEncoder.ts index b1c15e32f7..56d046a289 100644 --- a/resources/RecordEncoder.ts +++ b/resources/RecordEncoder.ts @@ -1016,7 +1016,9 @@ export function recordUpdater(store, tableId, auditStore) { version: options?.recordVersion ?? newVersion, tableId, recordId: id, - previousVersion: isRocksDB ? existingEntry?.version : existingEntry?.localTime, + previousVersion: isRocksDB + ? (options?.previousTxnLogKey ?? existingEntry?.version) + : existingEntry?.localTime, nodeId, user: username, type, diff --git a/resources/Table.ts b/resources/Table.ts index 08db66c1b0..c18066b3a2 100644 --- a/resources/Table.ts +++ b/resources/Table.ts @@ -717,40 +717,30 @@ export function makeTable(options) { nodeId: number | undefined, refs?: Array<{ version: number; nodeId: number }> ) { - if (version != null) { - const directHead = auditStore.getSync(version, tableId, id, nodeId); - if (directHead?.version === version) return { txnLogKey: version, nodeId }; - } const visited = new Set(); function findHead(candidateRefs?: Array<{ version: number; nodeId: number }>) { if (!candidateRefs) return; - const pending: Array<{ ref: { version: number; nodeId: number }; auditRecord?: any }> = candidateRefs - .slice() - .reverse() - .map((ref) => ({ ref })); + const pending: Array<{ version: number; nodeId: number }> = candidateRefs.slice().reverse(); while (pending.length > 0) { - const candidate: any = pending.pop(); - const { ref, auditRecord } = candidate; - if (auditRecord) { - if (auditRecord.version === version) return { txnLogKey: ref.version, nodeId: ref.nodeId }; - continue; - } + const ref = pending.pop()!; const identity = `${ref.nodeId ?? 0}:${ref.version}`; if (visited.has(identity)) continue; visited.add(identity); const entry = auditStore.getSync(ref.version, tableId, id, ref.nodeId); if (!entry) continue; - // Historical audit-only entries can carry the surviving record version in their body. - // Their previous refs still identify the real head, so prefer them over the fold entry. - pending.push({ ref, auditRecord: entry }); + if (entry.version === version) return { txnLogKey: ref.version, nodeId: ref.nodeId }; const previousRefs = entry.previousAdditionalAuditRefs; if (previousRefs) { - for (let index = previousRefs.length - 1; index >= 0; index--) pending.push({ ref: previousRefs[index] }); + for (let index = previousRefs.length - 1; index >= 0; index--) pending.push(previousRefs[index]); } } } const referencedHead = findHead(refs); if (referencedHead) return referencedHead; + if (version != null) { + const directHead = auditStore.getSync(version, tableId, id, nodeId); + if (directHead?.version === version) return { txnLogKey: version, nodeId }; + } return { txnLogKey: version, nodeId }; } class TableResource extends Resource { @@ -3587,6 +3577,15 @@ export function makeTable(options) { residencyId, expiresAt, recordVersion: txnTime, + previousTxnLogKey: + isRocksDB && audit && existingEntry + ? resolveAuditHead( + id, + existingEntry.version, + existingEntry.nodeId, + existingEntry.additionalAuditRefs + ).txnLogKey + : undefined, nodeId: options?.nodeId, viaNodeId: options?.viaNodeId, originatingOperation: (context as any)?.originatingOperation, @@ -3754,6 +3753,15 @@ export function makeTable(options) { transaction, tableToTrack: tableName, recordVersion: txnTime, + previousTxnLogKey: + isRocksDB && audit && existingEntry + ? resolveAuditHead( + id, + existingEntry.version, + existingEntry.nodeId, + existingEntry.additionalAuditRefs + ).txnLogKey + : undefined, additionalAuditRefs: isRocksDB && audit && txnLogKey !== txnTime ? [{ version: txnLogKey, nodeId: options?.nodeId }] @@ -7079,6 +7087,15 @@ export function makeTable(options) { residencyId, transaction, tableToTrack: tableName, + previousTxnLogKey: + writeAudit && existingEntry + ? resolveAuditHead( + id, + existingEntry.version, + existingEntry.nodeId, + existingEntry.additionalAuditRefs + ).txnLogKey + : undefined, additionalAuditRefs: writeAudit && txnLogKey !== recordVersion ? [{ version: txnLogKey, nodeId: 0 }] : undefined, }, @@ -7100,7 +7117,22 @@ export function makeTable(options) { recordVersion, 0, (audit && hasChanges) || null, - { user: (sourceContext as any)?.user, transaction, tableToTrack: tableName }, + { + user: (sourceContext as any)?.user, + transaction, + tableToTrack: tableName, + recordVersion, + previousTxnLogKey: resolveAuditHead( + id, + existingEntry.version, + existingEntry.nodeId, + existingEntry.additionalAuditRefs + ).txnLogKey, + additionalAuditRefs: + audit && hasChanges && txnLogKey !== recordVersion + ? [{ version: txnLogKey, nodeId: 0 }] + : undefined, + }, 'delete', Boolean(invalidated) ); diff --git a/resources/crdt.ts b/resources/crdt.ts index 7b0077d37e..4819eeb27e 100644 --- a/resources/crdt.ts +++ b/resources/crdt.ts @@ -138,7 +138,7 @@ function reconstructForward(auditStore, store, tableId: number, recordId: any, f entries.push(auditEntry); } } - auditTime = auditEntry.previousVersion; + auditTime = previousAuditTime(auditStore, tableId, recordId, auditEntry); } if (entries.length === 0) return null; // record did not exist at `timestamp` // Replay oldest-first. The base is the put if the chain reached one; otherwise an empty record @@ -158,6 +158,14 @@ function reconstructForward(auditStore, store, tableId: number, recordId: any, f return record; } +function previousAuditTime(auditStore, tableId: number, recordId: any, auditEntry) { + for (const ref of auditEntry.previousAdditionalAuditRefs ?? []) { + const previous = auditStore.get(ref.version, tableId, recordId, ref.nodeId); + if (previous?.version === auditEntry.previousVersion) return ref.version; + } + return auditEntry.previousVersion; +} + /** * Reconstruct the record state at a given timestamp by going back through the audit history and reversing any changes * @param currentEntry @@ -169,6 +177,13 @@ export function getRecordAtTime(currentEntry, timestamp, store, tableId: number, const auditStore = store.rootStore.auditStore; let record = { ...currentEntry.value }; let auditTime = currentEntry.localTime; + for (const ref of currentEntry.additionalAuditRefs ?? []) { + const head = auditStore.get(ref.version, tableId, recordId, ref.nodeId); + if (head?.version === currentEntry.version) { + auditTime = ref.version; + break; + } + } // Iterate in reverse through the record history, trying to reverse all changes const unknowns = new Set(); while (auditTime > timestamp) { @@ -187,7 +202,7 @@ export function getRecordAtTime(currentEntry, timestamp, store, tableId: number, // state forward instead (issue #1330: a key deleted then re-inserted). return reconstructForward(auditStore, store, tableId, recordId, auditEntry.previousVersion, timestamp); } - auditTime = auditEntry.previousVersion; + auditTime = previousAuditTime(auditStore, tableId, recordId, auditEntry); } // If the most recent entry at or before `timestamp` is a delete, the record did not exist then. // (A delete reached as a boundary — rather than crossed, which returns via reconstructForward — diff --git a/unitTests/resources/crdt.test.js b/unitTests/resources/crdt.test.js index 5067c368ad..4bbdf0e3a1 100644 --- a/unitTests/resources/crdt.test.js +++ b/unitTests/resources/crdt.test.js @@ -7,14 +7,16 @@ const { getRecordAtTime, applyForward, addValues } = require('#src/resources/crd // version via the previousVersion chain). function makeStore(events) { const byVersion = new Map(); - for (const event of events) byVersion.set(event.version, event); + for (const event of events) byVersion.set(event.txnLogKey ?? event.version, event); const auditStore = { get(version) { const event = byVersion.get(version); if (!event) return undefined; return { type: event.type, + version: event.version, previousVersion: event.previousVersion, + previousAdditionalAuditRefs: event.previousAdditionalAuditRefs, getValue: () => event.value, }; }, @@ -23,11 +25,31 @@ function makeStore(events) { } // currentEntry mirrors the live record entry getRecordAtTime starts the reverse walk from. -function currentEntry(value, localTime) { - return { value, localTime }; +function currentEntry(value, localTime, options = {}) { + return { value, localTime, ...options }; } describe('crdt getRecordAtTime', () => { + it('walks divergent heads and previous links in the transaction-log-key domain', () => { + const events = [ + { txnLogKey: 100, version: 10, type: 'put', value: { id: 'D', count: 1 }, previousVersion: 0 }, + { + txnLogKey: 200, + version: 20, + type: 'patch', + value: { count: { __op__: 'add', value: 2 } }, + previousVersion: 10, + previousAdditionalAuditRefs: [{ version: 100, nodeId: 1 }], + }, + ]; + const store = makeStore(events); + const current = currentEntry({ id: 'D', count: 3 }, 20, { + version: 20, + additionalAuditRefs: [{ version: 200, nodeId: 1 }], + }); + assert.deepStrictEqual(getRecordAtTime(current, 150, store, 1, 'D'), { id: 'D', count: 1 }); + }); + describe('record deleted then re-inserted under the same key (issue #1330)', () => { // put(n:1) -> patch(n:2) -> patch(n:3) -> delete -> put(n:4, re-insert, current) const events = [ diff --git a/unitTests/resources/dualClockAuditRecord.test.js b/unitTests/resources/dualClockAuditRecord.test.js index 591be3877b..0a8200541e 100644 --- a/unitTests/resources/dualClockAuditRecord.test.js +++ b/unitTests/resources/dualClockAuditRecord.test.js @@ -31,6 +31,7 @@ describe('Dual-clock audit records (harper#2412)', () => { type: auditRecord.type, version: auditRecord.version, txnLogKey: auditRecord.txnLogKey, + previousVersion: auditRecord.previousVersion, }); } return entries; @@ -173,6 +174,7 @@ describe('Dual-clock audit records (harper#2412)', () => { ); const olderAudit = auditEntriesFor(Plain, id).find((entry) => entry.txnLogKey === olderLogKey); assert.equal(olderAudit.version, olderVersion, "crash replay must see the folded write's original version"); + assert.equal(olderAudit.previousVersion, logKey, 'history links must stay in the log-key domain'); }); it('keeps a log-key pointer to an applied delete whose version differs', async function () { From 4b12232bb17af540ff16e9a9380525220ff7f134 Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Fri, 4 Sep 2026 11:14:47 -0600 Subject: [PATCH 09/16] Resolve dual-clock history links on read --- resources/RecordEncoder.ts | 4 +-- resources/Table.ts | 33 ------------------- unitTests/resources/crdt.test.js | 2 +- .../resources/dualClockAuditRecord.test.js | 4 ++- 4 files changed, 5 insertions(+), 38 deletions(-) diff --git a/resources/RecordEncoder.ts b/resources/RecordEncoder.ts index 56d046a289..b1c15e32f7 100644 --- a/resources/RecordEncoder.ts +++ b/resources/RecordEncoder.ts @@ -1016,9 +1016,7 @@ export function recordUpdater(store, tableId, auditStore) { version: options?.recordVersion ?? newVersion, tableId, recordId: id, - previousVersion: isRocksDB - ? (options?.previousTxnLogKey ?? existingEntry?.version) - : existingEntry?.localTime, + previousVersion: isRocksDB ? existingEntry?.version : existingEntry?.localTime, nodeId, user: username, type, diff --git a/resources/Table.ts b/resources/Table.ts index c18066b3a2..40ca7b9c69 100644 --- a/resources/Table.ts +++ b/resources/Table.ts @@ -3577,15 +3577,6 @@ export function makeTable(options) { residencyId, expiresAt, recordVersion: txnTime, - previousTxnLogKey: - isRocksDB && audit && existingEntry - ? resolveAuditHead( - id, - existingEntry.version, - existingEntry.nodeId, - existingEntry.additionalAuditRefs - ).txnLogKey - : undefined, nodeId: options?.nodeId, viaNodeId: options?.viaNodeId, originatingOperation: (context as any)?.originatingOperation, @@ -3753,15 +3744,6 @@ export function makeTable(options) { transaction, tableToTrack: tableName, recordVersion: txnTime, - previousTxnLogKey: - isRocksDB && audit && existingEntry - ? resolveAuditHead( - id, - existingEntry.version, - existingEntry.nodeId, - existingEntry.additionalAuditRefs - ).txnLogKey - : undefined, additionalAuditRefs: isRocksDB && audit && txnLogKey !== txnTime ? [{ version: txnLogKey, nodeId: options?.nodeId }] @@ -7087,15 +7069,6 @@ export function makeTable(options) { residencyId, transaction, tableToTrack: tableName, - previousTxnLogKey: - writeAudit && existingEntry - ? resolveAuditHead( - id, - existingEntry.version, - existingEntry.nodeId, - existingEntry.additionalAuditRefs - ).txnLogKey - : undefined, additionalAuditRefs: writeAudit && txnLogKey !== recordVersion ? [{ version: txnLogKey, nodeId: 0 }] : undefined, }, @@ -7122,12 +7095,6 @@ export function makeTable(options) { transaction, tableToTrack: tableName, recordVersion, - previousTxnLogKey: resolveAuditHead( - id, - existingEntry.version, - existingEntry.nodeId, - existingEntry.additionalAuditRefs - ).txnLogKey, additionalAuditRefs: audit && hasChanges && txnLogKey !== recordVersion ? [{ version: txnLogKey, nodeId: 0 }] diff --git a/unitTests/resources/crdt.test.js b/unitTests/resources/crdt.test.js index 4bbdf0e3a1..555c085ea5 100644 --- a/unitTests/resources/crdt.test.js +++ b/unitTests/resources/crdt.test.js @@ -30,7 +30,7 @@ function currentEntry(value, localTime, options = {}) { } describe('crdt getRecordAtTime', () => { - it('walks divergent heads and previous links in the transaction-log-key domain', () => { + it('walks divergent heads through ref-based transaction-log keys', () => { const events = [ { txnLogKey: 100, version: 10, type: 'put', value: { id: 'D', count: 1 }, previousVersion: 0 }, { diff --git a/unitTests/resources/dualClockAuditRecord.test.js b/unitTests/resources/dualClockAuditRecord.test.js index 0a8200541e..fc88b936cf 100644 --- a/unitTests/resources/dualClockAuditRecord.test.js +++ b/unitTests/resources/dualClockAuditRecord.test.js @@ -32,6 +32,7 @@ describe('Dual-clock audit records (harper#2412)', () => { version: auditRecord.version, txnLogKey: auditRecord.txnLogKey, previousVersion: auditRecord.previousVersion, + previousAdditionalAuditRefs: auditRecord.previousAdditionalAuditRefs, }); } return entries; @@ -174,7 +175,8 @@ describe('Dual-clock audit records (harper#2412)', () => { ); const olderAudit = auditEntriesFor(Plain, id).find((entry) => entry.txnLogKey === olderLogKey); assert.equal(olderAudit.version, olderVersion, "crash replay must see the folded write's original version"); - assert.equal(olderAudit.previousVersion, logKey, 'history links must stay in the log-key domain'); + assert.equal(olderAudit.previousVersion, version, 'the compatibility link remains the prior record version'); + assert.deepEqual(olderAudit.previousAdditionalAuditRefs, [{ version: logKey, nodeId: 0 }]); }); it('keeps a log-key pointer to an applied delete whose version differs', async function () { From 82dcb92d5d5b100842d7b19d5f702212b64d0eef Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Fri, 4 Sep 2026 11:34:15 -0600 Subject: [PATCH 10/16] Follow dual-clock audit history by log key --- resources/RecordEncoder.ts | 15 ++++++++-- resources/Table.ts | 50 +++++++++++++++++++++----------- resources/crdt.ts | 9 +++++- unitTests/resources/crdt.test.js | 36 +++++++++++++++++++++++ 4 files changed, 90 insertions(+), 20 deletions(-) diff --git a/resources/RecordEncoder.ts b/resources/RecordEncoder.ts index b1c15e32f7..273fb9e094 100644 --- a/resources/RecordEncoder.ts +++ b/resources/RecordEncoder.ts @@ -983,8 +983,19 @@ export function recordUpdater(store, tableId, auditStore) { const nodeId = options?.nodeId ?? getThisNodeId(auditStore) ?? 0; const viaNodeId = options?.viaNodeId ?? nodeId; if (resolveRecord && existingEntry?.localTime) { - const replacingId = existingEntry?.localTime; - const replacingEntry = auditStore.get(replacingId, tableId, id); + let replacingId = existingEntry.localTime; + let replacingEntry; + if (isRocksDB) { + for (const ref of existingEntry.additionalAuditRefs ?? []) { + const candidate = auditStore.get(ref.version, tableId, id, ref.nodeId); + if (candidate?.version === existingEntry.version) { + replacingId = ref.version; + replacingEntry = candidate; + break; + } + } + } + replacingEntry ??= auditStore.get(replacingId, tableId, id); if (replacingEntry) { const previousVersion = replacingEntry.previousVersion; result = auditStore[isRocksDB ? 'putSync' : 'put']( diff --git a/resources/Table.ts b/resources/Table.ts index 40ca7b9c69..837fe38cea 100644 --- a/resources/Table.ts +++ b/resources/Table.ts @@ -3166,7 +3166,10 @@ export function makeTable(options) { } } // incremental CRDT updates are only available with audit logging on - let localTime = existingEntry.localTime; + const initialAuditHead = isRocksDB + ? resolveAuditHead(id, existingEntry.version, existingEntry.nodeId, existingEntry.additionalAuditRefs) + : { txnLogKey: existingEntry.localTime, nodeId: existingEntry.nodeId }; + let localTime = initialAuditHead.txnLogKey; let auditedVersion = existingEntry.version; logger.debug?.( 'Applying CRDT update to record with id: ', @@ -3179,7 +3182,7 @@ export function makeTable(options) { new Date(localTime) ); - let nodeId = existingEntry.nodeId; + let nodeId = initialAuditHead.nodeId; const succeedingUpdates = []; // record the "future" updates, as we need to apply the updates in reverse order const auditRefsToVisit: Array<{ localTime: number; nodeId: number }> = existingEntry.additionalAuditRefs ? existingEntry.additionalAuditRefs.map((ref) => ({ localTime: ref.version, nodeId: ref.nodeId })) @@ -3193,6 +3196,28 @@ export function makeTable(options) { } let addedAuditRef = false; let nextRef: { localTime: number; nodeId: number }; + const visitedAuditRefs = new Set(); + const queuePreviousAuditRefs = (auditRecord) => { + const previousRefs = auditRecord.previousAdditionalAuditRefs; + if (previousRefs) { + for (const ref of previousRefs) { + auditRefsToVisit.push({ localTime: ref.version, nodeId: ref.nodeId }); + logger.debug?.('Adding audit ref from audit record to visit queue', { + version: ref.version, + nodeId: ref.nodeId, + }); + } + } + }; + const advanceToPreviousAudit = (auditRecord) => { + const previousRefs = auditRecord.previousAdditionalAuditRefs; + const previousHead = + isRocksDB && previousRefs?.length + ? resolveAuditHead(id, auditRecord.previousVersion, auditRecord.previousNodeId, previousRefs) + : { txnLogKey: auditRecord.previousVersion, nodeId: auditRecord.previousNodeId }; + localTime = previousHead.txnLogKey; + nodeId = previousHead.nodeId; + }; let walkSteps = 0; let auditWalkCapped = false; // Early-out residual: as we walk the chain newest-first, fold each succeeding patch into a @@ -3226,6 +3251,9 @@ export function makeTable(options) { }; do { while (localTime > txnTime || (auditedVersion >= txnTime && localTime > 0)) { + const auditIdentity = `${nodeId ?? 0}:${localTime}`; + if (visitedAuditRefs.has(auditIdentity)) break; + visitedAuditRefs.add(auditIdentity); // Bound the walk only for RocksDB, where the OOM was observed (issue #1114): each step // is a transaction-log range scan + msgpackr decode, and the per-node logs can be huge. // LMDB audit entries are keyed by local audit time (not version), so the duplicate @@ -3236,6 +3264,7 @@ export function makeTable(options) { } const auditRecord = auditStore.get(localTime, tableId, id, nodeId); if (!auditRecord) break; + queuePreviousAuditRefs(auditRecord); if ( isRocksDB && !stagedOwnAuditEntry && @@ -3267,8 +3296,7 @@ export function makeTable(options) { } if (precedesExisting > 0) { // if the existing version is older, we can skip this update - localTime = auditRecord.previousVersion; - nodeId = auditRecord.previousNodeId; + advanceToPreviousAudit(auditRecord); continue; } } @@ -3315,17 +3343,6 @@ export function makeTable(options) { nodeId: options?.nodeId, }); } - // Collect any additional audit refs from this audit record to traverse other branches - if (auditRecord.previousAdditionalAuditRefs) { - for (const ref of auditRecord.previousAdditionalAuditRefs) { - auditRefsToVisit.push({ localTime: ref.version, nodeId: ref.nodeId }); - logger.debug?.('Adding audit ref from audit record to visit queue', { - version: ref.version, - nodeId: ref.nodeId, - }); - } - } - // Every field of this write is overwritten by newer writes, and there is no alternate // audit branch left to scan, so it is fully superseded — the same outcome as walking to // the end and taking the `writeCommit(false)` escape below, reached without paying the rest @@ -3344,8 +3361,7 @@ export function makeTable(options) { return writeCommit(false); } - localTime = auditRecord.previousVersion; - nodeId = auditRecord.previousNodeId; + advanceToPreviousAudit(auditRecord); } // Check if we need to scan additional audit refs from this record if (auditWalkCapped) break; diff --git a/resources/crdt.ts b/resources/crdt.ts index 4819eeb27e..35a13f48ed 100644 --- a/resources/crdt.ts +++ b/resources/crdt.ts @@ -200,7 +200,14 @@ export function getRecordAtTime(currentEntry, timestamp, store, tableId: number, // The reverse walk reached a delete that is newer than `timestamp`. There is no // base record to keep reversing patches against, so reconstruct the pre-delete // state forward instead (issue #1330: a key deleted then re-inserted). - return reconstructForward(auditStore, store, tableId, recordId, auditEntry.previousVersion, timestamp); + return reconstructForward( + auditStore, + store, + tableId, + recordId, + previousAuditTime(auditStore, tableId, recordId, auditEntry), + timestamp + ); } auditTime = previousAuditTime(auditStore, tableId, recordId, auditEntry); } diff --git a/unitTests/resources/crdt.test.js b/unitTests/resources/crdt.test.js index 555c085ea5..12919d9bf7 100644 --- a/unitTests/resources/crdt.test.js +++ b/unitTests/resources/crdt.test.js @@ -50,6 +50,42 @@ describe('crdt getRecordAtTime', () => { assert.deepStrictEqual(getRecordAtTime(current, 150, store, 1, 'D'), { id: 'D', count: 1 }); }); + it('crosses a dual-clock delete through its ref-based predecessor', () => { + const events = [ + { txnLogKey: 100, version: 10, type: 'put', value: { id: 'D', count: 1 }, previousVersion: 0 }, + { + txnLogKey: 200, + version: 20, + type: 'patch', + value: { count: { __op__: 'add', value: 2 } }, + previousVersion: 10, + previousAdditionalAuditRefs: [{ version: 100, nodeId: 1 }], + }, + { + txnLogKey: 300, + version: 30, + type: 'delete', + value: null, + previousVersion: 20, + previousAdditionalAuditRefs: [{ version: 200, nodeId: 1 }], + }, + { + txnLogKey: 400, + version: 40, + type: 'put', + value: { id: 'D', count: 9 }, + previousVersion: 30, + previousAdditionalAuditRefs: [{ version: 300, nodeId: 1 }], + }, + ]; + const store = makeStore(events); + const current = currentEntry({ id: 'D', count: 9 }, 40, { + version: 40, + additionalAuditRefs: [{ version: 400, nodeId: 1 }], + }); + assert.deepStrictEqual(getRecordAtTime(current, 250, store, 1, 'D'), { id: 'D', count: 3 }); + }); + describe('record deleted then re-inserted under the same key (issue #1330)', () => { // put(n:1) -> patch(n:2) -> patch(n:3) -> delete -> put(n:4, re-insert, current) const events = [ From d31d253e6c4a892a4035922c6d6b57914914ca9d Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Fri, 4 Sep 2026 11:40:15 -0600 Subject: [PATCH 11/16] Resolve nested dual-clock audit identities --- resources/auditStore.ts | 15 ++++--- resources/crdt.ts | 45 +++++++++++++------ unitTests/resources/crdt.test.js | 20 ++++++++- .../resources/dualClockAuditRecord.test.js | 3 ++ 4 files changed, 61 insertions(+), 22 deletions(-) diff --git a/resources/auditStore.ts b/resources/auditStore.ts index 2974576311..0dda4d3b15 100644 --- a/resources/auditStore.ts +++ b/resources/auditStore.ts @@ -380,16 +380,17 @@ export function openAuditStore(rootStore) { * different write and authorize destroying live state (a tombstone, a still-referenced blob). * * On LMDB this is exact — the audit-store key IS the record's `txnLogKey`. On RocksDB a record stores - * one word, its version, which equals its log key for every write except a source fill (harper#2065); - * a fill therefore answers false here, as the pre-normalization version-versus-log-key compare - * already did. Absent identity answers false too. The direction is deliberate: uncertainty retains. + * its version in the compatibility word and keeps a divergent log key in `additionalAuditRefs`. + * Absent identity answers false. The direction is deliberate: uncertainty retains. */ export function isAuditEntryWrite(entry: any, auditRecord: AuditRecord): boolean { + if (entry == null || auditRecord.txnLogKey == null) return false; + const auditNodeId = auditRecord.nodeId ?? 0; return ( - entry != null && - auditRecord.txnLogKey != null && - entry.localTime === auditRecord.txnLogKey && - (entry.nodeId ?? 0) === (auditRecord.nodeId ?? 0) + (entry.localTime === auditRecord.txnLogKey && (entry.nodeId ?? 0) === auditNodeId) || + entry.additionalAuditRefs?.some( + (ref) => ref.version === auditRecord.txnLogKey && (ref.nodeId ?? 0) === auditNodeId + ) === true ); } diff --git a/resources/crdt.ts b/resources/crdt.ts index 35a13f48ed..db23b67be0 100644 --- a/resources/crdt.ts +++ b/resources/crdt.ts @@ -158,12 +158,32 @@ function reconstructForward(auditStore, store, tableId: number, recordId: any, f return record; } -function previousAuditTime(auditStore, tableId: number, recordId: any, auditEntry) { - for (const ref of auditEntry.previousAdditionalAuditRefs ?? []) { - const previous = auditStore.get(ref.version, tableId, recordId, ref.nodeId); - if (previous?.version === auditEntry.previousVersion) return ref.version; +function resolveAuditTime(auditStore, tableId: number, recordId: any, version, refs) { + const visited = new Set(); + const pending = (refs ?? []).slice().reverse(); + while (pending.length > 0) { + const ref = pending.pop(); + const identity = `${ref.nodeId ?? 0}:${ref.version}`; + if (visited.has(identity)) continue; + visited.add(identity); + const entry = auditStore.get(ref.version, tableId, recordId, ref.nodeId); + if (!entry) continue; + if (entry.version === version) return ref.version; + for (const previousRef of (entry.previousAdditionalAuditRefs ?? []).slice().reverse()) { + pending.push(previousRef); + } } - return auditEntry.previousVersion; + return version; +} + +function previousAuditTime(auditStore, tableId: number, recordId: any, auditEntry) { + return resolveAuditTime( + auditStore, + tableId, + recordId, + auditEntry.previousVersion, + auditEntry.previousAdditionalAuditRefs + ); } /** @@ -176,14 +196,13 @@ function previousAuditTime(auditStore, tableId: number, recordId: any, auditEntr export function getRecordAtTime(currentEntry, timestamp, store, tableId: number, recordId: any) { const auditStore = store.rootStore.auditStore; let record = { ...currentEntry.value }; - let auditTime = currentEntry.localTime; - for (const ref of currentEntry.additionalAuditRefs ?? []) { - const head = auditStore.get(ref.version, tableId, recordId, ref.nodeId); - if (head?.version === currentEntry.version) { - auditTime = ref.version; - break; - } - } + let auditTime = resolveAuditTime( + auditStore, + tableId, + recordId, + currentEntry.version ?? currentEntry.localTime, + currentEntry.additionalAuditRefs + ); // Iterate in reverse through the record history, trying to reverse all changes const unknowns = new Set(); while (auditTime > timestamp) { diff --git a/unitTests/resources/crdt.test.js b/unitTests/resources/crdt.test.js index 12919d9bf7..eb08849fa2 100644 --- a/unitTests/resources/crdt.test.js +++ b/unitTests/resources/crdt.test.js @@ -33,19 +33,35 @@ describe('crdt getRecordAtTime', () => { it('walks divergent heads through ref-based transaction-log keys', () => { const events = [ { txnLogKey: 100, version: 10, type: 'put', value: { id: 'D', count: 1 }, previousVersion: 0 }, + { + txnLogKey: 150, + version: 15, + type: 'patch', + value: { ignored: true }, + previousVersion: 10, + previousAdditionalAuditRefs: [{ version: 100, nodeId: 1 }], + }, { txnLogKey: 200, version: 20, type: 'patch', value: { count: { __op__: 'add', value: 2 } }, previousVersion: 10, - previousAdditionalAuditRefs: [{ version: 100, nodeId: 1 }], + previousAdditionalAuditRefs: [{ version: 150, nodeId: 1 }], + }, + { + txnLogKey: 300, + version: 30, + type: 'patch', + value: { ignored: true }, + previousVersion: 20, + previousAdditionalAuditRefs: [{ version: 200, nodeId: 1 }], }, ]; const store = makeStore(events); const current = currentEntry({ id: 'D', count: 3 }, 20, { version: 20, - additionalAuditRefs: [{ version: 200, nodeId: 1 }], + additionalAuditRefs: [{ version: 300, nodeId: 1 }], }); assert.deepStrictEqual(getRecordAtTime(current, 150, store, 1, 'D'), { id: 'D', count: 1 }); }); diff --git a/unitTests/resources/dualClockAuditRecord.test.js b/unitTests/resources/dualClockAuditRecord.test.js index fc88b936cf..ca8f8fa097 100644 --- a/unitTests/resources/dualClockAuditRecord.test.js +++ b/unitTests/resources/dualClockAuditRecord.test.js @@ -13,6 +13,7 @@ const { table } = require('#src/resources/databases'); const { Resource } = require('#src/resources/Resource'); const { setMainIsWorker } = require('#js/server/threads/manageThreads'); const { transaction } = require('#src/resources/transaction'); +const { removeAuditEntry } = require('#src/resources/auditStore'); const { waitFor } = require('../waitFor.js'); const isLMDB = process.env.HARPER_STORAGE_ENGINE === 'lmdb'; @@ -200,6 +201,8 @@ describe('Dual-clock audit records (harper#2412)', () => { const deleteAudit = auditEntriesFor(Plain, id).find((entry) => entry.txnLogKey === deleteLogKey); assert.equal(deleteAudit.type, 'delete'); assert.equal(deleteAudit.version, deleteVersion); + await removeAuditEntry(auditStore, auditStore.get(deleteLogKey, Plain.tableId, id, 0)); + assert.equal(Plain.primaryStore.getEntry(id), undefined, 'removing the matching audit entry removes its tombstone'); }); it('does not point a copy-applied record at an audit entry that was never written', async function () { From 9211de8bde4759a7a7decaa00ce6af9385587e10 Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Fri, 4 Sep 2026 11:46:43 -0600 Subject: [PATCH 12/16] Keep dual-clock cleanup and history origin-aware --- resources/Table.ts | 6 ++- resources/crdt.ts | 49 ++++++++++++------- unitTests/resources/crdt.test.js | 23 +++++++-- .../resources/dualClockAuditRecord.test.js | 6 +++ 4 files changed, 63 insertions(+), 21 deletions(-) diff --git a/resources/Table.ts b/resources/Table.ts index 837fe38cea..d21d942fe3 100644 --- a/resources/Table.ts +++ b/resources/Table.ts @@ -6054,7 +6054,11 @@ export function makeTable(options) { for (const entry of primaryStore.getRange({ start: 0, versions: true })) { const { key, value, localTime, version } = entry; await rest(); // yield to other async operations - if (value === null && version != null && localTime < endTime) { + const auditTime = + isRocksDB && version != null + ? resolveAuditHead(key, version, entry.nodeId, entry.additionalAuditRefs).txnLogKey + : localTime; + if (value === null && version != null && auditTime < endTime) { const backpressure = queueRemoval( () => primaryStore.remove(key, version), 'Error removing deleted record during deleteHistory' diff --git a/resources/crdt.ts b/resources/crdt.ts index db23b67be0..e3d52debc0 100644 --- a/resources/crdt.ts +++ b/resources/crdt.ts @@ -109,21 +109,21 @@ export function applyForward(record, update) { * against, and everything newer than the delete is irrelevant to a `timestamp` that precedes it (a * key that was deleted then re-inserted, see issue #1330). * - * `fromVersion` is the newest pre-delete entry (the delete's previousVersion); the walk follows the - * previousVersion chain from there. Entries newer than `timestamp` are skipped (the cutoff may fall + * `fromPosition` is the newest pre-delete audit position; the walk follows the previous links from + * there. Entries newer than `timestamp` are skipped (the cutoff may fall * between entries). Returns null if the record did not exist at `timestamp` (the nearest in-range * history boundary is a delete with no surviving writes, or there is no in-range history). */ -function reconstructForward(auditStore, store, tableId: number, recordId: any, fromVersion, timestamp) { +function reconstructForward(auditStore, store, tableId: number, recordId: any, fromPosition, timestamp) { // Collect the in-range entries (at or before `timestamp`) back to a base boundary, newest-first. // The boundary is a full `put` (snapshot) or a `delete` (everything older is erased); a record // whose first write was a `patch` has no put and bottoms out at the start of history. Only // `put`/`patch` contribute to the value, matching the reverse walk's switch (other partial types // such as `invalidate` are ignored). const entries = []; - let auditTime = fromVersion; + let { txnLogKey: auditTime, nodeId: auditNodeId } = fromPosition; while (auditTime > 0) { - const auditEntry = auditStore.get(auditTime, tableId, recordId); + const auditEntry = auditStore.get(auditTime, tableId, recordId, auditNodeId); if (!auditEntry) break; if (auditEntry.type === 'delete') { // A delete at or before `timestamp` bounds the history; the record is rebuilt from the @@ -138,7 +138,9 @@ function reconstructForward(auditStore, store, tableId: number, recordId: any, f entries.push(auditEntry); } } - auditTime = previousAuditTime(auditStore, tableId, recordId, auditEntry); + const previousPosition = previousAuditPosition(auditStore, tableId, recordId, auditEntry); + auditTime = previousPosition.txnLogKey; + auditNodeId = previousPosition.nodeId; } if (entries.length === 0) return null; // record did not exist at `timestamp` // Replay oldest-first. The base is the put if the chain reached one; otherwise an empty record @@ -158,7 +160,7 @@ function reconstructForward(auditStore, store, tableId: number, recordId: any, f return record; } -function resolveAuditTime(auditStore, tableId: number, recordId: any, version, refs) { +function resolveAuditPosition(auditStore, tableId: number, recordId: any, version, nodeId, refs) { const visited = new Set(); const pending = (refs ?? []).slice().reverse(); while (pending.length > 0) { @@ -168,20 +170,21 @@ function resolveAuditTime(auditStore, tableId: number, recordId: any, version, r visited.add(identity); const entry = auditStore.get(ref.version, tableId, recordId, ref.nodeId); if (!entry) continue; - if (entry.version === version) return ref.version; + if (entry.version === version) return { txnLogKey: ref.version, nodeId: ref.nodeId }; for (const previousRef of (entry.previousAdditionalAuditRefs ?? []).slice().reverse()) { pending.push(previousRef); } } - return version; + return { txnLogKey: version, nodeId }; } -function previousAuditTime(auditStore, tableId: number, recordId: any, auditEntry) { - return resolveAuditTime( +function previousAuditPosition(auditStore, tableId: number, recordId: any, auditEntry) { + return resolveAuditPosition( auditStore, tableId, recordId, auditEntry.previousVersion, + auditEntry.previousNodeId, auditEntry.previousAdditionalAuditRefs ); } @@ -196,17 +199,20 @@ function previousAuditTime(auditStore, tableId: number, recordId: any, auditEntr export function getRecordAtTime(currentEntry, timestamp, store, tableId: number, recordId: any) { const auditStore = store.rootStore.auditStore; let record = { ...currentEntry.value }; - let auditTime = resolveAuditTime( + const initialPosition = resolveAuditPosition( auditStore, tableId, recordId, currentEntry.version ?? currentEntry.localTime, + currentEntry.nodeId, currentEntry.additionalAuditRefs ); + let auditTime = initialPosition.txnLogKey; + let auditNodeId = initialPosition.nodeId; // Iterate in reverse through the record history, trying to reverse all changes const unknowns = new Set(); while (auditTime > timestamp) { - const auditEntry = auditStore.get(auditTime, tableId, recordId); + const auditEntry = auditStore.get(auditTime, tableId, recordId, auditNodeId); if (!auditEntry) break; switch (auditEntry.type) { case 'put': @@ -224,17 +230,19 @@ export function getRecordAtTime(currentEntry, timestamp, store, tableId: number, store, tableId, recordId, - previousAuditTime(auditStore, tableId, recordId, auditEntry), + previousAuditPosition(auditStore, tableId, recordId, auditEntry), timestamp ); } - auditTime = previousAuditTime(auditStore, tableId, recordId, auditEntry); + const previousPosition = previousAuditPosition(auditStore, tableId, recordId, auditEntry); + auditTime = previousPosition.txnLogKey; + auditNodeId = previousPosition.nodeId; } // If the most recent entry at or before `timestamp` is a delete, the record did not exist then. // (A delete reached as a boundary — rather than crossed, which returns via reconstructForward — // is otherwise missed, leaving `record` holding a newer re-inserted value. See issue #1330.) if (auditTime > 0) { - const boundaryEntry = auditStore.get(auditTime, tableId, recordId); + const boundaryEntry = auditStore.get(auditTime, tableId, recordId, auditNodeId); if (boundaryEntry?.type === 'delete') return null; } // A reversed patch that set a field to a plain value can't be undone (a plain set has no inverse), @@ -245,7 +253,14 @@ export function getRecordAtTime(currentEntry, timestamp, store, tableId: number, // single delta rather than the folded value at `timestamp`. If the history needed to reconstruct a // key is unavailable (pruned), the key keeps its live value (best effort, as before). if (unknowns.size > 0 && auditTime > 0) { - const priorRecord = reconstructForward(auditStore, store, tableId, recordId, auditTime, timestamp); + const priorRecord = reconstructForward( + auditStore, + store, + tableId, + recordId, + { txnLogKey: auditTime, nodeId: auditNodeId }, + timestamp + ); if (priorRecord) { for (const key of unknowns) { // Object.hasOwn, not `in`: an unknown field named like a prototype member (toString, diff --git a/unitTests/resources/crdt.test.js b/unitTests/resources/crdt.test.js index eb08849fa2..30b032f03b 100644 --- a/unitTests/resources/crdt.test.js +++ b/unitTests/resources/crdt.test.js @@ -6,16 +6,25 @@ const { getRecordAtTime, applyForward, addValues } = require('#src/resources/crd // entry by its exact version (matching the real store, which is only ever queried by exact // version via the previousVersion chain). function makeStore(events) { + const byPosition = new Map(); const byVersion = new Map(); - for (const event of events) byVersion.set(event.txnLogKey ?? event.version, event); + for (const event of events) { + const txnLogKey = event.txnLogKey ?? event.version; + const nodeId = event.nodeId ?? 1; + byPosition.set(`${nodeId}:${txnLogKey}`, event); + const atVersion = byVersion.get(txnLogKey) ?? []; + atVersion.push(event); + byVersion.set(txnLogKey, atVersion); + } const auditStore = { - get(version) { - const event = byVersion.get(version); + get(version, _tableId, _recordId, nodeId) { + const event = nodeId == null ? byVersion.get(version)?.at(-1) : byPosition.get(`${nodeId}:${version}`); if (!event) return undefined; return { type: event.type, version: event.version, previousVersion: event.previousVersion, + previousNodeId: event.previousNodeId, previousAdditionalAuditRefs: event.previousAdditionalAuditRefs, getValue: () => event.value, }; @@ -57,6 +66,14 @@ describe('crdt getRecordAtTime', () => { previousVersion: 20, previousAdditionalAuditRefs: [{ version: 200, nodeId: 1 }], }, + { + txnLogKey: 200, + nodeId: 2, + version: 999, + type: 'put', + value: { id: 'D', count: 999 }, + previousVersion: 0, + }, ]; const store = makeStore(events); const current = currentEntry({ id: 'D', count: 3 }, 20, { diff --git a/unitTests/resources/dualClockAuditRecord.test.js b/unitTests/resources/dualClockAuditRecord.test.js index ca8f8fa097..2cdb31c4b7 100644 --- a/unitTests/resources/dualClockAuditRecord.test.js +++ b/unitTests/resources/dualClockAuditRecord.test.js @@ -201,6 +201,12 @@ describe('Dual-clock audit records (harper#2412)', () => { const deleteAudit = auditEntriesFor(Plain, id).find((entry) => entry.txnLogKey === deleteLogKey); assert.equal(deleteAudit.type, 'delete'); assert.equal(deleteAudit.version, deleteVersion); + await Plain.deleteHistory(deleteLogKey - 1, true); + assert.equal( + Plain.primaryStore.getEntry(id)?.value, + null, + 'cleanup must retain a tombstone while its delete audit entry is newer than the cutoff' + ); await removeAuditEntry(auditStore, auditStore.get(deleteLogKey, Plain.tableId, id, 0)); assert.equal(Plain.primaryStore.getEntry(id), undefined, 'removing the matching audit entry removes its tombstone'); }); From 72d8b542f7a5528de61f8d93d8e996e6dff3d52f Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Fri, 4 Sep 2026 12:04:22 -0600 Subject: [PATCH 13/16] Preserve audit heads for replayed write types --- resources/Table.ts | 46 +++++++--- .../resources/dualClockAuditRecord.test.js | 90 +++++++++++++++++++ 2 files changed, 126 insertions(+), 10 deletions(-) diff --git a/resources/Table.ts b/resources/Table.ts index d21d942fe3..7dc3134a26 100644 --- a/resources/Table.ts +++ b/resources/Table.ts @@ -2260,8 +2260,10 @@ export function makeTable(options) { recordVersion: options?.version, lockHandle: this.#lockHandle && this.#lockHandle.keyId === writeKeyId(id) ? this.#lockHandle : undefined, commit: (txnTime, existingEntry, _retry, transaction: any) => { + const txnLogKey = + isRocksDB && options?.version != null ? (transaction?.getTimestamp?.() ?? txnTime) : txnTime; write.skipped = false; // reset on each retry; cleanup happens after commit if still true - if (precedesExistingVersion(txnTime, existingEntry, options?.nodeId) <= 0) { + if (precedesExistingVersion(txnTime, existingEntry, options?.nodeId) < 0) { write.skipped = true; return; } @@ -2288,6 +2290,11 @@ export function makeTable(options) { viaNodeId: options?.viaNodeId, transaction, tableToTrack: tableName, + recordVersion: txnTime, + additionalAuditRefs: + isRocksDB && audit && txnLogKey !== txnTime + ? [{ version: txnLogKey, nodeId: options?.nodeId }] + : undefined, }, 'invalidate' ); @@ -2315,7 +2322,9 @@ export function makeTable(options) { ? (this.constructor as any).source.relocate.bind((this.constructor as any).source, id, undefined, context) : undefined, commit: (txnTime, existingEntry, _retry, transaction: any) => { - if (precedesExistingVersion(txnTime, existingEntry, options?.nodeId) <= 0) return; + const txnLogKey = + isRocksDB && options?.version != null ? (transaction?.getTimestamp?.() ?? txnTime) : txnTime; + if (precedesExistingVersion(txnTime, existingEntry, options?.nodeId) < 0) return; const residency = TableResource.getResidencyRecord(options.residencyId); let metadata = 0; let newRecord = null; @@ -2347,6 +2356,11 @@ export function makeTable(options) { viaNodeId: options?.viaNodeId, expiresAt: options.expiresAt, transaction, + recordVersion: txnTime, + additionalAuditRefs: + isRocksDB && audit && txnLogKey !== txnTime + ? [{ version: txnLogKey, nodeId: options?.nodeId }] + : undefined, }, 'relocate', false, @@ -2853,6 +2867,7 @@ export function makeTable(options) { this.#assertLiveHandle(id); const context = this.getContext(); const transaction = txnForContext(context); + const replaying = transaction.isReplay === true; checkValidId(id); if (fullUpdate && recordUpdate == null && options?.isNotification) { // A source/replication-applied put must carry the record; these applies skip record @@ -3150,7 +3165,7 @@ export function makeTable(options) { // would find it and skip the write as "already applied" when the record was never committed. // A recommit of the same transaction survived that skip only because the old write batch // still carried the put; a fresh-transaction replay (ERR_TRY_AGAIN) would drop the write. - if (isRocksDB && !stagedOwnAuditEntry && dedupVersionCouldBeRetained(txnLogKey)) { + if (isRocksDB && !replaying && !stagedOwnAuditEntry && dedupVersionCouldBeRetained(txnLogKey)) { const priorAudit = auditStore.get(txnLogKey, tableId, id, options?.nodeId); if ( priorAudit && @@ -3236,7 +3251,7 @@ export function makeTable(options) { // appended this write's own audit entry, so the lookup would match it while the record was // never committed (see the up-front keyed dedup above). const isReDeliveredDuplicate = () => { - if (stagedOwnAuditEntry) return false; + if (replaying || stagedOwnAuditEntry) return false; if (!dedupVersionCouldBeRetained(txnLogKey)) return false; // pre-retention log key — skip the end-of-log scan (best-effort; see above) const duplicate = auditStore.get(txnLogKey, tableId, id, options?.nodeId); return ( @@ -3267,6 +3282,7 @@ export function makeTable(options) { queuePreviousAuditRefs(auditRecord); if ( isRocksDB && + !replaying && !stagedOwnAuditEntry && localTime === txnLogKey && precedesExistingVersion( @@ -3287,12 +3303,22 @@ export function makeTable(options) { options?.nodeId ); if (precedesExisting === 0) { - logger.debug?.( - 'The transaction time is equal to the existing version, treating as duplicate', - id - ); - write.skipped = true; - return; // treat a tie as a duplicate and drop it + if (isRocksDB && localTime !== txnLogKey) { + // Same origin and record version, but a distinct write. Its per-origin log key + // orders the otherwise non-unique record clock without comparing keys across origins. + precedesExisting = txnLogKey > localTime ? 1 : -1; + } else if (replaying || stagedOwnAuditEntry) { + // The log entry being replayed (or staged by this write's failed attempt) is + // the write itself, not proof that its primary-store mutation committed. + precedesExisting = 1; + } else { + logger.debug?.( + 'The transaction time and log key match the existing write, treating as duplicate', + id + ); + write.skipped = true; + return; + } } if (precedesExisting > 0) { // if the existing version is older, we can skip this update diff --git a/unitTests/resources/dualClockAuditRecord.test.js b/unitTests/resources/dualClockAuditRecord.test.js index 2cdb31c4b7..80cf776735 100644 --- a/unitTests/resources/dualClockAuditRecord.test.js +++ b/unitTests/resources/dualClockAuditRecord.test.js @@ -67,6 +67,22 @@ describe('Dual-clock audit records (harper#2412)', () => { }); } + function invalidateFromOrigin(TableClass, id, partialRecord, { logKey, version, nodeId = 1 }) { + const context = { source: {}, sourceApply: true, timestamp: logKey }; + return transaction(context, async () => { + const resource = await TableClass.getResource(id, context); + return resource._writeInvalidate(id, partialRecord, { nodeId, version }); + }); + } + + function relocateFromOrigin(TableClass, id, { logKey, version, nodeId = 1 }) { + const context = { source: {}, sourceApply: true, timestamp: logKey }; + return transaction(context, async () => { + const resource = await TableClass.getResource(id, context); + return resource._writeRelocate(id, { nodeId, version }); + }); + } + before(async function () { if (isLMDB) return; setupTestDBPath(); @@ -180,6 +196,28 @@ describe('Dual-clock audit records (harper#2412)', () => { assert.deepEqual(olderAudit.previousAdditionalAuditRefs, [{ version: logKey, nodeId: 0 }]); }); + it('distinguishes equal record versions from distinct log-key writes', async function () { + if (isLMDB) return this.skip(); + const id = 'equal-version-distinct-log-key-1'; + const version = Date.now() - 30_000; + await applyFromOrigin(Plain, id, { id, name: 'base', count: 0 }, { logKey: version, version, nodeId: 0 }); + const logKey = Date.now(); + await applyFromOrigin( + Plain, + id, + { count: { __op__: 'add', value: 1 } }, + { logKey, version, nodeId: 0, fullUpdate: false } + ); + assert.equal((await Plain.get(id)).count, 1, 'a distinct equal-version patch must still be folded'); + assert( + Plain.primaryStore + .getEntry(id) + .additionalAuditRefs?.some((ref) => ref.version === logKey && ref.nodeId === 0), + 'the distinct write remains addressable by its log identity' + ); + assert.equal(auditStore.get(logKey, Plain.tableId, id, 0)?.version, version); + }); + it('keeps a log-key pointer to an applied delete whose version differs', async function () { if (isLMDB) return this.skip(); const id = 'applied-delete-head-1'; @@ -211,6 +249,58 @@ describe('Dual-clock audit records (harper#2412)', () => { assert.equal(Plain.primaryStore.getEntry(id), undefined, 'removing the matching audit entry removes its tombstone'); }); + it('keeps a log-key pointer to an applied invalidation whose version differs', async function () { + if (isLMDB) return this.skip(); + const id = 'applied-invalidate-head-1'; + const baseVersion = Date.now() - 30_000; + await applyFromOrigin( + Plain, + id, + { id, name: 'present' }, + { logKey: baseVersion, version: baseVersion, nodeId: 0, isCopyApply: true } + ); + const logKey = Date.now() + 20; + const version = baseVersion; + await invalidateFromOrigin(Plain, id, { id, name: 'present' }, { logKey, version, nodeId: 0 }); + const invalidated = Plain.primaryStore.getEntry(id); + const invalidateAudit = auditEntriesFor(Plain, id).find((entry) => entry.type === 'invalidate'); + assert.equal(invalidated.version, version); + assert( + invalidated.additionalAuditRefs?.some((ref) => ref.version === logKey && ref.nodeId === 0), + `invalidated record must retain ${logKey}; refs=${JSON.stringify(invalidated.additionalAuditRefs)} audit=${JSON.stringify(invalidateAudit)}` + ); + assert.equal(invalidateAudit.type, 'invalidate'); + assert.equal(invalidateAudit.txnLogKey, logKey); + assert.equal(invalidateAudit.version, version); + assert((await Plain.getHistoryOfRecord(id)).some((entry) => entry.localTime === logKey)); + }); + + it('keeps a log-key pointer to an applied relocation whose version differs', async function () { + if (isLMDB) return this.skip(); + const id = 'applied-relocate-head-1'; + const baseVersion = Date.now() - 30_000; + await applyFromOrigin( + Plain, + id, + { id, name: 'present' }, + { logKey: baseVersion, version: baseVersion, nodeId: 0, isCopyApply: true } + ); + const logKey = Date.now() + 21; + const version = baseVersion; + await relocateFromOrigin(Plain, id, { logKey, version, nodeId: 0 }); + const relocated = Plain.primaryStore.getEntry(id); + const relocateAudit = auditEntriesFor(Plain, id).find((entry) => entry.type === 'relocate'); + assert.equal(relocated.version, version); + assert( + relocated.additionalAuditRefs?.some((ref) => ref.version === logKey && ref.nodeId === 0), + `relocated record must retain ${logKey}; refs=${JSON.stringify(relocated.additionalAuditRefs)} audit=${JSON.stringify(relocateAudit)}` + ); + assert.equal(relocateAudit.type, 'relocate'); + assert.equal(relocateAudit.txnLogKey, logKey); + assert.equal(relocateAudit.version, version); + assert((await Plain.getHistoryOfRecord(id)).some((entry) => entry.localTime === logKey)); + }); + it('does not point a copy-applied record at an audit entry that was never written', async function () { if (isLMDB) return this.skip(); const id = 'copy-head-1'; From 8281bd9daeba1c1ef7bbe890263fb54d9384e430 Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Fri, 4 Sep 2026 12:43:55 -0600 Subject: [PATCH 14/16] Format dual-clock regression tests --- unitTests/resources/dualClockAuditRecord.test.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/unitTests/resources/dualClockAuditRecord.test.js b/unitTests/resources/dualClockAuditRecord.test.js index 80cf776735..2c7fe715fb 100644 --- a/unitTests/resources/dualClockAuditRecord.test.js +++ b/unitTests/resources/dualClockAuditRecord.test.js @@ -210,9 +210,7 @@ describe('Dual-clock audit records (harper#2412)', () => { ); assert.equal((await Plain.get(id)).count, 1, 'a distinct equal-version patch must still be folded'); assert( - Plain.primaryStore - .getEntry(id) - .additionalAuditRefs?.some((ref) => ref.version === logKey && ref.nodeId === 0), + Plain.primaryStore.getEntry(id).additionalAuditRefs?.some((ref) => ref.version === logKey && ref.nodeId === 0), 'the distinct write remains addressable by its log identity' ); assert.equal(auditStore.get(logKey, Plain.tableId, id, 0)?.version, version); From 597fdb16d8eb37c89f2efa49a70154f712e570ce Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Fri, 4 Sep 2026 13:33:16 -0600 Subject: [PATCH 15/16] Keep audit head origin paired with version --- resources/RecordEncoder.ts | 15 +++++++++------ resources/Table.ts | 8 ++++++-- resources/crdt.ts | 11 +++++++---- unitTests/resources/dualClockAuditRecord.test.js | 11 +++++++++-- 4 files changed, 31 insertions(+), 14 deletions(-) diff --git a/resources/RecordEncoder.ts b/resources/RecordEncoder.ts index 273fb9e094..9d85946046 100644 --- a/resources/RecordEncoder.ts +++ b/resources/RecordEncoder.ts @@ -908,9 +908,9 @@ export function recordUpdater(store, tableId, auditStore) { metadataInNextEncoding |= HAS_RESIDENCY_ID; extendedType |= HAS_CURRENT_RESIDENCY_ID; } else residencyIdAtNextEncoding = 0; - const nodeId = options?.nodeId ?? (audit ? getThisNodeId(auditStore) : undefined); - if (nodeId >= 0) { - nodeIdAtNextEncoding = nodeId; + const recordNodeId = options?.recordNodeId ?? options?.nodeId ?? (audit ? getThisNodeId(auditStore) : undefined); + if (recordNodeId >= 0) { + nodeIdAtNextEncoding = recordNodeId; metadataInNextEncoding |= HAS_NODE_ID; } else nodeIdAtNextEncoding = -1; const additionalAuditRefs = options?.additionalAuditRefs; @@ -985,10 +985,13 @@ export function recordUpdater(store, tableId, auditStore) { if (resolveRecord && existingEntry?.localTime) { let replacingId = existingEntry.localTime; let replacingEntry; - if (isRocksDB) { - for (const ref of existingEntry.additionalAuditRefs ?? []) { + if (isRocksDB && existingEntry.additionalAuditRefs) { + for (const ref of existingEntry.additionalAuditRefs) { const candidate = auditStore.get(ref.version, tableId, id, ref.nodeId); - if (candidate?.version === existingEntry.version) { + if ( + candidate?.version === existingEntry.version && + (candidate.nodeId ?? 0) === (existingEntry.nodeId ?? 0) + ) { replacingId = ref.version; replacingEntry = candidate; break; diff --git a/resources/Table.ts b/resources/Table.ts index 7dc3134a26..e053d23d0f 100644 --- a/resources/Table.ts +++ b/resources/Table.ts @@ -717,6 +717,7 @@ export function makeTable(options) { nodeId: number | undefined, refs?: Array<{ version: number; nodeId: number }> ) { + if (!refs?.length) return { txnLogKey: version, nodeId }; const visited = new Set(); function findHead(candidateRefs?: Array<{ version: number; nodeId: number }>) { if (!candidateRefs) return; @@ -728,7 +729,8 @@ export function makeTable(options) { visited.add(identity); const entry = auditStore.getSync(ref.version, tableId, id, ref.nodeId); if (!entry) continue; - if (entry.version === version) return { txnLogKey: ref.version, nodeId: ref.nodeId }; + if (entry.version === version && (entry.nodeId ?? 0) === (nodeId ?? 0)) + return { txnLogKey: ref.version, nodeId: ref.nodeId }; const previousRefs = entry.previousAdditionalAuditRefs; if (previousRefs) { for (let index = previousRefs.length - 1; index >= 0; index--) pending.push(previousRefs[index]); @@ -739,7 +741,8 @@ export function makeTable(options) { if (referencedHead) return referencedHead; if (version != null) { const directHead = auditStore.getSync(version, tableId, id, nodeId); - if (directHead?.version === version) return { txnLogKey: version, nodeId }; + if (directHead?.version === version && (directHead.nodeId ?? 0) === (nodeId ?? 0)) + return { txnLogKey: version, nodeId }; } return { txnLogKey: version, nodeId }; } @@ -3619,6 +3622,7 @@ export function makeTable(options) { residencyId, expiresAt, recordVersion: txnTime, + recordNodeId: precedesExisting < 0 ? existingEntry?.nodeId : options?.nodeId, nodeId: options?.nodeId, viaNodeId: options?.viaNodeId, originatingOperation: (context as any)?.originatingOperation, diff --git a/resources/crdt.ts b/resources/crdt.ts index e3d52debc0..fd35ece3f6 100644 --- a/resources/crdt.ts +++ b/resources/crdt.ts @@ -161,8 +161,9 @@ function reconstructForward(auditStore, store, tableId: number, recordId: any, f } function resolveAuditPosition(auditStore, tableId: number, recordId: any, version, nodeId, refs) { + if (!refs?.length) return { txnLogKey: version, nodeId }; const visited = new Set(); - const pending = (refs ?? []).slice().reverse(); + const pending = refs.slice().reverse(); while (pending.length > 0) { const ref = pending.pop(); const identity = `${ref.nodeId ?? 0}:${ref.version}`; @@ -170,9 +171,11 @@ function resolveAuditPosition(auditStore, tableId: number, recordId: any, versio visited.add(identity); const entry = auditStore.get(ref.version, tableId, recordId, ref.nodeId); if (!entry) continue; - if (entry.version === version) return { txnLogKey: ref.version, nodeId: ref.nodeId }; - for (const previousRef of (entry.previousAdditionalAuditRefs ?? []).slice().reverse()) { - pending.push(previousRef); + if (entry.version === version && (entry.nodeId ?? 0) === (nodeId ?? 0)) + return { txnLogKey: ref.version, nodeId: ref.nodeId }; + const previousRefs = entry.previousAdditionalAuditRefs; + if (previousRefs) { + for (let index = previousRefs.length - 1; index >= 0; index--) pending.push(previousRefs[index]); } } return { txnLogKey: version, nodeId }; diff --git a/unitTests/resources/dualClockAuditRecord.test.js b/unitTests/resources/dualClockAuditRecord.test.js index 2c7fe715fb..3955e6d69c 100644 --- a/unitTests/resources/dualClockAuditRecord.test.js +++ b/unitTests/resources/dualClockAuditRecord.test.js @@ -32,7 +32,9 @@ describe('Dual-clock audit records (harper#2412)', () => { type: auditRecord.type, version: auditRecord.version, txnLogKey: auditRecord.txnLogKey, + nodeId: auditRecord.nodeId, previousVersion: auditRecord.previousVersion, + previousNodeId: auditRecord.previousNodeId, previousAdditionalAuditRefs: auditRecord.previousAdditionalAuditRefs, }); } @@ -182,13 +184,18 @@ describe('Dual-clock audit records (harper#2412)', () => { { logKey: olderLogKey, version: olderVersion, nodeId: 2, fullUpdate: false } ); assert.deepEqual(await Plain.get(id), { id, name: 'newer', count: 1 }); + assert.equal( + Plain.primaryStore.getEntry(id).nodeId, + 0, + 'the stored node id stays paired with the surviving record version' + ); assert( Plain.primaryStore.getEntry(id).additionalAuditRefs?.some((ref) => ref.version === logKey && ref.nodeId === 0), 'an out-of-order merge must retain the surviving head in the log-key domain' ); assert( (await Plain.getHistoryOfRecord(id)).some((entry) => entry.localTime === logKey), - 'an audit-only fold must not displace the real head of the surviving record' + `an audit-only fold must not displace the real head of the surviving record: ${JSON.stringify(auditEntriesFor(Plain, id))}` ); const olderAudit = auditEntriesFor(Plain, id).find((entry) => entry.txnLogKey === olderLogKey); assert.equal(olderAudit.version, olderVersion, "crash replay must see the folded write's original version"); @@ -433,7 +440,7 @@ describe('Dual-clock audit records (harper#2412)', () => { // auditStore.get(key, ...) walks the entries at one log key; a fill's entry sits at its commit // key while the record itself stores the source version, so keying by version must not find it. const id = 'lookup-1'; - const logKey = Date.now() + 3; + const logKey = Date.now() + 1_000; const version = logKey - 120_000; // nodeId 0 so the entry lands in — and is read back from — the one log a single-node test has await applyFromOrigin(Plain, id, { id, name: 'lookup' }, { logKey, version, nodeId: 0 }); From 755329ed1f7a21255a623ebe503cd8d27fbc64d9 Mon Sep 17 00:00:00 2001 From: Kris Zyp Date: Fri, 4 Sep 2026 13:39:10 -0600 Subject: [PATCH 16/16] Resolve audit heads within the record origin --- resources/RecordEncoder.ts | 2 +- resources/Table.ts | 4 ++-- resources/crdt.ts | 2 +- unitTests/resources/crdt.test.js | 3 +++ 4 files changed, 7 insertions(+), 4 deletions(-) diff --git a/resources/RecordEncoder.ts b/resources/RecordEncoder.ts index 9d85946046..bc2a0baf9a 100644 --- a/resources/RecordEncoder.ts +++ b/resources/RecordEncoder.ts @@ -998,7 +998,7 @@ export function recordUpdater(store, tableId, auditStore) { } } } - replacingEntry ??= auditStore.get(replacingId, tableId, id); + replacingEntry ??= auditStore.get(replacingId, tableId, id, existingEntry.nodeId); if (replacingEntry) { const previousVersion = replacingEntry.previousVersion; result = auditStore[isRocksDB ? 'putSync' : 'put']( diff --git a/resources/Table.ts b/resources/Table.ts index e053d23d0f..0b9b74853b 100644 --- a/resources/Table.ts +++ b/resources/Table.ts @@ -729,7 +729,7 @@ export function makeTable(options) { visited.add(identity); const entry = auditStore.getSync(ref.version, tableId, id, ref.nodeId); if (!entry) continue; - if (entry.version === version && (entry.nodeId ?? 0) === (nodeId ?? 0)) + if (entry.version === version && (nodeId == null || (entry.nodeId ?? 0) === nodeId)) return { txnLogKey: ref.version, nodeId: ref.nodeId }; const previousRefs = entry.previousAdditionalAuditRefs; if (previousRefs) { @@ -741,7 +741,7 @@ export function makeTable(options) { if (referencedHead) return referencedHead; if (version != null) { const directHead = auditStore.getSync(version, tableId, id, nodeId); - if (directHead?.version === version && (directHead.nodeId ?? 0) === (nodeId ?? 0)) + if (directHead?.version === version && (nodeId == null || (directHead.nodeId ?? 0) === nodeId)) return { txnLogKey: version, nodeId }; } return { txnLogKey: version, nodeId }; diff --git a/resources/crdt.ts b/resources/crdt.ts index fd35ece3f6..e65768d38d 100644 --- a/resources/crdt.ts +++ b/resources/crdt.ts @@ -171,7 +171,7 @@ function resolveAuditPosition(auditStore, tableId: number, recordId: any, versio visited.add(identity); const entry = auditStore.get(ref.version, tableId, recordId, ref.nodeId); if (!entry) continue; - if (entry.version === version && (entry.nodeId ?? 0) === (nodeId ?? 0)) + if (entry.version === version && (nodeId == null || (entry.nodeId ?? 0) === nodeId)) return { txnLogKey: ref.version, nodeId: ref.nodeId }; const previousRefs = entry.previousAdditionalAuditRefs; if (previousRefs) { diff --git a/unitTests/resources/crdt.test.js b/unitTests/resources/crdt.test.js index 30b032f03b..ab3d232a99 100644 --- a/unitTests/resources/crdt.test.js +++ b/unitTests/resources/crdt.test.js @@ -23,6 +23,7 @@ function makeStore(events) { return { type: event.type, version: event.version, + nodeId: event.nodeId ?? 1, previousVersion: event.previousVersion, previousNodeId: event.previousNodeId, previousAdditionalAuditRefs: event.previousAdditionalAuditRefs, @@ -78,6 +79,7 @@ describe('crdt getRecordAtTime', () => { const store = makeStore(events); const current = currentEntry({ id: 'D', count: 3 }, 20, { version: 20, + nodeId: 1, additionalAuditRefs: [{ version: 300, nodeId: 1 }], }); assert.deepStrictEqual(getRecordAtTime(current, 150, store, 1, 'D'), { id: 'D', count: 1 }); @@ -114,6 +116,7 @@ describe('crdt getRecordAtTime', () => { const store = makeStore(events); const current = currentEntry({ id: 'D', count: 9 }, 40, { version: 40, + nodeId: 1, additionalAuditRefs: [{ version: 400, nodeId: 1 }], }); assert.deepStrictEqual(getRecordAtTime(current, 250, store, 1, 'D'), { id: 'D', count: 3 });