test_framework: detect failure of bitcoind startup #7744

pull laanwj wants to merge 1 commits into bitcoin:master from laanwj:2016_03_detect_startup_failure changing 1 files +40 −20

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).
laanwj added the label Tests on Mar 25, 2016
laanwj added the label RPC on Mar 25, 2016
laanwj force-pushed on Mar 25, 2016
laanwj force-pushed on Mar 25, 2016

laanwj commented at 3:12 PM on March 25, 2016: member

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

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

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

laanwj force-pushed on Mar 25, 2016
laanwj force-pushed on Mar 25, 2016
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.

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

laanwj force-pushed on Mar 26, 2016
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.

in qa/rpc-tests/test_framework/util.py:None 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!

jonasschnelli commented at 6:56 PM on March 28, 2016: contributor

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

utACK 018b60c
laanwj merged this on Mar 29, 2016
laanwj closed this on Mar 29, 2016
laanwj referenced this in commit d8e862a5a7 on Mar 29, 2016
in qa/rpc-tests/test_framework/util.py:None 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
MarcoFalke referenced this in commit 80b6bfaeaa on Jun 9, 2016
nomnombtc referenced this in commit 4adb243d84 on Nov 12, 2016
nomnombtc referenced this in commit 7918cf33e0 on Nov 12, 2016
nomnombtc referenced this in commit 38051716c2 on Nov 13, 2016
sickpig referenced this in commit ccc70d8624 on Nov 14, 2016
ftrader referenced this in commit 5ba022fab1 on Dec 9, 2016
zkbot referenced this in commit b2ab91b032 on Mar 24, 2020
zkbot referenced this in commit 38755ebc22 on Mar 24, 2020
zkbot referenced this in commit da12da80c4 on Apr 2, 2020
zkbot referenced this in commit 1be7250db9 on Apr 3, 2020
MarcoFalke locked this on Sep 8, 2021

Contributors

Labels

Linked (view graph)

#7463 salvagewallet fails verification #7745 test: wallet reindex test has a race