Flag structure updates per transaction log so replicas decode against current structures (#1348) - #1352
Conversation
… current structures (#1348) HAS_STRUCTURE_UPDATE is a one-shot, process-wide-per-table flag: saveStructures sets it on a mint and the next audit write to that table consumes it. Transaction logs are partitioned per origin node (RocksTransactionLogStore.logById) and interleave entries from every table, so the flag can be recorded in a different per-node log than the one whose entries first reference the new structure. A linear reader of that other log (the replication send path) then never resends TABLE_FIXED_STRUCTURE and the peer decodes later entries against a stale structure set — records come back undecodable / null (#1348). put() now re-derives HAS_STRUCTURE_UPDATE per (log, tableId) from the monotonic structureVersion, so any entry that advances a (log, table)'s structure count carries the flag regardless of where the one-shot flag was consumed. The watermark is keyed by (log, tableId) because a per-node log interleaves tables while structureVersion is per-table, and it advances only on durable commit (a deferred hook) so an aborted/discarded entry can't raise it and suppress a later entry's flag. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates RocksTransactionLogStore to track structure versions per transaction log and table ID using a WeakMap. It ensures that the HAS_STRUCTURE_UPDATE flag is set on any entry that advances the structure version for its specific log and table, and defers advancing the watermark until the transaction is durably committed. Additionally, a comprehensive set of unit tests has been added to verify this behavior under various scenarios, including aborted transactions and multi-table logs. There are no review comments, and I have no feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
Reviewed; no blockers found. |
|
Heads up for reviewers: the red Integration Tests 4/6 (Node v22/24/26) is pre-existing on (Generated by Claude — Opus 4.8.) |
1 participant
Summary
RocksTransactionLogStore.put()now re-derivesHAS_STRUCTURE_UPDATEper(log, tableId)from the monotonicstructureVersion, advancing the per-(log, table)watermark only on durable commit.Purpose — fixes #1348
On a production cluster, replicated records intermittently decode to
nullwithError decoding record: Data read, but end of buffer not reached. The application swallows it (Promise.allSettled+ empty fallback), so it surfaces as silent degradation — affected records serve incomplete.Root cause:
HAS_STRUCTURE_UPDATEis a one-shot, process-wide-per-table flag —saveStructuressets it on a structure mint and the next audit write to that table consumes it (RecordEncoder.ts). But transaction logs are partitioned per origin node (logById) and interleave entries from every table, so the flag can be recorded in a different per-node log than the one whose entries first reference the new structure. A linear reader of that other log — the replication send path, which reloads structures on the flag (harper-pro replicationConnection.ts) — then never resendsTABLE_FIXED_STRUCTURE, and the peer decodes later entries against a stale structure set → undecodable / null.The fix makes every per-node log self-describing: any entry advancing a
(log, table)'s structure count carries the flag, regardless of where the one-shot flag was consumed.Where to put attention
put()+ a unified commit hook). I folded the existingaftercommitblock into a single per-transactiononCommitthat does both the emit and the watermark advance, so the watermark rises only on a durable commit — an aborted/discarded entry must not raise it and suppress a later entry's flag. This store is the sole setter oftransaction.onCommit. Please confirm theaftercommitsemantics read as preserved (the resources suite, incl.auditLog.test.js, passes).(log, tableId), not per-log. A per-node log interleaves tables whilestructureVersionis per-table; a log-wide watermark would let a high-version table suppress another table's first use of a new structure. (Caught in review.)Uint8Arrayentry branch is intentionally not re-derived — pre-encoded entries already carry the correct flag; re-deriving would break cross-node byte parity.(log, table); never an under-flag.ascii8→string8promotion is deliberately not flagged — it is decode-equivalent (ASCII bytes are valid UTF-8 and both types decode via the samereadStringpath instructon), so it needs no resend. New structures (count increases) are the only decode-relevant change.Validation
transactionLogStructureUpdate.test.js): monotonic flagging, per-log independence, per-(log,table)independence, commit-deferred/abort-safety, flag preservation.test:unit:resourcesgreen.Generated by Claude (Opus 4.8). Cross-model reviewed (Codex + Gemini); the two findings they raised (per-table keying, commit-deferral) are addressed in the diff.