Expose record version and txnLogKey as separate audit clocks - #2497
Draft
kriszyp wants to merge 16 commits into
Draft
Expose record version and txnLogKey as separate audit clocks#2497kriszyp wants to merge 16 commits into
kriszyp wants to merge 16 commits into
Conversation
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces dual-clock audit records (harper#2412 stage 0b) to separate a record's own version (version) from its transaction log key (localTime), preventing issues where the two clocks differ during source fills or replication. The feedback identifies a high-severity bug in LMDBTransaction.ts where the transaction argument is omitted in write.commit, causing logTime to incorrectly fall back to the record version and breaking dual-clock separation on LMDB. Additionally, the feedback suggests using strict assertions (assert.strictEqual and assert.deepStrictEqual) in the new test files to prevent type-coercion bugs.
…DB audit records 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WMMWWeFsqFuKduJHNMP6ZY
kriszyp
force-pushed
the
kris/harper-2412-stage0b-dual-clock
branch
from
September 4, 2026 17:52
eda1ffb to
9211de8
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
RocksDB audit records have two clocks with different jobs:
versionis the record's LWW version, whiletxnLogKeyis the entry's position in the origin node's transaction log.versiondrives ordering,@updatedTime, and ETags;txnLogKeydrives replay, resume, grouping, and write identity.Previously the internal audit API called the second value
localTime, inherited from the deprecated LMDB model. That name is actively misleading once a replicated entry preserves both the origin's record version and the origin's log position. This PR makestxnLogKeythe canonical internal name.localTimeremains only on the existing listener, history, and pro-to-core compatibility payloads; this is not an API migration.Why the clocks must remain separate
For an ordinary local write, both values are the transaction timestamp. They diverge for a source fill: the record keeps the source's
lastModified, while its audit entry is committed under the fill transaction's log key. Usingversionas a cursor moves replication into the record-ordering clock domain; usingtxnLogKeyas the record version can make a legitimate later write look stale.Applied and replayed writes therefore preserve the body
version, but bound the effective write version tomin(version, txnLogKey). The bound is required by the current audit format: an audit-only out-of-order entry can carry the later surviving record version in its body. Without the bound, recovery could resurrect the superseded value at that later version.What changes
{ version, txnLogKey }internally on both storage engines.txnLogKey; each write retains its ownversion.txnLogKeydomain.(nodeId, txnLogKey)identity, including equal record versions with distinct same-origin log keys; crash replay does not treat the audit entry it is replaying as proof that the primary mutation committed.(nodeId, txnLogKey), including the fallback audit walk when an exact keyed lookup misses.localTimefield.This is the core half of Harper Pro #812, "Preserve origin record versions and transaction-log keys during replication". The two PRs ship as one artifact.
Verification
npm run build: clean.The focused suites above were rerun on the exact pushed head. CI remains the broader verification surface.
Scope and follow-ups
This is the naming and receive/replay stage of #2412. It deliberately does not redesign the stored audit entry. A later format stage can replace the additional audit-head reference, remove the overloaded audit-only body version, and eliminate the temporary
min(version, txnLogKey)normalization. Publish messages remain outside sourced-record clock separation: a legitimate locally originated message hasversion === txnLogKeyand does not carry a source-owned record version. Deprecated LMDB keeps its legacy transaction-version apply behavior; it is not the vocabulary source or a constraint on the RocksDB contract.Refs #2412
Refs #2411
Review-Coverage: authored=unknown; ran=none; rounds=1 @ 755329e
Human-Review-Need: 4 @ 755329e