Skip to content

Rebuild structures from durable store when a save is declined - #186

Merged
kriszyp merged 1 commit into
v1.11.xfrom
kris/savestructures-uninitialized-on-failure
Jun 8, 2026
Merged

kriszyp merged 1 commit into
v1.11.xfrom
kris/savestructures-uninitialized-on-failure

Conversation

@kriszyp

@kriszyp kriszyp commented Jun 8, 2026

Copy link
Copy Markdown
Owner

Summary

When saveStructures returns false (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 via getStructures, 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 saveStructures runs. 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 throws Record id is not defined for N — the production decode error on CDI's rt-dev cluster.

resetStructures only nulls transitions past a 10k-transition threshold, so the poisoned trie normally survives the re-pack. This extends the long-standing TODO on the adjacent encodingError branch ("make the structures uninitialized so they get rebuilt next time") to the save-failure path.

What to look at

  • One line in pack.js (the saveStructures(...) === false branch): structures.uninitialized = true before the re-pack. On the re-pack, the existing if (structures.uninitialized) reload (line ~91) pulls durable structures via getStructures, and if (!structures.transitions) rebuilds the trie.
  • Pairing: this is half of the fix. The host's saveStructures must also return false (not undefined) 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").
  • Re-pack termination: the re-pack recurses on persistent 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.x maintenance branch for a 1.11.14 patch (deployed 5.0.x line). Will port to master (v2 / 5.1) as well.

🤖 Generated by Claude (Opus 4.7).

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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant