Repository navigation
Rebuild structures from durable store when a save is declined - #186
Merged
Merged
Conversation
When saveStructures returns false — a concurrent writer updated the shared structures, or the host store transaction did not durably commit — the encoder's in-memory structures and transition trie may reference record ids that were never persisted. The existing re-pack (`return packr.pack(value)`) reused the cached transition and re-emitted the SAME record pointing at an unpersisted structure, so a fresh reader (another thread / after restart) throws "Record id is not defined for N". Mark the structures uninitialized before the re-pack so it reloads the durable structures via getStructures, rebuilds the transition trie, and re-mints + re-saves. Extends the long-standing TODO on the adjacent encodingError branch to the save-failure path. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This was referenced Jun 8, 2026
kriszyp
marked this pull request as ready for review
June 8, 2026 11:37
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.
Summary
When
saveStructuresreturnsfalse(a concurrent writer updated the shared structures, or the host store transaction did not durably commit), mark the in-memory structures uninitialized before re-packing so the re-pack reloads the durable structures viagetStructures, rebuilds the transition trie, and re-mints + re-saves.Why
The encoder mints a shared-structure record id and encodes the record synchronously, before the host's
saveStructuresruns. The existing re-pack on a declined save (return packr.pack(value)) reused the cached transition trie, so it re-emitted the same record pointing at a structure id that was never persisted. A fresh reader (another worker thread, or after restart) then throwsRecord id is not defined for N— the production decode error on CDI's rt-dev cluster.resetStructuresonly nullstransitionspast a 10k-transition threshold, so the poisoned trie normally survives the re-pack. This extends the long-standing TODO on the adjacentencodingErrorbranch ("make the structures uninitialized so they get rebuilt next time") to the save-failure path.What to look at
pack.js(thesaveStructures(...) === falsebranch):structures.uninitialized = truebefore the re-pack. On the re-pack, the existingif (structures.uninitialized)reload (line ~91) pulls durable structures viagetStructures, andif (!structures.transitions)rebuilds the trie.saveStructuresmust also returnfalse(notundefined) on a non-committed save — see the companion harper change. Verified end-to-end: declined save → re-pack reloads + re-mints + re-saves → no dangling record (regression test added: "declined structure save re-packs against durable structures").false. In practice declines converge (CAS conflict → merge winner's structures → next save compatible; aborts are transient). A persistent-decline guard could be added but matches the pre-existing re-pack behavior.Tests
102 passing (added the declined-save regression). Targets the new
v1.11.xmaintenance branch for a 1.11.14 patch (deployed 5.0.x line). Will port tomaster(v2 / 5.1) as well.🤖 Generated by Claude (Opus 4.7).