ci: Avoid oversubscription in functional tests on Windows

hebasto commented at 8:55 am on September 1, 2023: member

This PR aims to reduce the frequency of functional test failures on Windows like this one:

0
12023-09-01T01:05:01.850000Z TestFramework (ERROR): Assertion failed
2Traceback (most recent call last):
3  File "D:\a\bitcoin\bitcoin\test\functional\test_framework\test_framework.py", line 552, in start_nodes
4    node.wait_for_rpc_connection()
5  File "D:\a\bitcoin\bitcoin\test\functional\test_framework\test_node.py", line 296, in wait_for_rpc_connection
6    self._raise_assertion_error("Unable to connect to bitcoind after {}s".format(self.rpc_timeout))
7  File "D:\a\bitcoin\bitcoin\test\functional\test_framework\test_node.py", line 177, in _raise_assertion_error
8    raise AssertionError(self._node_msg(msg))
9AssertionError: [node 1] Unable to connect to bitcoind after 2400s

This code has had zero failures in my personal repository in more than 25 runs (and is still counting).

The second commit is a minor improvement to avoid “Cache save failed.” warnings during job re-runs. For example:

DrahtBot commented at 8:55 am on September 1, 2023: contributor

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	MarcoFalke

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

DrahtBot added the label Tests on Sep 1, 2023

maflcko commented at 9:19 am on September 1, 2023: member

The first commit will probably increase the run-time, but seems fine if it decreases the failure rate. Though, I don’t understand why the failures would happen previously, maybe an OOM?

The second commit seems fine, but I think can also be dropped. The config is hard enough to parse already and I think a warning should be fine to ignore, but no strong opinion.

maflcko approved

hebasto commented at 9:26 am on September 1, 2023: member

The first commit will probably increase the run-time

Yes, but insignificantly so.

Though, I don’t understand why the failures would happen previously, maybe an OOM?

My best guess is that the reason is the flawed thread scheduler in the virtual environment, which might cause a new thread to stall.

The second commit seems fine, but I think can also be dropped. The config is hard enough to parse already and I think a warning should be fine to ignore, but no strong opinion.

I’d keep it. This commit follows the behavior of the combined actions/cache.

ci: Avoid oversubscription in functional tests on Windows 14e5de6d02

ci: Avoid saving the same Ccache cache

This occurred when a job was being rerun.

f2d4e510b3

hebasto force-pushed on Sep 1, 2023

hebasto commented at 10:18 am on September 1, 2023: member

Updated 81806e257adc03bddd9bc3b2ccc5b57c318e3182 -> f2d4e510b37c0b132732ede214d29a8bf1daea93 (pr28384.01 -> pr28384.02, diff):

restored accidentally omitted TEST_RUNNER_EXTRA

hebasto commented at 11:12 am on September 1, 2023: member

GHA CI is green. Going to restart it to demonstrate the benefits of the second commit.

hebasto commented at 12:24 pm on September 1, 2023: member

Going to restart it to demonstrate the benefits of the second commit.

It is green again and clear of warnings as expected. @MarcoFalke

Want to leave your final (I hope) comment here?

maflcko commented at 12:33 pm on September 1, 2023: member

lgtm ACK f2d4e510b37c0b132732ede214d29a8bf1daea93 🐾

Signature:

0untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
1RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
2trusted comment: lgtm ACK f2d4e510b37c0b132732ede214d29a8bf1daea93 🐾
3iKbO5ixXRNdF9iEB+CfzJo/B1ZhUiZFKDkCDNCE9fHpfNOHeNpbpOGtH5up4V1e4O7teHxUn/a2AYpF0alPADw==

glozow merged this on Sep 1, 2023

glozow closed this on Sep 1, 2023

hebasto deleted the branch on Sep 1, 2023

sidhujag referenced this in commit a8f91ff997 on Sep 26, 2023

bitcoin locked this on Aug 31, 2024

ci: Avoid oversubscription in functional tests on Windows #28384

Reviews