ci: reduce nproc multipliers

[Sjors]

The timeout factor increase in #263 reliably causes SIGTERM/143 exit code errors.

https://github.com/bitcoin-core/libmultiprocess/actions/runs/23559105412/job/68593864277?pr=240 https://github.com/bitcoin-core/libmultiprocess/actions/runs/23559105412/job/68593864283?pr=240 https://github.com/bitcoin-core/libmultiprocess/actions/runs/23567207693/job/68621529932 https://github.com/bitcoin-core/libmultiprocess/actions/runs/23567207693/job/68621529938 https://github.com/bitcoin-core/libmultiprocess/actions/runs/23583136495/job/68670044896?pr=256 https://github.com/bitcoin-core/libmultiprocess/actions/runs/23583136495/job/68670044932?pr=256 https://github.com/bitcoin-core/libmultiprocess/actions/runs/23587830246/job/68685257102?pr=249 https://github.com/bitcoin-core/libmultiprocess/actions/runs/23587830246/job/68685257218?pr=249

Reduce the nproc multipliers to try and prevent that.

[DrahtBot]

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	ryanofsky

If your review is incorrectly listed, please copy-paste <!--meta-tag:bot-skip--> into the comment that the bot should ignore.

[Sjors]

This seems more stable, given that it ran for an hour without failing. But now it takes too long, so the total number of jobs needs to be reduced (we can add a long runner on master later).

Pushed again: https://github.com/bitcoin-core/libmultiprocess/compare/31a4b41ac12d42f9b2112165e175695963844ddb..cb9586c9e8ad65ca962e97030761749bc6196b9e

[ryanofsky]

This seems more stable, given that it ran for an hour without failing. But now it takes too long, so the total number of jobs needs to be reduced (we can add a long runner on master later).

It seems like there is a big disparity in the time it takes to run the interface_ipc.py test, which is 1-3 sec, and the time it takes to interface_ipc_mining.py test, which is around 2 minutes. Ideally, we would not need to reduce the number of times the first test is run just because the second test is slow.

The disparity could also explain the reason why the resource exhaustion doesn't happen right away, because if the second test uses more resources than the first test, the runner will start off executing using equal numbers of both tests and the resource limit won't be hit. But as instances of the first test quickly finish, more copies of the second test will be running and resource usage will gradually increase.

I think a good solution to these problems might be simplify the run_ipc_functional_tests function to just run test_runner.py twice and use a higher number of runs and higher job count for the cheap test and a lower number of runs and lower job count for the expensive test. We could replace functional_test_runs and nproc_multiplier with separate variables for each test to make this configurable, or hardcode values into the function, or use different multiplier variables

[Sjors]

I reduced the number of runs further.

Ideally, we would not need to reduce the number of times the first test is run just because the second test is slow.

I don't think these tests "deserve" equal run time. The mining interface is far more likely to have bugs, so the fact that uses proportionally more runtime seems fine to me.

We could replace functional_test_runs and nproc_multiplier with separate variables for each test to make this configurable

For this PR I'd rather not increase complexity. In general I tried to write this in a way that if Bitcoin Core adds new IPC functional tests, or splits the big mining test, they get picked up, but then the number of variables would have to keep increasing.

[ryanofsky]

I don't think these tests "deserve" equal run time.

The current approach seems ok, but I think if the goal of the repeating the tests is to detect race conditions, the faster tests will be more effective at triggering races so it makes sense to run them more times. Similarly, if one test uses less resources than the other, it could make sense to use a higher number of jobs for it so resource usage is maximized.

So I do think running just test runner twice as suggested could help us get more utility out of these tests and not add much complexity, though I'd agree it would not be an ideal solution.

Probably more ideally the test runner would accept a --repeat-for option that automatically repeats tests for a number of seconds or minutes, continuously adding tests to the queue and roughly giving equal runtime to each test so slow/expensive tests do not dominate the queue. (A simple approach might be to add new test to the queue each time an existing test completes, choosing the test that has the least total runtime at that point.)

[ryanofsky]

Code review ACK 3360233. Thanks for the fix!

Happy to merge as-is or you can let me know if you want to make more tweaks

[Sjors]

The "run functional test" phases are still slightly longer than I would like, but it's good enough. Assuming TSan passes, go ahead and merge.

[Sjors]

the test runner would accept a --repeat-for option that automatically repeats tests for a number of seconds or minutes

Yeah that would be much better.