test_framework: detect failure of bitcoind startup #7744

1. 
   laanwj commented at 1:24 pm on March 25, 2016: member

   Replace the bitcoin-cli -rpcwait after spawning bitcoind with our own loop that detects when bitcoind exits prematurely (before RPC is up).

   This prevents a hang in such a case (see #7463).

2. laanwj added the label Tests on Mar 25, 2016
3. laanwj added the label RPC on Mar 25, 2016
4. laanwj force-pushed on Mar 25, 2016
5. laanwj force-pushed on Mar 25, 2016
6. 
   laanwj commented at 3:12 pm on March 25, 2016: member

   Not ready to merge yet. I’ve tested this with a -salvagewallet loop:

    0Unexpected exception caught during testing: bitcoind exited with status 1 during initialization
    1  File "/data/src/bitcoin/qa/rpc-tests/test_framework/test_framework.py", line 135, in main
    2    self.run_test()
    3  File "./wallet.py", line 295, in run_test
    4    self.nodes = start_nodes(3, self.options.tmpdir, [[m]] * 3)
    5  File "/data/src/bitcoin/qa/rpc-tests/test_framework/util.py", line 283, in start_nodes
    6    return [ start_node(i, dirname, extra_args[i], rpchost, binary=binary[i]) for i in range(num_nodes) ]
    7  File "/data/src/bitcoin/qa/rpc-tests/test_framework/util.py", line 267, in start_node
    8    wait_for_bitcoind_start(bitcoind_processes[i], url, i)
    9  File "/data/src/bitcoin/qa/rpc-tests/test_framework/util.py", line 144, in wait_for_bitcoind_start
   10    raise Exception('bitcoind exited with status %i during initialization' % process.returncode)
   11Stopping nodes
   

   Even though reporting is better, it still hangs after “Stopping nodes”, going to investigate why.

7. laanwj force-pushed on Mar 25, 2016
8. laanwj force-pushed on Mar 25, 2016
9. 
   laanwj commented at 4:27 pm on March 25, 2016: member

   Should be fixed now. The problem was that start_nodes can fail with part of the nodes already started. wait_bitcoinds will subsequently wait forever for them to exit (as it uses a global variable to communicate bitcoind_processes). One-to-last commit makes sure that they’re stopped properly in that case.
10. test_framework: detect failure of bitcoind startup

    Replace the `bitcoin-cli -rpcwait` after spawning bitcoind
    with our own loop that detects when bitcoind exits prematurely.
    
    And if one node fails to start, stop the others.
    
    This prevents a hang in such a case (see #7463).

    018b60c5ea
11. laanwj force-pushed on Mar 26, 2016
12. 
    laanwj commented at 9:37 am on March 26, 2016: member

    This is faster than the current -rpcwait construction, so #7745 triggers more, seems I need to solve that too here. Thanks Marco Falke for fixing this already.
13. in qa/rpc-tests/test_framework/util.py: in 018b60c5ea

    279@@ -267,7 +280,14 @@ def start_nodes(num_nodes, dirname, extra_args=None, rpchost=None, binary=None):
    280     """
    281     if extra_args is None: extra_args = [ None for i in range(num_nodes) ]
    282     if binary is None: binary = [ None for i in range(num_nodes) ]
    283-    return [ start_node(i, dirname, extra_args[i], rpchost, binary=binary[i]) for i in range(num_nodes) ]
    284+    rpcs = []
    285+    try:
    286+        for i in range(num_nodes):
    287+            rpcs.append(start_node(i, dirname, extra_args[i], rpchost, binary=binary[i]))
    288+    except: # If one node failed to start, stop the others
    

    ---

    ---

    

    MarcoFalke commented at 3:29 pm on March 28, 2016:

    Nice!
14. 
    jonasschnelli commented at 6:56 pm on March 28, 2016: contributor

    Good idea! utACK 018b60c5ea703ed12edcde034a185f79e77e5576
15. 
    MarcoFalke commented at 2:55 pm on March 29, 2016: member

    utACK 018b60c
16. laanwj merged this on Mar 29, 2016
17. laanwj closed this on Mar 29, 2016

    ---

18. laanwj referenced this in commit d8e862a5a7 on Mar 29, 2016
19. in qa/rpc-tests/test_framework/util.py: in 018b60c5ea

    130@@ -130,11 +131,33 @@ def initialize_datadir(dirname, n):
    131         f.write("listenonion=0\n")
    132     return datadir
    133 
    134+def rpc_url(i, rpchost=None):
    135+    return "http://rt:rt@%s:%d" % (rpchost or '127.0.0.1', rpc_port(i))
    

    ---

    ---

    

    MarcoFalke commented at 6:28 pm on April 10, 2016:

    Nit: rpcbind_test.py no longer works after this.

    ---

    

    laanwj commented at 1:40 pm on April 15, 2016:

    We should probably re-add that test to one of the standard sequence, otherwise it will just bitrot
20. MarcoFalke referenced this in commit 80b6bfaeaa on Jun 9, 2016
21. nomnombtc referenced this in commit 4adb243d84 on Nov 12, 2016
22. nomnombtc referenced this in commit 7918cf33e0 on Nov 12, 2016
23. nomnombtc referenced this in commit 38051716c2 on Nov 13, 2016
24. sickpig referenced this in commit ccc70d8624 on Nov 14, 2016
25. ftrader referenced this in commit 5ba022fab1 on Dec 9, 2016
26. zkbot referenced this in commit b2ab91b032 on Mar 24, 2020
27. zkbot referenced this in commit 38755ebc22 on Mar 24, 2020
28. zkbot referenced this in commit da12da80c4 on Apr 2, 2020
29. zkbot referenced this in commit 1be7250db9 on Apr 3, 2020
30. MarcoFalke locked this on Sep 8, 2021

Contributors
laanwj MarcoFalke jonasschnelli

Labels
Tests RPC/REST/ZMQ