[qa] pull-tester: Run rpc test in parallel

MarcoFalke commented at 10:50 am on April 29, 2016: member

Idea by @jtimon https://botbot.me/freenode/bitcoin-core-dev/2016-03-18/?msg=62458558&page=3 :+1:

MarcoFalke added the label Tests on Apr 29, 2016

MarcoFalke commented at 11:00 am on April 29, 2016: member

Looks like this runs the extended tests (no pruning) 4 times faster on my machine:

0Total time: 314 s (accumulated: 1306 s)

jonasschnelli commented at 11:31 am on April 29, 2016: contributor

Wow. This is a great idea. Concept ACK.

theuni commented at 7:09 pm on April 29, 2016: member

Concept ACK, thanks for working on this!

jtimon commented at 7:33 pm on April 29, 2016: contributor

Nice! Concept ACK. I wil review better and benchmark another day.

MarcoFalke force-pushed on Apr 30, 2016

MarcoFalke force-pushed on May 1, 2016

fanquake commented at 12:22 pm on May 5, 2016: member

First run testing this (shortened tests),

0ALL   | True   | 764 s (accumulated)
1
2Runtime: 212 s

Concept ACK

sdaftuar commented at 2:50 pm on May 5, 2016: member

@MarcoFalke This is great, thanks for tackling! Any idea what’s causing the travis failure?

I think it would be nice to make the number of simultaneous jobs configurable on the command line; the default of 4 seems good but being able to tweak it for the machine it’s running on (say if other developers run this script locally, as I do) would be nice.

I also noticed that by re-sorting the standard tests so that the slowest jobs start first, I was able to get the runtime of all the standard tests running in parallel down to the same as running the single longest test (walletbackup.py) – awesome!

sdaftuar commented at 3:23 pm on May 5, 2016: member

Hm, I just ran the extended tests and got an odd failure – this test looks like it is supposed to be marked successful.

 0bip9-softforks.py:
 1Initializing test directory /tmp/testE9D_TH
 2MiniNode: Connecting to Bitcoin Node IP # 127.0.0.1:13641
 3Test 1: PASS [143]
 4Test 2: PASS [287]
 5Test 3: PASS [431]
 6Test 4: PASS [574]
 7Test 5: PASS [575]
 8Test 6: PASS [575]
 9Initializing test directory /tmp/testE9D_TH
10MiniNode: Connecting to Bitcoin Node IP # 127.0.0.1:13641
11Test 7: PASS [143]
12Test 8: PASS [287]
13Test 9: PASS [431]
14Test 10: PASS [574]
15Test 11: PASS [575]
16Test 12: PASS [575]
17Initializing test directory /tmp/testE9D_TH
18MiniNode: Connecting to Bitcoin Node IP # 127.0.0.1:13641
19Test 13: PASS [143]
20Test 14: PASS [287]
21Test 15: PASS [431]
22Test 16: PASS [574]
23Test 17: PASS [575]
24Test 18: PASS [575]
25Initializing test directory /tmp/testE9D_TH
26MiniNode: Connecting to Bitcoin Node IP # 127.0.0.1:13641
27Stopping nodes
28Cleaning up
29Tests successful
30
31BDB3028 /tmp/testE9D_TH/blocks.db: unable to flush: No such file or directory
32
33Pass: False, Duration: 41 s

Maybe that last line about “unable to flush” is going to stderr, causing this to be recorded as failure? If so then probably the test should be fixed.

fanquake commented at 7:37 am on May 6, 2016: member

Agree with @sdaftuar that there is benefit in sorting the standard tests. Longest -> shortest:

0ALL | True   | 857 s (accumulated)
1Runtime: 218 s

Compared to as is:

0ALL | True   | 845 s (accumulated)
1Runtime: 238 s

paveljanik commented at 9:33 am on May 6, 2016: contributor

ACK https://github.com/bitcoin/bitcoin/pull/7972/commits/fa48ef8e82c7289fed12636bda8c99ca3ffcca4d

Thanks!

Edit: I was checking it pre-python3, so …

fanquake commented at 9:47 am on May 6, 2016: member

This needs to be updated to use http.client before it can merged, now that the Python3 change is in.

MarcoFalke force-pushed on May 9, 2016

[qa] pull-tester: Run rpc test in parallel fa494dec79

[qa] Add option --portseed to test_framework ccccc591a4

MarcoFalke force-pushed on May 9, 2016

MarcoFalke commented at 6:01 pm on May 9, 2016: member

Rebased and translated to python3
made it possible to specify -parallel=n on the command line
Moved walletbackup.py to first position
--portseed is set in a deterministic manner for each rpc test

MarcoFalke force-pushed on May 9, 2016

jtimon commented at 2:38 pm on May 10, 2016: contributor

 0$ python3 ./qa/pull-tester/rpc-tests.py -parallel=20000
 1...
 2ALL                            | True   | 634 s (accumulated)
 3Runtime: 143 s
 4
 5$ python3 ./qa/pull-tester/rpc-tests.py -parallel=20000 -extended
 6...
 7maxuploadtarget.py             | True   | 81 s
 8walletbackup.py                | True   | 135 s
 9smartfees.py                   | True   | 341 s
10pruning.py                     | True   | 1503 s
11
12ALL                            | False  | 3076 s (accumulated)
13
14Runtime: 1503 s

Now I’m too lazzy to run them before this patch to compare…benchmark conclusion: this is great.

It also comes to mind that, in terms of time, only smartfees and pruning deserve to be in the extended tests group more then walletbackup. But probably moving walletbackup to the extended tests (and maybe some of the faster ones out of the extended group to the regular one at the same time?) is out of the scope of this PR.

Tested ACK.

theuni commented at 3:43 pm on May 10, 2016: member

This is great. Looking at a random travis before/after, this goes from 535sec to 164sec. untested (locally) ACK.

sdaftuar commented at 4:02 pm on May 10, 2016: member

This looks great, though there’s still the issue with the bip9-softforks.py test outputting to stderr, causing the extended tests to fail. I’m trying to figure out a fix to suggest for that, but I’m not having any great ideas at the moment…

paveljanik commented at 4:05 pm on May 10, 2016: contributor

reACK https://github.com/bitcoin/bitcoin/pull/7972/commits/ccccc591a456f9a8fcf5f8e68a5bacdbbaeb0937

MarcoFalke commented at 4:06 pm on May 10, 2016: member

@sdaftuar This seems to be a local problem or a problem with the bip9 test and is not caused by this pull, I assume?

Runs fine locally, just now:

0TEST              | PASSED | DURATION
1
2bip9-softforks.py | True   | 66 s
3
4ALL               | True   | 66 s (accumulated)
5
6Runtime: 66 s

sdaftuar commented at 4:26 pm on May 10, 2016: member

Yep I didn’t realize this was just affecting me, will open a separate issue.

ACK.

MarcoFalke merged this on May 10, 2016

MarcoFalke closed this on May 10, 2016

MarcoFalke referenced this in commit 423ca302a3 on May 10, 2016

MarcoFalke deleted the branch on May 10, 2016

codablock referenced this in commit 614c117604 on Sep 16, 2017

codablock referenced this in commit 135bf14ace on Sep 19, 2017

codablock referenced this in commit f1996dc8ca on Dec 21, 2017

zkbot referenced this in commit 025bd44543 on Nov 21, 2020

zkbot referenced this in commit 7a0a268054 on Dec 2, 2020

zkbot referenced this in commit c8896f9907 on Dec 2, 2020

MarcoFalke locked this on Sep 8, 2021

[qa] pull-tester: Run rpc test in parallel #7972