feat(sei-db): roll back state store from snapshot and WAL - #3967
Conversation
Integrate state-store rollback with seid rollback by restoring a retained snapshot and replaying its changelog to the target. Anchor WAL retention to snapshots and make every rollback mutation durable and crash-resumable. Co-authored-by: Cursor <cursoragent@cursor.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
PR SummaryHigh Risk Overview Rollback is crash-resumable: a durable target marker, directory swap, and reopen recovery keep the snapshot, changelog, and version watermark consistent after a crash. Snapshot retention now anchors changelog prune to the oldest retained snapshot without dropping below the count-based recovery floor, so empty-block gaps remain replayable. Reviewed by Cursor Bugbot for commit 8b6e603. Bugbot is set up for automated code reviews on this repo. Configure here. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3967 +/- ##
==========================================
- Coverage 58.57% 57.60% -0.98%
==========================================
Files 2318 2232 -86
Lines 198159 188834 -9325
==========================================
- Hits 116077 108773 -7304
+ Misses 71352 70011 -1341
+ Partials 10730 10050 -680
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 85b0437. Configure here.
| // that starts at the gap proves nothing and is refused. | ||
| if next == oldest && next != base+1 { | ||
| return 0, fmt.Errorf("cannot roll back state store to version %d: the changelog starts at version %d, above snapshot %d, so the versions in between may have been pruned", target, next, base) | ||
| } |
There was a problem hiding this comment.
Empty-block tail rollback rejected
Medium Severity
rollbackBaseVersion refuses a target above the snapshot when WALVersionsAfter reports no newer changelog entry. That is also what happens when every height after the snapshot wrote nothing, which recovery already handles by raising the watermark to the pending target. The pre-flight then skips state-store rollback, so seid rollback rewinds commit store and Tendermint while the state store stays ahead.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 85b0437. Configure here.
![seidroid[bot]](https://avatars.githubusercontent.com/in/2745135?s=60&v=4){kind=small}
There was a problem hiding this comment.
A large, carefully sequenced state-store rollback feature with strong crash-recovery test coverage (every step of the swap, both directory layouts, WAL retention bounds). No blocking defects found; two narrow durability/completeness gaps in the rollback plan and marker-clearing paths are worth addressing.
Findings: 0 blocking | 4 non-blocking | 2 posted inline
Blockers
- None at the file/PR level.
Non-blocking
Manager.PruneSnapshots/prunenow callPruneWALBeforeVersiononm.scheduler, andDatabase.PruneWALBeforeVersion/WALVersionsAfterreaddb.streamHandlerwithout holding a lock whileDatabase.Closenils it. The composite's own coordinator is stopped beforecosmosStore.Close(), so the in-process path is ordered, but the external GC path (management/gc/storage_garbage_collector.go->PruneSnapshots) is not obviously ordered against close. Worth confirming the GC is quiesced before close, or guarding the handle.removeSnapshotsAbovedeletes snapshot directories above the target but leaves the snapshot root'scurrentsymlink pointing at a removed directory. It self-heals becausesnapshot.Manager.OpencallsupdateCurrentLink(m.Newest()), but nothing heals it if snapshots are subsequently disabled (SnapshotInterval <= 0), andcurrentSnapshotVersion()reads a dangling link successfully in the meantime. Cheap to update the link in the same step.- 2 suggestion(s)/nit(s) flagged inline on specific lines.
| pendingRollbackTarget, err) | ||
| } | ||
| } | ||
| if err := clearRollbackTarget(dbHome); err != nil { |
There was a problem hiding this comment.
[suggestion] Durability asymmetry between the marker clear and the writes it protects: clearRollbackTarget fsyncs the directory entry, but SetLatestVersion (and the replay writes above it) go through defaultWriteOpts = pebble.NoSync (sei-db/db_engine/pebbledb/mvcc/db.go:68, :473).
Most of the exposure is covered by ordinary recovery: the changelog is cut to the target and still reaches down to the restored snapshot, so RecoverCompositeStateStore replays the lost range on the next open. The case that does not heal is exactly the one this block exists for — a target whose height wrote no changelog entry. There, SetLatestVersion(pendingRollbackTarget) is the only record of the advance; if a power loss lands after the marker removal is durable but before Pebble flushes, the next open replays to the last entry at or below the target and stops there, with no marker left to redo the advance. The store then sits below the target that SC and Tendermint were rewound to, and the rollback reported success.
Syncing the version write (or flushing Pebble) before clearRollbackTarget closes the window and matches the durability discipline the rest of this sequence follows.
| return 0, fmt.Errorf("cannot roll back state store to version %d: nearest snapshot is %d and the changelog is empty", target, base) | ||
| } | ||
| if next == 0 { | ||
| return 0, fmt.Errorf("cannot roll back state store to version %d: nearest snapshot is %d and the changelog has no newer entries", target, base) |
There was a problem hiding this comment.
[suggestion] next == 0 (no changelog entry above base) is refused, but the rest of the machinery already supports this plan. The proof the next check relies on applies here too: retention prunes a prefix only, so a retained entry at or below base (oldest != 0 and oldest <= base) proves versions base+1..target were never written rather than pruned away. The rollback would then restore the snapshot, replay nothing, and let the marker path in NewCompositeStateStore advance the watermark to the target — which is precisely the empty-block case that code comments describe.
As written, planRollback refuses a target the sequence can reach, so ValidateRollback reports "cannot follow" and the operator gets the SS-stays-ahead warning instead of a rollback. Rare in practice (it needs a whole snapshot interval of blocks with no state writes), but it is an inconsistency between the two gap arguments in this function.
| if oldest == 0 { | ||
| return 0, fmt.Errorf("cannot roll back state store to version %d: nearest snapshot is %d and the changelog is empty", target, base) | ||
| } | ||
| if next == 0 { |
There was a problem hiding this comment.
rollbackBaseVersion allows a middle empty block, WALVersionsAfter returns next == 0 when nothing in the log is above base. Retention only truncates a prefix, so a retained entry at or below base (oldest != 0 && oldest <= base) already proves base+1..target were never written. The reopen path in NewCompositeStateStore is built for exactly that: restore the snapshot, replay nothing, then SetLatestVersion(pendingRollbackTarget) from the marker.
Rolling back to the snapshot height is allowed even with an empty changelog (base == target returns early). Rolling back above it by empty heights is not. That is an inconsistency in the same function.
There was a problem hiding this comment.
I do not think oldest <= base is enough evidence for a missing suffix.
It proves a gap between retained entries was not removed by prefix retention. It does not prove the WAL tail was never removed. This codebase has two suffix-removal paths: truncateChangelogAfterVersion deliberately truncates the tail during rollback, and opening the underlying WAL can repair a corrupt tail.
The existing test now named changelog tail cut below target demonstrates the ambiguity. The live store is at version 8 and versions 6–8 wrote data. Its WAL is cut at snapshot 5, which gives exactly oldest != 0, oldest <= base, and next == 0. If target 7 were accepted, rollback would restore snapshot 5, replay nothing, set only the watermark to 7, and silently lose the writes from 6 and 7.
base == target is not the same case: snapshot base is already the exact requested state, so no suffix evidence is required.
Supporting an all-empty suffix safely needs new durable evidence, such as WAL records for empty blocks or a separate verified coverage marker. The current WAL cannot distinguish it from a missing tail, so the conservative refusal remains. Commit 8b6e603e4 adds this rationale beside the check and makes the corruption case explicit in the test. The composite race suite and lint are clean.
There was a problem hiding this comment.
next == 0 also could be the WAL is truncated or corrupted tail.
we have a test where the store is at version 8, the snapshot is at 5, versions 6–8 wrote data, and the WAL tail is cut at 5. This produces oldest <= base and next == 0. accepting rollback to 7 would restore snapshot 5, replay nothing, and silently lose writes from versions 6 and 7.
base == target is safe because the snapshot is already the exact requested state. For target > base, we need durable evidence for the suffix. The current WAL does not provide it, so we keep the conservative refuse.
A retained prefix proves gaps between retained entries are empty blocks, but it cannot distinguish an empty suffix from a truncated or repaired WAL tail. Document the conservative refusal and the corruption case that requires it. Co-authored-by: Cursor <cursoragent@cursor.com>
2 participants
Summary
Make
seid rollbackrewind the state store with Tendermint and state-commit. The state store restores the newest retained snapshot at or below the target and replays its changelog to the exact target height. If the configured state store cannot follow, the command logs a warning and keeps the previous behavior of rolling back Tendermint and state-commit.sei-db/state_db/ss/composite: adds validation, snapshot selection, WAL replay, and a crash-resumable rollback sequence. Durable markers, directory synchronization, and restart recovery keep the snapshot, changelog, and version watermark consistent after a process crash or power loss. Rollback rejects unsupported EVM-split and non-Pebble state stores.sei-cosmos/storev2/rootmulti: validates state-store rollback before moving state-commit, then coordinates both stores through the existing rollback command.sei-db/db_engine/pebbledb/mvccandsei-db/state_db/ss/snapshot: anchor changelog retention to the oldest retained snapshot while preserving the count-based recovery floor. Empty-block version gaps remain replayable.sei-db/walandsei-db/common/utils: add version-aware WAL searches and durable Pebble directory cloning.sei-db/db_engine/types: defines optional rollback, rollback-validation, WAL-read, and WAL-pruning capabilities.Test plan
go build ./...go vet ./sei-db/state_db/ss/composite/... ./sei-db/db_engine/pebbledb/mvcc/... ./sei-cosmos/storev2/rootmulti/...golangci-lint run ./sei-db/state_db/ss/composite/... ./sei-db/db_engine/pebbledb/mvcc/... ./sei-cosmos/storev2/rootmulti/...go test -race -count=1 ./sei-db/state_db/ss/composite/... ./sei-db/db_engine/pebbledb/mvcc/... ./sei-cosmos/storev2/rootmulti/...