test: wallet reindex test has a race #7745

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

   It doesn’t actually wait for the reindex to complete - so sometimes one gets this error, where there’s no balance yet:

    0check -reindex
    1Assertion failed: [Decimal('33.99911100'), Decimal('5649.99796160'), Decimal('59.99974800')] != [Decimal('33.99911100'), Decimal('5649.99796160'), Decimal('0E-8')]
    2  File "/data/src/bitcoin/qa/rpc-tests/test_framework/test_framework.py", line 135, in main
    3    self.run_test()
    4  File "./wallet.py", line 296, in run_test
    5    assert_equal(balance_nodes, [self.nodes[i].getbalance() for i in range(3)])
    6  File "/data/src/bitcoin/qa/rpc-tests/test_framework/util.py", line 428, in assert_equal
    7    raise AssertionError("%s != %s"%(str(thing1),str(thing2)))
    8Stopping nodes
    9Cleaning up
   10Failed
   

   The error is kind of rare as reindexing of the regtest chain is very fast. It does seem to happen more after #7744, as it has less delay in the startup.

2. laanwj added the label Tests on Mar 25, 2016
3. laanwj added the label RPC on Mar 25, 2016
4. 
   MarcoFalke commented at 3:10 pm on March 25, 2016: member

   I wasn’t really sure what is required here but it seems connecting the nodes is too much, so I removed that in fad7dc8a6c0ca9c067a249cf8896dd2e64703e48.
5. 
   laanwj commented at 4:13 pm on March 25, 2016: member

   Most straightforward would be to get the block height before the maintenance tests, store it, then after the reindex poll and wait for the block height to be that again.
6. 
   MarcoFalke commented at 4:19 pm on March 25, 2016: member

   Shouldn’t the call fail instead and indicate that the node is still “loading”?

   0$ src/bitcoin-cli -testnet getbalance
   1error code: -28
   2error message:
   3Loading block index...
   

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

   When?

   Note that bitcoin-cli -rpcwait as well as wait_for_bitcoind_start in #7744 waits for the warmup phase (code -28) to finish.

8. 
   MarcoFalke commented at 3:36 pm on March 28, 2016: member

   It doesn’t actually wait for the reindex to complete

   waits for the warmup phase (code -28) to finish.

   So reindex is not counted as “warmup” but should be, instead?

9. 
   sipa commented at 6:16 pm on March 28, 2016: member

   I’m not sure. Reindex is not really special, as the node is fully up and running (apart from block sync), it’s just simultaneously importing blocks from disk.
10. laanwj closed this on Mar 29, 2016

    ---

11. 
    laanwj commented at 9:01 am on March 29, 2016: member

    So reindex is not counted as “warmup” but should be, instead?

    No, I don’t think so. Remember reindex can take a long time on the main chain, we have no reason for RPC to be available in the mean time. The warmup phase exists because it would be unsafe to offer RPC functionality (e.g. everything expects pcoinsTip to be populated).

    I wouldn’t be against adding a reindex_in_progress status field to some RPC call though.

12. DrahtBot locked this on Sep 8, 2021

Contributors
laanwj MarcoFalke sipa

Labels
Tests RPC/REST/ZMQ