ci, db/state: matrix-test serial vs parallel commitment across the test workflows#22141
ci, db/state: matrix-test serial vs parallel commitment across the test workflows#22141mh0lt wants to merge 12 commits into
Conversation
…st workflows Parallel execution (dbg.Exec3Parallel) is now the default, so the serial/parallel exec-mode CI matrix from #21017 no longer needs to toggle it — both legs run parallel exec. This repurposes that matrix axis to instead exercise serial vs parallel commitment, so divergence between statecfg.ExperimentalParallelCommitment true and false is caught on the PR rather than after release. The toggle is plumbed via ERIGON_COMMITMENT_PARALLEL: state_schema.go now reads COMMITMENT_PARALLEL via dbg.EnvBool (envLookup auto-prepends ERIGON_), mirroring EXEC3_PARALLEL. The existing CLI flag still composes — backend.go only flips the flag on, and the integration command's BoolVar now defaults to the env-derived value instead of clobbering it to false at registration. Across the 11 test workflows the exec_mode axis is renamed commitment_mode, the ERIGON_EXEC3_PARALLEL assignment is dropped, and ERIGON_COMMITMENT_PARALLEL is set from the axis (or, for the two workflow_dispatch perf tests, from the renamed input). Docker ENV injection for the hive/kurtosis/txpool images is updated to the new var. Per-mode Go build-cache lineages and artifact/enclave/testbed names carry over unchanged in shape.
mh0lt
requested review from
AskAlexSharov,
lystopad,
mriccobene,
sudeepdino008 and
yperbasis
as code owners
|
@mh0lt need to also update test-eest-spec.yml shards |
Addresses review: the EEST spec shard matrix also toggled execution mode via a -sequential/-parallel suffix (manifest exec3-parallel → ERIGON_EXEC3_PARALLEL). Repurpose it the same way as the other workflows: the suffix now selects serial vs parallel commitment, execution is parallel in every shard (default). tools/eest-spec-shards.yml: rename the per-entry field exec3-parallel → commitment-parallel. tools/run-eest-spec-test.sh: read commitment-parallel and export ERIGON_COMMITMENT_PARALLEL instead of ERIGON_EXEC3_PARALLEL. Shard names (and therefore the ci-gate check names and Makefile targets) are unchanged.
|
Good catch — done in f86ceab. The EEST spec shards toggled execution via the
Shard names are unchanged, so the ci-gate check names and |
There was a problem hiding this comment.
Pull request overview
This PR repurposes the existing CI “serial vs parallel” matrix axis from execution mode (now parallel by default) to state commitment mode, so CI exercises both serial and parallel commitment paths via a new environment toggle.
Changes:
- Add
ERIGON_COMMITMENT_PARALLELenv plumbing by initializingstatecfg.ExperimentalParallelCommitmentfromdbg.EnvBool("COMMITMENT_PARALLEL", false). - Preserve env-derived commitment defaults through CLI flag registration (notably in the
integrationcommand). - Update CI workflows and EEST tooling to drive serial vs parallel commitment via the new matrix axis and env var.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
db/state/statecfg/state_schema.go |
Initializes ExperimentalParallelCommitment from env (COMMITMENT_PARALLEL via dbg.EnvBool). |
cmd/integration/commands/flags.go |
Uses env-derived defaults when registering experimental commitment flags so env isn’t clobbered. |
tools/run-eest-spec-test.sh |
Switches EEST shard runner to export ERIGON_COMMITMENT_PARALLEL from the shard manifest. |
tools/eest-spec-shards.yml |
Renames shard-manifest key to commitment-parallel to control commitment mode per shard. |
.github/workflows/test-all-erigon.yml |
Renames matrix axis to commitment_mode and drives ERIGON_COMMITMENT_PARALLEL for Go tests. |
.github/workflows/test-all-erigon-race.yml |
Same as above for race-detector test groups with mode-separated build cache keys. |
.github/workflows/test-bench.yml |
Runs benchmarks in both commitment modes via ERIGON_COMMITMENT_PARALLEL. |
.github/workflows/test-hive.yml |
Updates Hive matrix to commitment_mode and injects ERIGON_COMMITMENT_PARALLEL into the Erigon client Dockerfile. |
.github/workflows/test-hive-eest.yml |
Same commitment-mode matrix + Docker ENV injection for Hive EEST runs. |
.github/workflows/test-kurtosis-assertoor.yml |
Updates Kurtosis matrix axis and bakes ERIGON_COMMITMENT_PARALLEL into the runtime image per leg. |
.github/workflows/qa-stage-exec.yml |
Switches QA stage-exec matrix to commitment mode and disambiguates artifacts/datadirs accordingly. |
.github/workflows/qa-txpool-performance-test.yml |
Switches txpool perf matrix to commitment mode and bakes the env var into the runtime image. |
.github/workflows/qa-rpc-integration-tests-latest.yml |
Switches RPC integration tests to commitment-mode matrix and artifacts naming. |
.github/workflows/qa-rpc-performance-tests.yml |
Renames workflow_dispatch input and runtime env handling to ERIGON_COMMITMENT_PARALLEL. |
.github/workflows/qa-rpc-performance-comparison-tests.yml |
Same input/env handling update for the comparison workflow. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| workers: 12 | ||
| max-allowed-failures: 0 | ||
| exec3-parallel: true | ||
| commitment-parallel: true |
| workers: 12 | ||
| max-allowed-failures: 0 | ||
| exec3-parallel: true | ||
| commitment-parallel: true |
|
@awskii fyi to check the |
blocktests-devnet and blocktests-devnet-race-amsterdam carried commitment-parallel: true as a mechanical rename of the old exec3-parallel: true (which forced parallel execution — now the default). Those shards have no -parallel suffix, so per the manifest's suffix-selects-mode rule they must run the default serial commitment; the override forced parallel commitment with no serial counterpart. Drop it so commitment-parallel is set iff the shard name ends in -parallel.
|
Addressed the two shard-config comments in |
…-matrix # Conflicts: # .github/workflows/test-hive.yml
With COMMITMENT_PARALLEL set globally, every SharedDomains selected the parallel trie, but only DB-backed consumers (exec, builder, squeeze, backtester) wire the per-worker TrieContextFactory it needs — integrity checks, RPC-created domains, and test harnesses failed with 'ParallelPatriciaHashed.Process requires a TrieContextFactory'. The context now starts on the sequential trie and upgrades to the selected parallel/streaming variant when EnableParaTrieDB provides the DB. SeekCommitment may restore state before the DB is wired, so the upgrade adopts the already-restored trie as the parallel template instead of re-encoding (SetState re-reads a sole-account root through the not-yet-installed context). Touching before the upgrade panics: keys collected on the sequential buffer would be dropped.
|
The remaining red legs here (eest blocktests/enginextests parallel, race-tests core-rpc/execution-other parallel) all fail on one class: after an unwind or on a fresh tiny chain, the restored commitment state is a sole-account root with no root branch record, and #22257 fixes this. Verified locally on this branch with it trial-merged: both repro tests pass, and the full So the order is: land #22257, then merge main into this branch again — the parallel legs should go green with no further changes here. |
Summary
Parallel execution (
dbg.Exec3Parallel) is now the default, so the serial/parallel exec-mode CI matrix added in #21017 no longer needs to toggle it — both legs already run parallel exec. This PR repurposes that matrix axis to exercise serial vs parallel commitment instead, so divergence betweenstatecfg.ExperimentalParallelCommitmenttrueandfalseis caught on the PR rather than after a release.Modeled directly on #21017 (the exec-mode matrix PR).
Mechanism
ERIGON_COMMITMENT_PARALLEL.db/state/statecfg/state_schema.gonow initializesExperimentalParallelCommitmentfromdbg.EnvBool("COMMITMENT_PARALLEL", false);envLookupincommon/dbg/dbg_env.goauto-prepends theERIGON_prefix, exactly likeEXEC3_PARALLEL.node/eth/backend.go), so the env value survives when the flag isn't passed;integrationcommand'sBoolVarnow defaults to the env-derived value instead of clobbering it back tofalseat flag registration.exec_modematrix axis is renamedcommitment_mode, theERIGON_EXEC3_PARALLELassignment is dropped (exec is parallel in both legs by default), andERIGON_COMMITMENT_PARALLELis driven from the axis. For the twoworkflow_dispatchperf tests the manualexec_modeinput becomescommitment_mode. DockerENVinjection for the hive/kurtosis/txpool runtime images is updated to the new var. Per-mode Go build-cache lineages and artifact/enclave/testbed names carry over unchanged in shape.Why
ExperimentalParallelCommitmentwas previously reachable only via a CLI flag, so no CI job exercised the parallel-commitment trie path. Reusing the (now redundant) exec-mode axis gives serial-vs-parallel commitment coverage at no extra matrix cost — same runner count, the axis just tests a different dimension.Note on coverage trade-off
This drops the dedicated serial-execution CI leg. That is deliberate: parallel execution is the default and is what ships; the serial-exec matrix leg was guarding a non-default path, whereas serial-vs-parallel commitment is the currently-diverging surface worth gating. Serial execution remains runnable locally via
ERIGON_EXEC3_PARALLEL=false.Workflows touched:
test-all-erigon,test-all-erigon-race,test-bench,test-hive,test-hive-eest,test-kurtosis-assertoor,qa-stage-exec,qa-txpool-performance-test,qa-rpc-integration-tests-latest,qa-rpc-performance-tests,qa-rpc-performance-comparison-tests.