Merge bitcoin/bitcoin#30132: indexes: Don't wipe indexes again when continuing a prior reindex

f68cba29b3 blockman: Replace m_reindexing with m_blockfiles_indexed (Ryan Ofsky)
1b1c6dcca0 test: Add functional test for continuing a reindex (TheCharlatan)
201c1a9282 indexes: Don't wipe indexes again when already reindexing (TheCharlatan)
804f09dfa1 kernel: Add less confusing reindex options (Ryan Ofsky)
e172553223 validation: Remove needs_init from LoadBlockIndex (TheCharlatan)
533eab7d67 bugfix: Streamline setting reindex option (TheCharlatan)

Pull request description:

  When restarting `bitcoind` during an ongoing reindex without setting the `-reindex` flag again, the block and coins db is left intact, but any data from the optional indexes is discarded. While not a bug per se, wiping the data again is
  wasteful, both in terms of having to write it again,  as well as potentially leading to longer startup times. So keep the  index data instead when continuing a prior reindex.

  Also includes a bugfix and smaller code cleanups around the reindexing code. The bug was introduced in b47bd95920: "kernel: De-globalize fReindex".

ACKs for top commit:
  stickies-v:
    ACK f68cba29b3
  fjahr:
    Code review ACK f68cba29b3
  furszy:
    Code review ACK f68cba29b3
  ryanofsky:
    Code review ACK f68cba29b3. Only changes since last review were cherry-picking suggested commits that rename variables, improving comments, and making some tweaks to test code.

Tree-SHA512: b252228cc76e9f1eaac56d5bd9e4eac23408e0fc04aeffd97a85417f046229364673ee1ca7410b9b6e7b692b03f13ece17c42a10176da0d7e975a8915deb98ca

test/functional/feature_reindex.py

@@ -73,6 +73,25 @@ class ReindexTest(BitcoinTestFramework):
         # All blocks should be accepted and processed.
         assert_equal(self.nodes[0].getblockcount(), 12)
 
+    def continue_reindex_after_shutdown(self):
+        node = self.nodes[0]
+        self.generate(node, 1500)
+
+        # Restart node with reindex and stop reindex as soon as it starts reindexing
+        self.log.info("Restarting node while reindexing..")
+        node.stop_node()
+        with node.busy_wait_for_debug_log([b'initload thread start']):
+            node.start(['-blockfilterindex', '-reindex'])
+            node.wait_for_rpc_connection(wait_for_import=False)
+        node.stop_node()
+
+        # Start node without the reindex flag and verify it does not wipe the indexes data again
+        db_path = node.chain_path / 'indexes' / 'blockfilter' / 'basic' / 'db'
+        with node.assert_debug_log(expected_msgs=[f'Opening LevelDB in {db_path}'], unexpected_msgs=[f'Wiping LevelDB in {db_path}']):
+            node.start(['-blockfilterindex'])
+            node.wait_for_rpc_connection(wait_for_import=False)
+        node.stop_node()
+
     def run_test(self):
         self.reindex(False)
         self.reindex(True)
@@ -80,6 +99,7 @@ class ReindexTest(BitcoinTestFramework):
         self.reindex(True)
 
         self.out_of_order()
+        self.continue_reindex_after_shutdown()
 
 
 if __name__ == '__main__':

test/functional/test_framework/test_node.py

@@ -241,7 +241,7 @@ class TestNode():
         if self.start_perf:
             self._start_perf()
 
-    def wait_for_rpc_connection(self):
+    def wait_for_rpc_connection(self, *, wait_for_import=True):
         """Sets up an RPC connection to the bitcoind process. Returns False if unable to connect."""
         # Poll at a rate of four times per second
         poll_per_s = 4
@@ -263,7 +263,7 @@ class TestNode():
                 )
                 rpc.getblockcount()
                 # If the call to getblockcount() succeeds then the RPC connection is up
-                if self.version_is_at_least(190000):
+                if self.version_is_at_least(190000) and wait_for_import:
                     # getmempoolinfo.loaded is available since commit
                     # bb8ae2c (version 0.19.0)
                     self.wait_until(lambda: rpc.getmempoolinfo()['loaded'])