Repository navigation
ci: run consensus_sim test cases in parallel - #203
Conversation
|
Reviewed the driver against the harness — the parallelism premise holds:
Two things I'd want before merging: 1. A count cross-check. The whole run now rests on parsing 2. Rebase after #202. Not blocking the approach: the ~15 min serial binary on one core while 3 cores idle is real waste, and the driver is the right shape. |
consensus_sim is a single Boost.Test binary. In CI it ran serially on one core of a 4-core runner: ~15.5 min in "consensus_sim + pm unit tests" and ~19 min in "Testnet config (1 validator)", the longest step in both jobs. tests/consensus_sim/run_parallel.py lists the enabled cases with --list_content and runs each one as its own process, -j N at a time. Cases are already independent: every simulated node opens its own unique temp directory, and failure logs are named by seed and scenario. It prints each failing case's full output, a slowest-cases table, and exits non-zero if any case failed. --filter keeps the 2-validator arm to the same three suites it ran before. Both jobs now call it from build/tests/consensus_sim, the directory ctest used, so failures/ logs and the upload-artifact path are unchanged. The other ctest suites still run through ctest.
seed_sweep_one_hundred_all_fire ran 100 seeds in one test case, ~9 s each under ASan: 822-927 s of the ~930 s wall time once the other cases run in parallel. Make it a BOOST_DATA_TEST_CASE over 10 chunks of 10 contiguous seeds (_0 = seeds 0-9 ... _9 = seeds 90-99). Same seeds, same assertions, same failure logs; a static_assert keeps the chunks covering every seed exactly once. --run_test=.../seed_sweep_one_hundred_all_fire still runs all of them.
Review feedback on #203: the run rested on parsing the indented --list_content output, so a format change that made the parser return a subset would go green having run fewer cases. - Build the case list from the DOT listing, where test cases are identified by id (TUT_CASE bit), not layout, and require it to match the indented listing exactly; abort on any difference. - --min-cases N fails the run if the binary has fewer enabled cases than the floor (checked before --filter). CI passes 117 / 172. - Print "skipped (disabled): N" and list them. - --filter naming a suite with no enabled cases is an error. - A --run_test that matches nothing ("Test setup error", exit 0 on some Boost versions) counts as a failure.
tests.yml and docker-pr-build.yml take ~30 min; a new push to the same PR made the old run's result irrelevant but it still held runners to the end. Cancel in-progress runs per PR. Master pushes use the run id as the group, so they are never cancelled and every merged commit is tested.
a5e934f to
ea188e0
Compare
|
Thanks, both done. I rebased on master (now includes #202's
I moved the PR to draft until this CI run is green. Details and local checks are in the description. |
On1x
left a comment
There was a problem hiding this comment.
Re-reviewed on the new head ea188e08 (rebased on 022dd398). Both blockers from my earlier comment are resolved, and I checked the claims against the run rather than the description:
Case-count cross-check. run_parallel: 117 cases, 4 at a time; skipped (disabled): 0 in the log, and --min-cases 117 passes — the floor and the measured count agree, so the number is not a copy that can rot. The DOT listing is built from the TUT_CASE bit and its path from the edge list, which is layout-independent, and a disagreement with the indented listing aborts instead of running a subset. --run_test matching nothing is now a failure rather than a Boost-version-dependent pass — that closes the "green over an empty set" hole.
Parallelism safety, verified beyond the two properties I checked earlier:
- per-node data dirs stay unique (
bfs::unique_pathinsimulated_node.cpp), failure logs are<seed>-<scenario>.logand the sweep split keeps every seed in exactly one chunk (static_assert, seeds 0..99 partitioned into 10×10), so no two concurrent cases can write the same artifact; - no case binds a socket — the endpoint/p2p cases are string predicates over host:port, not listeners, so
-j4cannot collide on a port; - ctest still owns the other suites (
-E '^consensus_sim$'), and the driver runs frombuild/tests/consensus_sim, the directory ctest used.
Measured effect. wall 250s, sum of cases 920s (3.7× on 4 cores) vs 1070 s for the serial consensus_sim on my #205 run — about 14 minutes off the longest step. The 2-validator arm: 11 cases after --filter, --min-cases 172 checked before the filter.
Concurrency block. Correct on both paths: pull_request groups by PR number and cancels the superseded run; push groups by run_id so master runs are never cancelled. tests.yml does not listen to labeled, so a label that re-triggers the docker workflow cannot cancel the test run.
One follow-up, pre-existing and not a blocker: the artifact step uploads tests/consensus_sim/failures/ — that path is relative to the workspace root, i.e. the SOURCE tree, where it holds only .gitkeep; the logs are written under build/tests/consensus_sim/failures/. With if-no-files-found: ignore the artifact is empty and silent. Since this PR is what makes the directory choice explicit, it is the natural place for the one-line fix (path: build/tests/consensus_sim/failures/); otherwise it is worth an issue of its own — a failure artifact nobody can open turns "logs uploaded" into a claim.
Approved from my side. Merge is the owner's, per the repo policy for public VIZ repositories.
|
CI is green on the rebased branch (with #202), so this is ready again.
|
Review on #203: the harness writes failures/<seed>-<scenario>.log under its working directory, bfs::current_path() / "failures" in harness/failure_log.cpp, and the job runs it from build/tests/consensus_sim. The upload step pointed at tests/consensus_sim/failures/ in the source tree, which only holds .gitkeep; with if-no-files-found: ignore the artifact was silently empty. Pre-existing, but this PR is what pins the working directory.
|
Good catch, thanks. Fixed in I checked the path in the harness: |
#205 added db_invariants_suite/healthy_chain_reconciles_and_divergence_is_detected (scenarios/test_db_invariants.cpp, built in both configurations), so the enabled case count is now 118 in a regular build and 173 in a testnet build. With the floors left at 117 / 172 the guard would no longer catch a single-case test file dropping out of the build.
|
Rebase-free follow-up after #205 landed: it adds |
consensus_simis one Boost.Test binary that CI runs serially on one core of a 4-core runner. It's the longest step in two required jobs:What changes
tests/consensus_sim/run_parallel.py: lists enabled cases via--list_content, runs each as its own process (--run_test=suite/case),-j Nat a time. On failure it prints that case's full output; it always prints a slowest-cases table; it exits non-zero if any case failed.--filterlimits to given top-level suites; extra args after--are passed to every run.tests.yml: both jobs call it with-j$(nproc)frombuild/tests/consensus_sim, the directory ctest used, sofailures/logs and the upload-artifact path stay the same. The 2-validator arm keeps its three suites via--filter. The other ctest suites (pm_, node_admin_, …) still run throughctest -E '^consensus_sim$'.Why it's safe to split. Every
simulated_nodeopens its ownunique_pathtemp dir (64 MB shared file), failure logs are named<seed>-<scenario>.log, and there's no global fixture or shared mutable state across cases. Same set of cases runs as before — disabled cases stay skipped.Verified locally. The
--list_contentparser was checked against real output from Boost 1.83 (the Ubuntu floor) and 1.92 using a test binary with nested suites, a fixture suite and a disabled case: the right leaves are found, disabled ones skipped,--filterand pass-through args work, a failing case prints its output and gives exit 1. I couldn't run the real ASan-instrumentedconsensus_sim_testshere (the box has a 2 GB memory cap), so this PR's CI run is the real check.Expected effect. Up to ~4× on the simulator step, bounded by the slowest single case — the table at the end of the log will show it. If one case dominates (e.g. a big
pm_lifecycle_suitescenario), that's the next thing to look at. Independent of #202 (ccache); together they cut both compile and test time.