backport: partial bitcoin#33079 to run more asan jobs at the same time - #7633
Conversation
3b23f95 ci: limit max stack size to 512 KiB (dergoegge) 2931a87 ci: limit stack size to 512kb in native macOS jobs (fanquake) Partial: only the asan target and only the functional test stage. Upstream sets CI_LIMIT_STACK_SIZE in ci/test/03_test_script.sh, which Dash does not have; the equivalent point in Dash's flow is ci/dash/test_integrationtests.sh, after the build and before test_runner. Upstream also sets the variable for the arm, msan, tsan and previous_releases targets. Those are green today and unit tests are not where the job runs out of memory, so leave them alone rather than perturb a passing configuration. Upstream's 512 KiB does not work for Dash: dashd dies with SIGSEGV during test framework startup. 1024 KiB is the smallest value tested that starts cleanly (512 fails, 1024, 2048 and 4096 all pass). Taken here for its effect on memory rather than its original purpose. ASan sizes a thread's fake stack from the stack rlimit, and the threads that actually run fault theirs in, so lowering the limit cuts resident memory substantially. Peak resident set of all dashd nodes over feature_llmq_data_recovery, feature_llmq_rotation, p2p_instantsend and p2p_quorum_data at -j4: 8 MiB (default) 14335 MiB 2048 KiB 12912 MiB 1024 KiB 10657 MiB Thread count is not what drives this. The same run with -par=4 -parbls=4, which takes each node from 15 script and 23 BLS worker threads down to 3 and 3, peaked at 14375 MiB, unchanged. Idle workers never fault their stacks in.
|
:( do we really need asan on each pr? Takes so long |
|
✅ Final review complete — no blockers (commit e357eb1) |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review. WalkthroughThe ASAN test setup now runs four parallel jobs instead of two and exports Estimated code review effort: 1 (Trivial) | ~5 minutes Merge Risk: ⚪ Minimal · up to This PR adjusts CI test parallelism and sanitizer memory settings without any identified merge-blocking risk; it is merge-ready after normal checks and review. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Final validation — Codex/Sol only (Phase 2 disabled)
At exact head e357eb1, the ASan-only environment variable propagates through the CI matrix, applies the 1024 KiB stack limit before functional tests, and restores four-way test parallelism as intended. The changed scripts pass shell syntax validation, the configured stack limit applies successfully, and no in-scope defects were identified.
Source: Codex general, commit-history, and backport-reviewer lanes (exact backend model IDs were not supplied in the evidence); final verifier: Claude Agent SDK (exact backend model ID was not supplied). openclaw-agent/cliproxy/gpt-5.6-sol is orchestration-only and is not reviewer evidence.
Validated zero-blocker Codex/Sol precheck evidence was promoted to final because Phase 2 (Sonnet/Opus) is temporarily disabled. This is Codex/Sol-only final validation, not Codex + Sonnet/Opus coverage.
Review provenance
- Codex reviewers:
gpt-5.6-sol— general (completed),gpt-5.6-sol— dash-core-commit-history (completed),gpt-5.6-sol— backport-reviewer (completed) - Verifier:
gpt-5.6-sol— verifier - Sonnet/Opus: not run (Phase 2 disabled — temporary Codex/Sol-only final)
- Secondary pass: disabled (
temporary_phase2_sonnet_disable)
asan: Job even finished faster than tsan on 7633. Also note that
Not really need but it seems useful for me btw, asan already found a real issue on my PR - see dcca45b . Agents also pointed this issue out, so, I'd say it's useful but not like we won't survive without it. But looking to your pessimism about asan I am not gonna push msan ; I will create PR with msan related changes but won't enable it on CI. |
3 participants
Issue being fixed or feature implemented
Having 4jobs for asan causes functional tests to be aborted without an explanation on CI in logs.
Most likely it's caused by short memory and amount of jobs has been reduced to only 2 jobs.
What was done?
Backport bitcoin#33079 partially + increased amount of jobs
How Has This Been Tested?
Local tests shown that RAM used significantly reduced if 33079 included:
Probably it will let to run more than 2 jobs at once.
Breaking Changes
N/A
Checklist: