Backport release/v6.7: retain 72 FlatKV checkpoints instead of mirroring memIAVL's count
#4146
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,8 +9,24 @@ import ( | |
| ) | ||
|
|
||
| const ( | ||
| DefaultSnapshotInterval uint32 = 10000 | ||
|
|
||
| // DefaultSnapshotKeepRecent keeps 72 old checkpoints besides the latest one, | ||
| // which at DefaultSnapshotInterval is a guaranteed reach of 720,000 blocks — | ||
| // about 89 hours at mainnet's block rate, so it spans the EVM migration | ||
| // window at the rate that window is planned for. | ||
| // | ||
|
Comment on lines
+14
to
+18
There was a problem hiding this comment.
Rewrite this exported godoc to state only that the constant is the default number of retained checkpoints. The migration timing, disk-cost estimates, hardlink mechanism, and operational rationale turn it into a three-paragraph design note that will become stale with deployment conditions and violates the repository requirement that godocs describe what a subject is in one or two sentences rather than why or how it works. AGENTS.md reference: AGENTS.md:L59-L72 Useful? React with 👍 / 👎. |
||
| // It is this deep because a FlatKV checkpoint is nearly free. Checkpoints | ||
| // hardlink their SSTs, so one only costs the bytes compaction has since made | ||
| // obsolete: measured at mainnet state size, 261 MiB of pinned SSTs plus about | ||
| // 25 MiB of retained state WAL. 72 of them is roughly 20 GiB. The cost is | ||
| // linear in depth, because each older checkpoint pins exactly the files | ||
| // obsoleted during its own interval and those sets are disjoint. | ||
| // | ||
| // Reach matters because it bounds what can be answered about a past height at | ||
| // all. Below it, migrate-evm-status, dump-flatkv and a cross-backend digest | ||
| // cannot open a version, and a rollback has no base snapshot to rewind to. | ||
| DefaultSnapshotKeepRecent uint32 = 72 | ||
|
There was a problem hiding this comment. [suggestion] The ~20 GiB cost model is extrapolated from a steady-state measurement ("post- The WAL term is the bigger gap. Probably still affordable on the 2 TiB volume, but the number operators will plan against is off by roughly an order of magnitude in exactly the window the default exists for. Worth re-measuring mid-drain and correcting this comment and the CHANGELOG, or bounding WAL retention independently of snapshot count. |
||
| ) | ||
|
|
||
| // Config defines configuration for the FlatKV (EVM) commit store. | ||
|
|
@@ -40,7 +56,12 @@ type Config struct { | |
| // SnapshotKeepRecent defines how many old snapshots to keep besides the | ||
| // latest one. 0 means keep only the current snapshot (no old snapshots). | ||
| // Ignored entirely when ExternalPruning is set. | ||
| // | ||
| // It is not mirrored from memIAVL's sc-keep-recent, and the production store | ||
| // reads no app.toml key for it, so a node runs the in-code default. See | ||
| // composite.alignFlatKVSnapshotIntervalWithMemIAVL for why the two backends | ||
| // share an interval but not a retention count. | ||
| // Default: 72 | ||
| SnapshotKeepRecent uint32 `mapstructure:"snapshot-keep-recent"` | ||
|
|
||
| // ExternalPruning hands retention to the StorageGarbageCollector: the store stops pruning its | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[suggestion] After this change FlatKV's retention has no operator-visible lever at all:
parseSCConfigs(app/seidb.go:101) reads no FlatKV snapshot key, the template hidesflatkv.*, and thesc-keep-recentmirror is gone. A disk-constrained node cannot reduce the 73 retained checkpoints without a new binary. The doc's own argument for keeping the interval mirror unconditional — "otherwise FlatKVOnly would have no template-visible way to tune it" — now applies to the count with no answer.Relatedly, because retention is a fixed count while the interval is mirrored from
sc-snapshot-interval, both the reach and the disk cost scale with whatever the operator sets there.sc-snapshot-interval = 1000collapses the "guaranteed" 720,000-block reach to 72,000 blocks (~9 h, well under the 85-h drain) with no warning, and a larger interval multiplies the bytes each checkpoint pins.TestFlatKVDefaultRetentionSpansTheMigrationWindowonly pins this at the default interval, so that case is not covered.Expressing the target in blocks (or clamping the count against the effective interval so the reach invariant holds for any interval) would make the guarantee this default is named for actually hold, and give the count a defined behavior when the interval is tuned.