db/state: use --experimental.concurrent-commitment flag for rebuild#20481
db/state: use --experimental.concurrent-commitment flag for rebuild#20481
Conversation
…nstead of env var Replace ERIGON_REBUILD_CONCURRENT_COMMITMENT env var with the existing statecfg.ExperimentalConcurrentCommitment global (set by the CLI flag) so both rebuild paths use a single source of truth.
There was a problem hiding this comment.
Pull request overview
This PR standardizes commitment rebuild trie-variant selection by removing a dedicated rebuild-only env var and instead using the existing --experimental.concurrent-commitment flag (via statecfg.ExperimentalConcurrentCommitment) as the single source of truth.
Changes:
- Switch
RebuildCommitmentFiles()to usestatecfg.ExperimentalConcurrentCommitmentfor both trie variant selection and ParaTrieDB enablement. - Update the concurrent rebuild integration test to toggle the global flag directly (and restore it via
t.Cleanup).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
db/state/squeeze.go |
Replaces env-var driven concurrent rebuild toggle with the global statecfg.ExperimentalConcurrentCommitment. |
db/state/squeeze_concurrent_rebuild_test.go |
Adjusts the integration test to set/restore the global flag instead of using t.Setenv. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Snapshot statecfg.ExperimentalConcurrentCommitment into a local at the top of RebuildCommitmentFiles to avoid re-reading the global mid-call. Set the flag to false immediately after save/restore in the test so Phase 1 data generation is deterministic.
Covers the squeeze=false path for both sequential and concurrent commitment rebuild, verifying root hash agreement without the squeeze/recompress pass.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // TestConcurrentRebuildCommitmentNoSqueeze verifies that rebuilding commitment files | ||
| // with squeeze=false produces the same root for both sequential and concurrent modes. | ||
| func TestConcurrentRebuildCommitmentNoSqueeze(t *testing.T) { |
There was a problem hiding this comment.
The new test name/comment imply that passing squeeze=false results in a rebuild that does not run the squeeze pass, but RebuildCommitmentFiles only skips squeezing when both squeeze==false and statecfg.Schema.CommitmentDomain.ReplaceKeysInValues==false. Since ReplaceKeysInValues defaults to true, this test will still execute the squeeze path even with squeeze=false. Consider renaming the test to reflect that it validates the "squeeze flag set to false" behavior, or explicitly toggling Schema.CommitmentDomain.ReplaceKeysInValues (with cleanup restore) if the intent is to cover the true no-squeeze branch.
| // TestConcurrentRebuildCommitmentNoSqueeze verifies that rebuilding commitment files | |
| // with squeeze=false produces the same root for both sequential and concurrent modes. | |
| func TestConcurrentRebuildCommitmentNoSqueeze(t *testing.T) { | |
| // TestConcurrentRebuildCommitmentSqueezeFlagFalse verifies that rebuilding commitment files | |
| // with the squeeze flag set to false produces the same root for both sequential and concurrent modes. | |
| func TestConcurrentRebuildCommitmentSqueezeFlagFalse(t *testing.T) { |
The per-buffer flush messages from the concurrent branch collector spam INFO during rebuild. Lower to LvlDebug.
ERIGON_REBUILD_CONCURRENT_COMMITMENTenv var with the existingstatecfg.ExperimentalConcurrentCommitmentglobal inRebuildCommitmentFiles()--experimental.concurrent-commitmentCLI flag as the single source of truth for trie variant selectiont.Setenv