Repository navigation
Backport release/v6.7: Guard FlatKV rollback floor - #4502
Conversation
|
Please cherry-pick the changes locally and resolve any conflicts. git fetch origin backport-4496-to-release/v6.7
git worktree add --checkout .worktree/backport-4496-to-release/v6.7 backport-4496-to-release/v6.7
cd .worktree/backport-4496-to-release/v6.7
git reset --hard HEAD^
git cherry-pick -x 8b5e550e9c5de26a0e0907f9bcc422e446cdc3a3
git push --force-with-lease |
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## release/v6.7 #4502 +/- ##
================================================
- Coverage 61.85% 61.09% -0.77%
================================================
Files 2168 2089 -79
Lines 188699 180212 -8487
================================================
- Hits 116721 110095 -6626
+ Misses 61128 59912 -1216
+ Partials 10850 10205 -645
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Guard FlatKV's retained-history floor before composite rollback mutates any backend, and treat stale idle FlatKV directories as disposable Auto-mode seeds instead of reconcilable state. - `sei-db/state_db/sc/flatkv/snapshot.go`: adds a single read-only reachability check for snapshots plus WAL. Rollback still uses the same check before any mutation. Only "no snapshot at or below the target" is tagged `ErrVersionUnreachable`; filesystem errors from the snapshot scan pass through untagged. - `sei-db/state_db/sc/flatkv/store.go`: rejects unreachable historical read-only loads before creating a `readonly-*` working directory. - `sei-db/state_db/giga/types/live_state_store.go`: exposes `CheckVersionReachable` so composite rollback can preflight the FlatKV backend, and owns its error contract, `ErrVersionUnreachable` (the only non-fatal error of the interface). FlatKV keeps `flatkv.ErrVersionUnreachable` as an alias. - `sei-db/state_db/sc/composite/store.go`: refuses rollback targets FlatKV cannot reach before memIAVL moves, and points operators to state sync. Before seeding or reconciliation, it removes an idle Auto-mode FlatKV dir whose seed is above memIAVL or more than one block below it. An idle seed exactly one block behind is kept for `reconcileVersions`, because that is what a crash between the memIAVL and FlatKV commits of the kickoff block leaves. - `sei-db/state_db/sc/composite/store_test.go`: updates the test fake for the new interface. - `sei-db/state_db/sc/flatkv/store_test.go`: updates the beyond-WAL read-only test for the new early reachability failure. - `sei-db/state_db/sc/composite/rollback_floor_test.go`: covers rollback to K-2 refusal without mutation, historical read/export below the floor returning errors without panic, rollback to K-1 followed by re-kickoff, delayed kickoff after a stale idle FlatKV dir, a torn kickoff commit (memIAVL at K, FlatKV seed at K-1) that must roll memIAVL back and replay K to the canonical AppHash, and state sync from a pre-kickoff snapshot with replay through migration. - `sei-db/state_db/sc/flatkv/snapshot_test.go`: an empty snapshot dir is `ErrVersionUnreachable`; a snapshot-scan read failure is not. - `sei-db/state_db/sc/flatkv/store_replay_test.go`: covers snapshot-only reachability, snapshot-plus-WAL reachability, below-history errors, and failing read-only loads before creating a read-only work dir. - `go test ./sei-db/state_db/sc/flatkv ./sei-db/state_db/sc/composite` - `make fmtcheck` - `make dblint` - `go test -race ./sei-db/state_db/sc/composite/... ./sei-db/state_db/sc/flatkv/... ./sei-db/state_db/giga/... ./sei-db/bootstrap/...` - `scripts/ramtest.sh ./sei-db/...` failed once in unrelated `TestRapidFlushes` with "Expected at most 25 flushes, got 32"; rerunning `scripts/ramtest.sh ./sei-db/db_engine/litt/disktable` passed. --------- Co-authored-by: Cursor <cursoragent@cursor.com> (cherry picked from commit 8b5e550)
790c84d to
be53485
Compare
PR SummaryMedium Risk Overview Composite checks reachability before FlatKV renames rollback reachability logic to Reviewed by Cursor Bugbot for commit 7f7531d. Bugbot is set up for automated code reviews on this repo. Configure here. |
|
@seidroid review |
![seidroid[bot]](https://avatars.githubusercontent.com/in/2745135?s=60&v=4){kind=small}
There was a problem hiding this comment.
This backport adds a FlatKV reachability check (CheckVersionReachable, which wraps ErrVersionUnreachable) so that composite rollback and historical reads below FlatKV's retained floor are refused before any backend changes, and it discards stale idle Auto-mode FlatKV seeds at load while keeping the one-block-behind torn-kickoff case. The rollback, discard and torn-commit paths hold up against the merge tree (5e76a49); I found nothing blocking (Go is not installed here, so I did not run the tests), only one non-blocking point about locking around the WAL check; the codex reading reported nothing and so added nothing to check or carry.
Non-blocking
1 finding on the changed lines, as inline comments.
seidroid review · decision approve · session 45db7019caf24c5494ade246161663bc · turn resp_claude_c4163f2d932014a03ab24604163d7b10 · item 14cd94dfe3dc52e09e256e4ec2ee7f0b
Findings: 0 blocking | 1 non-blocking | 1 posted inline
| return nil, errReadOnly | ||
| } | ||
| if targetVersion != 0 { | ||
| if err := s.CheckVersionReachable(targetVersion); err != nil { |
There was a problem hiding this comment.
suggestion — CheckVersionReachable now runs on the live store without holding s.mu, and it calls s.wal.GetStoredRange(). The StateWAL contract says calls must be serialized, which is why openReplayIterator takes s.mu against a concurrent Commit. Here the unlocked read of closed/fatalErr, and the fail() write if Bounds errors, can race with Commit during a live export or historical query. Taking s.mu around the check, or doing the check inside the locked replay path, keeps the contract.
There was a problem hiding this comment.
This PR backports #4496 to release/v6.7. FlatKV now refuses a rollback, read-only load or export at a version it can't rebuild from its retained snapshots and WAL, and does so before anything changes on disk; the error wraps ErrVersionUnreachable and tells the operator to use state sync. In Auto mode, a stale FlatKV directory that holds only its seed is now deleted, while a torn kickoff commit (FlatKV exactly one block behind) is kept and replayed. Tests cover these paths, and the one other flatkv.Store implementation (a test double) gets the new method, so I found nothing to raise.
Findings: 0 blocking | 0 non-blocking | 0 posted inline
Blockers
- None at the file/PR level.
Non-blocking
- None at the file/PR level.
2 participants
Backport of #4496 to
release/v6.7.