fix: testnet snapshot creation and restoration - #166
Open
OlufemiAdeOlusile wants to merge 3 commits into
Open
Conversation
test_local_snapshot_restoration.py assumed snapshot_creation ran first based on file collection order, but pytest collects it before test_snapshot_creation.py alphabetically. It now takes snapshot_created as a direct fixture dependency, so pytest always runs creation first regardless of file order, and uses the returned data directly instead of re-reading it from disk. postgres-start.sh killed only the first PID returned by lsof, quoting the full multi-line output into one broken kill argument whenever more than one process held the port. It now kills each PID in a loop and waits for the port to clear before wiping the data directory.
Importing snapshot_created into test_local_snapshot_restoration.py made pytest run it twice, once per module that requests it, since pytest does not cache an imported fixture across the module that defines it and the module that imports it. Moved it into conftest.py so there is exactly one shared instance regardless of which test file requests it first. Separately, stage 2 of snapshot creation returned the snapshot file name as-is from the script output, relative to cardano-db-sync (its own cwd), but callers resolved it against their own cwd instead. The restore step then looked for the file in the wrong directory. Stage 2 now returns the full path anchored to cardano-db-sync.
An era that finishes syncing within the same second it started (e.g. resuming from an already-mostly-synced db, as snapshot restoration does) rounds sync_duration_secs to 0, and dividing slots by that crashed. Sync speed is undefined in that case, so it is now 0 instead of raising.
5 tasks
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
Arthur asked about deleting the snapshot creation/restoration tests (#164), since he'd never run them through this codebase and assumed they didn't work anymore. Turned out the testnet path (create a snapshot, restore it, restart db-sync, confirm it resyncs) already existed and matches what he actually needs, but it hadn't been exercised in CI in months. Ran it for real and it failed. This fixes what was actually broken.
Five bugs, each only visible after fixing the one before it:
postgres-start.shonly killed the first PIDlsofreturned, so a port with more than one listener (normal after a real sync) left postgres alive and the following cleanup crashed. Now kills each PID and waits for the port to clear.conftest.py, the only place that guarantees one shared instance.Confirmed passing end to end on
preview: https://github.com/IntersectMBO/cardano-sync-tests/actions/runs/31015477492 (all 5 snapshot creation/restoration tests pass)One unrelated, pre-existing test (
test_sync_dbsync.py::test_perf_stats_written) failed in that same run. Different test failed differently in the run before that too. Looks like flakiness from dispatching several runs back to back on the same self-hosted runner, not something these changes touch. Not fixed here.test_iohk_snapshot_restoration.py(official mainnet-only snapshot restore) is untouched, separate question.Test plan
run_only_sync_test=false, twice more after intermediate failures, until clean: https://github.com/IntersectMBO/cardano-sync-tests/actions/runs/31015477492