ci: Use all available CPUs for functional tests in “Win64 native” task #26297

1. 
   hebasto commented at 8:54 am on October 12, 2022: member

   On the master branch:

   This PR branch:

   Also consider “CPU Usage” charts provided by CI.

   Overlooked in cda62657e95a90a5fd61ba43e2acbd407e3a4135 (bitcoin/bitcoin#25929).

2. ci: Use all available CPUs for functional tests in "Win64 native" task 6fbd173d8a
3. hebasto added the label Windows on Oct 12, 2022
4. hebasto added the label Tests on Oct 12, 2022
5. aureleoules approved
6. 
   aureleoules commented at 9:06 am on October 12, 2022: member

   ACK 6fbd173d8a4519967d75d2707b2285d62faa4424 CI error seems unrelated
7. in .cirrus.yml:187 in 6fbd173d8a

   183@@ -184,7 +184,7 @@ task:
   184     - netsh int ipv4 set dynamicport tcp start=1025 num=64511
   185     - netsh int ipv6 set dynamicport tcp start=1025 num=64511
   186     # Exclude feature_dbcrash for now due to timeout
   187-    - python test\functional\test_runner.py --nocleanup --ci --quiet --combinedlogslen=4000 --jobs=4 --timeout-factor=8 --extended --exclude feature_dbcrash
   188+    - python test\functional\test_runner.py --nocleanup --ci --quiet --combinedlogslen=4000 --jobs=6 --timeout-factor=8 --extended --exclude feature_dbcrash
   

   ---

   ---

   

   maflcko commented at 9:25 am on October 12, 2022:

   0    - python test\functional\test_runner.py --nocleanup --ci --quiet --combinedlogslen=4000 --jobs=12 --timeout-factor=8 --extended --exclude feature_dbcrash
   

   There should be no risk in saturating the CPU, given enough memory

   ---

   

   hebasto commented at 10:18 am on October 12, 2022:

   Implemented in #26297 (comment).
8. hebasto force-pushed on Oct 12, 2022
9. 
   hebasto commented at 10:17 am on October 12, 2022: member

   Updated 6fbd173d8a4519967d75d2707b2285d62faa4424 -> dc869c6a77c4542d4d9dcefdec577d1876e51061 (pr26297.01 -> pr26297.02, diff):

   • addressed @MarcoFalke’s comment

   Let CI task finish to see the actual effect of a possible over-subscription.

10. 
    maflcko commented at 1:27 pm on October 12, 2022: member

    Looks like this makes the intermittent issues more common
11. hebasto force-pushed on Oct 12, 2022
12. 
    hebasto commented at 3:04 pm on October 12, 2022: member

    Looks like this makes the intermittent issues more common

    Indeed. Reverted back to 6fbd173d8a4519967d75d2707b2285d62faa4424 (pr26297.01), which was already ACKed by @aureleoules.

13. 
    DrahtBot commented at 3:04 pm on October 12, 2022: contributor

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

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #26286 (test: Remove unused txmempool include from tests by MarcoFalke)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

14. jarolrod approved
15. 
    jarolrod commented at 4:53 pm on October 12, 2022: member

    ACK 6fbd173d8a4519967d75d2707b2285d62faa4424

    Lgtm

16. shaavan approved
17. 
    shaavan commented at 3:10 pm on October 13, 2022: contributor

    ACK 6fbd173d8a4519967d75d2707b2285d62faa4424

    Makes sense to increase number of jobs equal to number of CPUs.

18. fanquake merged this on Oct 14, 2022
19. fanquake closed this on Oct 14, 2022

    ---

20. hebasto deleted the branch on Oct 14, 2022
21. 
    aureleoules commented at 8:19 am on October 14, 2022: member

    The merge commit description included hebasto’s comment even tough it’s not a ACK @MarcoFalke, probably not that big of a deal though. Edit: Sorry for the ping I confused DrahtBot and the github-merge script :grimacing:

    ACKs for top commit: hebasto: Indeed. Reverted back to https://github.com/bitcoin/bitcoin/commit/6fbd173d8a4519967d75d2707b2285d62faa4424 (pr26297.01), which was already ACKed by @aureleoules. aureleoules: ACK https://github.com/bitcoin/bitcoin/commit/6fbd173d8a4519967d75d2707b2285d62faa4424 jarolrod: ACK https://github.com/bitcoin/bitcoin/commit/6fbd173d8a4519967d75d2707b2285d62faa4424 shaavan: ACK https://github.com/bitcoin/bitcoin/commit/6fbd173d8a4519967d75d2707b2285d62faa4424

22. 
    maflcko commented at 8:42 am on October 18, 2022: member

    I think we should fix the intermittent issues and then set the jobs to 12
23. 
    hebasto commented at 4:01 pm on October 20, 2022: member

    and then set the jobs to 12

    Why? CPUs are used quite effectively now:

    

24. 
    maflcko commented at 7:33 am on October 21, 2022: member

    Why

    So that intermittent issues are happening more consistently and can then hopefully be found and fixed before they make it into the master branch

25. bitcoin locked this on Oct 21, 2023

Contributors
hebasto aureleoules maflcko DrahtBot jarolrod shaavan

Labels
Windows Tests