fix(core): scope the state-sync truncate to the tables the snapshot restores - #551
Open
rickyrombo wants to merge 1 commit into
Open
fix(core): scope the state-sync truncate to the tables the snapshot restores#551rickyrombo wants to merge 1 commit into
rickyrombo wants to merge 1 commit into
Conversation
…estores Before loading a snapshot, the restore truncated every table in the public schema. The reason it truncates at all is narrow: the node runs migrations at startup, which populates core_db_migrations, and COPY then fails on duplicate keys. Truncating a table the dump does not contain cannot prevent that -- nothing COPYs into it -- so every table outside stateSyncSnapshotTables was destroyed for no reason. Postgres is shared between core and mediorum (one OPENAUDIO_DB_URL, one public schema), so the collateral was mediorum's uploads, blobs, audio_previews and qm_audio_analyses on every node that state synced. blobs is rebuilt by relisting the bucket, and uploads eventually refills via scrollUploadsFromPeers, but the previews and analyses are recomputed from audio -- real CPU, network-wide, whenever a node joins. Truncate the intersection of stateSyncSnapshotTables with the tables that exist, in one statement without CASCADE. The intersection matters because the pre-data restore deliberately tolerates schema drift, so a listed table may be absent. Dropping CASCADE matters because it follows foreign keys outward: a key from a non-snapshot table into a snapshot table would silently truncate the non-snapshot table too, which is this same bug through a different door. Failing loudly is the outcome we want there. Tests cover the scoping, the intersection, and the absence of CASCADE, and were confirmed to fail against the previous behaviour.
rickyrombo
added a commit
that referenced
this pull request
Aug 26, 2026
…d with the code Six places where the reference either contradicted the runbook or stated something the code does not do: - The facts table gave prod's validator count as 9. That is the genesis list; the live set is 67 of 72 registered nodes, and the runbook's quorum and jailing math is over the live set. - Section 7 said to leave BlockInterval at its default, which the runbook now overrides with 20,000. Records why the default is wrong here, that it is a producer-side setting, and that retention is Keep x BlockInterval so it cannot be lowered freely. - The state-sync truncation table listed blobs but not uploads, audio_previews or qm_audio_analyses, which are also wiped. Adds them with how each recovers, and notes #551 makes the row moot. - Section 3 asserted the bootstrap validator holds power 100. Its own registration rewrites that to ValidatorVotingPower, 10 on mainnet, because the writer never seeds core_registered_nodes. - The cursor discussion now points at api#1018, which is open and must land before flushing is first enabled. - Section 11 omitted the chain-aware fallback to core_indexed_blocks, which is only reached on ErrNoRows and so never runs on a database that has indexed the old chain. That is why the failure is a silent stall.
rickyrombo
added a commit
that referenced
this pull request
Aug 28, 2026
Every step that cannot be executed without a code change now names it, so the dependency is visible where the work happens rather than only in a list at the top: #553 for the binary (step 3), api#1029 for play routing (5 and 13), #551 before the fleet state syncs (10), api#1018 before flushing (11), api#1028 for the indexer bounds (12). Also records two things found while deriving the bootstrap's identity for #553: the node key is the comet key, so a node's P2P id is its validator address and the bootstrap's is derivable before it runs; and the bootstrap must not inherit node_key.json from the writer output, or its id will not match the peer list.
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.
The bug
Before loading a snapshot, the restore truncates every table in the
publicschema:The reason it truncates at all is narrow, and the comment says so: the node runs migrations at startup, which populates
core_db_migrations, andpg_restore's COPY then fails on duplicate keys.But only
stateSyncSnapshotTablesis ever dumped. Truncating a table the dump doesn't contain cannot prevent a COPY conflict, because nothing will COPY into it. Every table outside that list was being destroyed for no reason at all.What that costs
Core and mediorum share one
OPENAUDIO_DB_URLand therefore onepublicschema, so the collateral is mediorum's data. Any node that state syncs loses:blobsuploadsscrollUploadsFromPeersaudio_previewsfindMissedJobs→generateAudioPreviewForUploadqm_audio_analysesupload_cursors,repair_trackers, delist tablesNothing is permanently lost, but the last two are real CPU, on every node that joins. This matters most during a fleet-wide event where many nodes state sync at once — they all start regenerating simultaneously.
The fix
Truncate the intersection of
stateSyncSnapshotTableswith the tables that actually exist, in a single statement, withoutCASCADE.The intersection is needed because
pgRestore("pre-data")deliberately tolerates schema drift ("pre-data errors (missing tables from schema drift) are non-fatal"), so a listed table may not exist locally. Naming a missing table would fail the whole statement.Dropping
CASCADEis deliberate.CASCADEfollows foreign keys outward: a key from a non-snapshot table into a snapshot table would silently truncate the non-snapshot table too — this same bug through a different door. Truncating the set together in one statement satisfies foreign keys among its members, and if something outside the set references something inside it, this now fails loudly. That's the outcome we want.Tests
Three, all database-free so they run in CI, following the existing
state_sync_snapshot_tables_test.gopattern:etl_*) are never named, whilecore_db_migrations— the table that motivated truncating in the first place — still isCASCADE, and exactly oneTRUNCATEVerified they fail against the previous behaviour, not just pass against the new one:
Scope
Behaviour change is confined to which tables get truncated during a state-sync restore. The dump, the COPY, and the restore sequence are untouched.
One thing deliberately left out:
etl_*tables are chain-scoped and preserving them across a chain switch leaves stale rows. Today they are cleared as a side effect of this workaround — right for the wrong reason. If that needs to hold it should be an explicit "chain changed, clear ETL state" check rather than a byproduct of a COPY fix, and it is moot while no node runs the ETL.