-
Notifications
You must be signed in to change notification settings - Fork 38.7k
ci: Avoid oversubscription in functional tests on Windows #28384
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. |
|
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. |
Yes, but insignificantly so.
My best guess is that the reason is the flawed thread scheduler in the virtual environment, which might cause a new thread to stall.
I'd keep it. This commit follows the behavior of the combined |
This occurred when a job was being rerun.
81806e2 to
f2d4e51
Compare
|
Updated 81806e2 -> f2d4e51 (pr28384.01 -> pr28384.02, diff):
|
|
GHA CI is green. Going to restart it to demonstrate the benefits of the second commit. |
It is green again and clear of warnings as expected. Want to leave your final (I hope) comment here? |
|
lgtm ACK f2d4e51 🐾 Show signatureSignature: |
…n Windows f2d4e51 ci: Avoid saving the same Ccache cache (Hennadii Stepanov) 14e5de6 ci: Avoid oversubscription in functional tests on Windows (Hennadii Stepanov) Pull request description: This PR aims to reduce the frequency of functional test failures on Windows like this [one](https://github.com/bitcoin/bitcoin/actions/runs/6040229997): ``` 2023-09-01T01:05:01.850000Z TestFramework (ERROR): Assertion failed Traceback (most recent call last): File "D:\a\bitcoin\bitcoin\test\functional\test_framework\test_framework.py", line 552, in start_nodes node.wait_for_rpc_connection() File "D:\a\bitcoin\bitcoin\test\functional\test_framework\test_node.py", line 296, in wait_for_rpc_connection self._raise_assertion_error("Unable to connect to bitcoind after {}s".format(self.rpc_timeout)) File "D:\a\bitcoin\bitcoin\test\functional\test_framework\test_node.py", line 177, in _raise_assertion_error raise AssertionError(self._node_msg(msg)) AssertionError: [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](https://github.com/bitcoin/bitcoin/actions/runs/5998688759):  ACKs for top commit: MarcoFalke: lgtm ACK f2d4e51 🐾 Tree-SHA512: 0c92817d37325a114886900e49a4d644201397d98d6ac9f2dcd41170c7e7ea2cb1873f7e51b5cb3ad3cc2e59554ad1c8f87d439ea6c1c960bf5c339153be7040
This PR aims to reduce the frequency of functional test failures on Windows like this one:
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: