Repository navigation
fix(storage): write index nodes append-only so older snapshots survive compaction - #34
Merged
Merged
Conversation
…e compaction Index node writes reused previous storage addresses, so a stored snapshot holding deferred refs to those addresses silently observed post-flush content (e.g. report.db_before appeared to already contain the tx's own datoms after the compaction flush inside transact_conn/apply_report, making before = after for downstream delta computation). store_node now always allocates a fresh address in both buffered_node_storage and restoring_node_storage; obsolete addresses are reclaimed by collect_garbage. Adds a regression test that restores a conn and forces a compaction flush, then asserts the pre-transaction snapshot still reads its original datoms.
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
devin-ai-integration Bot
pushed a commit
to logseq/logseq
that referenced
this pull request
Oct 4, 2026
datascript-ocaml 3d6989d (logseq/datascript-ocaml#34): store_node no longer overwrites in-use addresses, so deferred refs in older snapshots (db_before) survive compaction — fixes empty :children patches after insertBatchBlock.
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
1 participant
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
Index node writes used to reuse each node's previous storage address (
store_node ?address), so a stored snapshot that still held deferred refs to an address would silently resolve to post-flush content after the tail-compaction store insidetransact_conn/apply_report. Concretely,report.db_beforeappeared to already contain the tx's own datoms —before = afterfor any consumer reading the pre-transaction snapshot (e.g. the Logseq db-worker'srender_deltamembership-at, which produced empty:childrenpatches so inserted blocks never rendered).buffered_node_storageandrestoring_node_storagenow ignore the inherited address and always allocate a fresh one — index writes are append-only. Write volume per flush is unchanged (same dirty nodes written, just at new addresses); superseded addresses become garbage and are reclaimed bycollect_garbage, matching the cljs fork's stored_dbs/GC model rather than its in-place reuse. The reuse in cljs latentlly corrupts snapshots the same way; logseq's cljs fork has the same overwrite-then-notify ordering inStorageAdapter.store/conn._transact_BANG_.Includes a regression test
test_db_before_snapshot_survives_compaction_store: restore a conn from storage, transact past the branching-factor compaction threshold, and assertreport.db_beforeand a captured snapshot still read exactly the pre-tx datoms (wasexpected 200, got 260before the fix).Link to Devin session: https://app.devin.ai/sessions/f1e2f055a0e14c7abf889688d09c8371
Open in Devin Desktop: https://app.devin.ai/desktop/session/f1e2f055a0e14c7abf889688d09c8371?variant=devin
Requested by: @tiensonqin