ci: Avoid oversubscription in functional tests on Windows #28384

1. 
   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:

   

2. 
   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.

3. DrahtBot added the label Tests on Sep 1, 2023
4. 
   MarcoFalke 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.

5. MarcoFalke approved
6. 
   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.

7. ci: Avoid oversubscription in functional tests on Windows 14e5de6d02
8. ci: Avoid saving the same Ccache cache

   This occurred when a job was being rerun.

   f2d4e510b3
9. hebasto force-pushed on Sep 1, 2023
10. 
    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
11. 
    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.
12. 
    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?

13. 
    MarcoFalke 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==
    

14. glozow merged this on Sep 1, 2023
15. glozow closed this on Sep 1, 2023

    ---

16. hebasto deleted the branch on Sep 1, 2023

Contributors
hebasto DrahtBot MarcoFalke

Labels
Tests