Repository navigation
fix(flatkv): keep 10 old checkpoints instead of mirroring memIAVL's count - #4322
Conversation
…ount A composite read needs a version both backends still hold. At mainnet state size memIAVL publishes a snapshot only about every 50,000 blocks, while one retained FlatKV checkpoint reaches back only 10,000 to 20,000 blocks. FlatKV therefore prunes each version before memIAVL publishes it, so no common version exists. This blocks a cross-backend digest and leaves a composite rollback with no shared base snapshot. FlatKV now keeps 10 old checkpoints, a guaranteed reach of 100,000 blocks at the default interval. alignFlatKVSnapshotWithMemIAVL still mirrors memIAVL's effective snapshot interval, but no longer overwrites FlatKV's keep-recent with memIAVL's sc-keep-recent. Without that second part the new default had no effect on a node, because the composite store reset it to 1. A checkpoint hardlinks its SSTs, so each costs only what compaction has obsoleted since: about 290 MiB at mainnet state size, roughly 2.8 GiB for ten. No app.toml change is needed. The node path (app/seidb.go parseSCConfigs) does not read state-commit.flatkv.snapshot-keep-recent, so a key left by an older template does not pin the old value. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
PR SummaryMedium Risk Overview FlatKV default Reviewed by Cursor Bugbot for commit d3c5d2e. Bugbot is set up for automated code reviews on this repo. Configure here. |
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
There was a problem hiding this comment.
This PR stops alignFlatKVSnapshotWithMemIAVL from copying memIAVL's keep-recent onto FlatKV and raises FlatKV's default from 1 to 10. FlatKV still takes its snapshot interval from memIAVL. The change is small and correct, and the tests that relied on the old copying were updated. The only note is that the fixed count of 10 no longer covers the gap if an operator lowers sc-snapshot-interval.
Findings: 0 blocking | 1 non-blocking | 0 posted inline
Blockers
- None at the file/PR level.
Non-blocking
- [suggestion] FlatKV's reach is now 10 × the snapshot interval, and the interval still comes from
sc-snapshot-interval. At mainnet size, memIAVL publishes a snapshot about every 50,000 blocks because writing one is slow, so that gap does not shrink when the interval does. An operator who lowerssc-snapshot-interval(e.g. to 1,000) gets 10,000 blocks of FlatKV reach, and the backends again share no version. Either document this interaction or derive keep-recent from a target block depth (e.g. ceil(100000/interval)) so the stated guarantee holds for any interval.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d193308172
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // than that: ten old checkpoints at this interval reach 100,000 blocks. A checkpoint | ||
| // hardlinks its SSTs, so each one costs only what compaction has obsoleted since, about | ||
| // 290 MiB at mainnet state size. | ||
| SnapshotKeepRecent: 10, |
There was a problem hiding this comment.
Pin the 10-checkpoint default in characterization
Add a literal characterization assertion for the new value. The added composite test obtains both the actual and expected values from DefaultStateCommitConfig() and merely checks that FlatKV differs from memIAVL, so changing this default from 10 to another non-1 value would remain green and the configuration change is not recorded as an old-to-new diff as required.
AGENTS.md reference: AGENTS.md:L22-L30
Useful? React with 👍 / 👎.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4322 +/- ##
==========================================
- Coverage 67.60% 66.51% -1.10%
==========================================
Files 2192 2066 -126
Lines 168752 156304 -12448
==========================================
- Hits 114087 103966 -10121
+ Misses 54655 52328 -2327
Partials 10 10
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Co-authored-by: Cursor <cursoragent@cursor.com> # Conflicts: # CHANGELOG.md
|
Created backport PR for
Please cherry-pick the changes locally and resolve any conflicts. git fetch origin backport-4322-to-release/v6.7
git worktree add --checkout .worktree/backport-4322-to-release/v6.7 backport-4322-to-release/v6.7
cd .worktree/backport-4322-to-release/v6.7
git reset --hard HEAD^
git cherry-pick -x 8c95ff34abd8634c1bfbabaddc6fbe85823a2c02
git push --force-with-lease |
…ount (#4322) A composite read needs a version that both backends still hold. At mainnet state size, memIAVL publishes a snapshot only about every 50,000 blocks. One retained FlatKV checkpoint reaches back only 10,000 to 20,000 blocks, so FlatKV prunes each version before memIAVL publishes it, and no common version exists. On the mainnet shadow pair, memIAVL published 229810000 when FlatKV held only 229840000 and 229850000. This blocks a cross-backend digest and leaves a composite rollback with no shared base snapshot. - FlatKV's default `SnapshotKeepRecent` goes from 1 to 10. At the default 10,000-block interval, this is a guaranteed reach of 100,000 blocks (about 12 hours). - `alignFlatKVSnapshotWithMemIAVL` still copies memIAVL's effective snapshot **interval** to FlatKV, but it no longer overwrites FlatKV's **keep-recent** with `sc-keep-recent`. Without this, the new default has no effect on a node (the review finding on #4158). - New test `TestNewCompositeCommitStoreKeepsFlatKVKeepRecent` builds a composite store from the default config and checks the effective FlatKV retention. `TestAlignFlatKVSnapshotWithMemIAVL` now checks that keep-recent is left as configured. - Three test configs that set only memIAVL keep-recent (and relied on the mirror) now also set FlatKV keep-recent. A checkpoint hardlinks its SSTs, so each one costs only what compaction has obsoleted since: about 261 MiB of pinned SSTs plus about 25 MiB of retained state WAL at mainnet state size, roughly 2.8 GiB for ten. No `app.toml` change is needed. The node path (`parseSCConfigs` in `app/seidb.go`) does not read `state-commit.flatkv.snapshot-keep-recent`, so a key rendered by the v6.4–v6.6 templates does not pin the old value. Every node gets the new default on restart. Replaces #4158, which targeted `release/v6.7` directly. Labelled `backport release/v6.7`. The FlatKV `config.go` differs between `main` and `release/v6.7` (v6.7 still has the `DefaultSnapshotKeepRecent` constant and the config golden records), so the automatic backport can conflict there. - `scripts/ramtest.sh` on `sei-db/state_db/sc/composite`, `sei-db/state_db/sc/flatkv/...`, `sei-db/config`, `sei-cosmos/storev2/rootmulti`, `sei-cosmos/server/config`, and the `app` config tests. - `make fmtcheck`. --------- Co-authored-by: Cursor <cursoragent@cursor.com> (cherry picked from commit 8c95ff3) Co-authored-by: Cursor <cursoragent@cursor.com>
…ount (#4322) A composite read needs a version that both backends still hold. At mainnet state size, memIAVL publishes a snapshot only about every 50,000 blocks. One retained FlatKV checkpoint reaches back only 10,000 to 20,000 blocks, so FlatKV prunes each version before memIAVL publishes it, and no common version exists. On the mainnet shadow pair, memIAVL published 229810000 when FlatKV held only 229840000 and 229850000. This blocks a cross-backend digest and leaves a composite rollback with no shared base snapshot. - FlatKV's default `SnapshotKeepRecent` goes from 1 to 10. At the default 10,000-block interval, this is a guaranteed reach of 100,000 blocks (about 12 hours). - `alignFlatKVSnapshotWithMemIAVL` still copies memIAVL's effective snapshot **interval** to FlatKV, but it no longer overwrites FlatKV's **keep-recent** with `sc-keep-recent`. Without this, the new default has no effect on a node (the review finding on #4158). - New test `TestNewCompositeCommitStoreKeepsFlatKVKeepRecent` builds a composite store from the default config and checks the effective FlatKV retention. `TestAlignFlatKVSnapshotWithMemIAVL` now checks that keep-recent is left as configured. - Three test configs that set only memIAVL keep-recent (and relied on the mirror) now also set FlatKV keep-recent. A checkpoint hardlinks its SSTs, so each one costs only what compaction has obsoleted since: about 261 MiB of pinned SSTs plus about 25 MiB of retained state WAL at mainnet state size, roughly 2.8 GiB for ten. No `app.toml` change is needed. The node path (`parseSCConfigs` in `app/seidb.go`) does not read `state-commit.flatkv.snapshot-keep-recent`, so a key rendered by the v6.4–v6.6 templates does not pin the old value. Every node gets the new default on restart. Replaces #4158, which targeted `release/v6.7` directly. Labelled `backport release/v6.7`. The FlatKV `config.go` differs between `main` and `release/v6.7` (v6.7 still has the `DefaultSnapshotKeepRecent` constant and the config golden records), so the automatic backport can conflict there. - `scripts/ramtest.sh` on `sei-db/state_db/sc/composite`, `sei-db/state_db/sc/flatkv/...`, `sei-db/config`, `sei-cosmos/storev2/rootmulti`, `sei-cosmos/server/config`, and the `app` config tests. - `make fmtcheck`. --------- Co-authored-by: Cursor <cursoragent@cursor.com> (cherry picked from commit 8c95ff3) Co-authored-by: Cursor <cursoragent@cursor.com>
…ount (#4322) A composite read needs a version that both backends still hold. At mainnet state size, memIAVL publishes a snapshot only about every 50,000 blocks. One retained FlatKV checkpoint reaches back only 10,000 to 20,000 blocks, so FlatKV prunes each version before memIAVL publishes it, and no common version exists. On the mainnet shadow pair, memIAVL published 229810000 when FlatKV held only 229840000 and 229850000. This blocks a cross-backend digest and leaves a composite rollback with no shared base snapshot. - FlatKV's default `SnapshotKeepRecent` goes from 1 to 10. At the default 10,000-block interval, this is a guaranteed reach of 100,000 blocks (about 12 hours). - `alignFlatKVSnapshotWithMemIAVL` still copies memIAVL's effective snapshot **interval** to FlatKV, but it no longer overwrites FlatKV's **keep-recent** with `sc-keep-recent`. Without this, the new default has no effect on a node (the review finding on #4158). - New test `TestNewCompositeCommitStoreKeepsFlatKVKeepRecent` builds a composite store from the default config and checks the effective FlatKV retention. `TestAlignFlatKVSnapshotWithMemIAVL` now checks that keep-recent is left as configured. - Three test configs that set only memIAVL keep-recent (and relied on the mirror) now also set FlatKV keep-recent. A checkpoint hardlinks its SSTs, so each one costs only what compaction has obsoleted since: about 261 MiB of pinned SSTs plus about 25 MiB of retained state WAL at mainnet state size, roughly 2.8 GiB for ten. No `app.toml` change is needed. The node path (`parseSCConfigs` in `app/seidb.go`) does not read `state-commit.flatkv.snapshot-keep-recent`, so a key rendered by the v6.4–v6.6 templates does not pin the old value. Every node gets the new default on restart. Replaces #4158, which targeted `release/v6.7` directly. Labelled `backport release/v6.7`. The FlatKV `config.go` differs between `main` and `release/v6.7` (v6.7 still has the `DefaultSnapshotKeepRecent` constant and the config golden records), so the automatic backport can conflict there. - `scripts/ramtest.sh` on `sei-db/state_db/sc/composite`, `sei-db/state_db/sc/flatkv/...`, `sei-db/config`, `sei-cosmos/storev2/rootmulti`, `sei-cosmos/server/config`, and the `app` config tests. - `make fmtcheck`. --------- Co-authored-by: Cursor <cursoragent@cursor.com> (cherry picked from commit 8c95ff3) Co-authored-by: Cursor <cursoragent@cursor.com>
* main: (21 commits) Backport evmonly parse, app-hash and changeset perf fixes from giga-1 (#4345) Remove the oracle module behind a v6.8 upgrade (#4319) fix(seidb): report only the current migration boundary on the snapshot gauge (#4327) fix(flatkv): keep 10 old checkpoints instead of mirroring memIAVL's count (#4322) Backport Autobahn execute-loop and produced-tx metrics from giga-1 (#4330) Add giga.storage.receipts to toggle the Autobahn receipt store (#4333) optimize gather phase (#4326) Regenerate the Unreleased changelog as a plain PR list (#4336) Bump sei-protocol/go-ethereum to v1.15.7-sei-21 (#4332) Fail dynamic-gas precompile out-of-gas as an EVM out-of-gas call (#4318) Add dashboard and topology option for Autobahn e2e (#4167) Add Giga fetch/serve and BlockDB prune metrics (#4329) Add eth_getLogs to the EVM-only Giga RPC (#4308) Generate v6.8 precompiles (#4320) Add [giga] app.toml section and honor it on the Autobahn node (#4323) feat(evmonly): add eth_estimateGas via existing libraries (#4325) Use Pebble batch directly in SS (#4300) Fix pruning issue in SS causing huge disk spike (#4321) Make Autobahn always run the EVM-only executor, disable/remove some integration tests (#4316) reduce seal lock contention (#4314) ...
Summary
A composite read needs a version that both backends still hold. At mainnet state size, memIAVL publishes a snapshot only about every 50,000 blocks. One retained FlatKV checkpoint reaches back only 10,000 to 20,000 blocks, so FlatKV prunes each version before memIAVL publishes it, and no common version exists. On the mainnet shadow pair, memIAVL published 229810000 when FlatKV held only 229840000 and 229850000. This blocks a cross-backend digest and leaves a composite rollback with no shared base snapshot.
Changes
SnapshotKeepRecentgoes from 1 to 10. At the default 10,000-block interval, this is a guaranteed reach of 100,000 blocks (about 12 hours).alignFlatKVSnapshotWithMemIAVLstill copies memIAVL's effective snapshot interval to FlatKV, but it no longer overwrites FlatKV's keep-recent withsc-keep-recent. Without this, the new default has no effect on a node (the review finding on fix(flatkv): keep 10 old checkpoints instead of mirroring memIAVL's count #4158).TestNewCompositeCommitStoreKeepsFlatKVKeepRecentbuilds a composite store from the default config and checks the effective FlatKV retention.TestAlignFlatKVSnapshotWithMemIAVLnow checks that keep-recent is left as configured.Cost
A checkpoint hardlinks its SSTs, so each one costs only what compaction has obsoleted since: about 261 MiB of pinned SSTs plus about 25 MiB of retained state WAL at mainnet state size, roughly 2.8 GiB for ten.
Config
No
app.tomlchange is needed. The node path (parseSCConfigsinapp/seidb.go) does not readstate-commit.flatkv.snapshot-keep-recent, so a key rendered by the v6.4–v6.6 templates does not pin the old value. Every node gets the new default on restart.Backport
Replaces #4158, which targeted
release/v6.7directly. Labelledbackport release/v6.7. The FlatKVconfig.godiffers betweenmainandrelease/v6.7(v6.7 still has theDefaultSnapshotKeepRecentconstant and the config golden records), so the automatic backport can conflict there.Testing
scripts/ramtest.shonsei-db/state_db/sc/composite,sei-db/state_db/sc/flatkv/...,sei-db/config,sei-cosmos/storev2/rootmulti,sei-cosmos/server/config, and theappconfig tests.make fmtcheck.