From dfcef536d0e6c40e98dce35ae7af6e3e4a2595cd Mon Sep 17 00:00:00 2001 From: Matthew Zipkin Date: Fri, 15 Sep 2023 14:16:22 -0400 Subject: [PATCH] blockstorage: do not flush block to disk if it is already there test: ensure we can reindex from read-only block files now --- src/node/blockstorage.cpp | 24 ++++++++++----------- test/functional/feature_reindex_readonly.py | 9 ++++---- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index 211d5578263..996ac30c57e 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -906,19 +906,19 @@ bool BlockManager::FindBlockPos(FlatFilePos& pos, unsigned int nAddSize, unsigne if (!fKnown) { LogPrint(BCLog::BLOCKSTORAGE, "Leaving block file %i: %s (onto %i) (height %i)\n", last_blockfile, m_blockfile_info[last_blockfile].ToString(), nFile, nHeight); - } - // Do not propagate the return code. The flush concerns a previous block - // and undo file that has already been written to. If a flush fails - // here, and we crash, there is no expected additional block data - // inconsistency arising from the flush failure here. However, the undo - // data may be inconsistent after a crash if the flush is called during - // a reindex. A flush error might also leave some of the data files - // untrimmed. - if (!FlushBlockFile(last_blockfile, !fKnown, finalize_undo)) { - LogPrintLevel(BCLog::BLOCKSTORAGE, BCLog::Level::Warning, - "Failed to flush previous block file %05i (finalize=%i, finalize_undo=%i) before opening new block file %05i\n", - last_blockfile, !fKnown, finalize_undo, nFile); + // Do not propagate the return code. The flush concerns a previous block + // and undo file that has already been written to. If a flush fails + // here, and we crash, there is no expected additional block data + // inconsistency arising from the flush failure here. However, the undo + // data may be inconsistent after a crash if the flush is called during + // a reindex. A flush error might also leave some of the data files + // untrimmed. + if (!FlushBlockFile(last_blockfile, !fKnown, finalize_undo)) { + LogPrintLevel(BCLog::BLOCKSTORAGE, BCLog::Level::Warning, + "Failed to flush previous block file %05i (finalize=%i, finalize_undo=%i) before opening new block file %05i\n", + last_blockfile, !fKnown, finalize_undo, nFile); + } } // No undo data yet in the new file, so reset our undo-height tracking. m_blockfile_cursors[chain_type] = BlockfileCursor{nFile}; diff --git a/test/functional/feature_reindex_readonly.py b/test/functional/feature_reindex_readonly.py index dd99c3c4fa3..25cff87a3b7 100755 --- a/test/functional/feature_reindex_readonly.py +++ b/test/functional/feature_reindex_readonly.py @@ -24,6 +24,7 @@ class BlockstoreReindexTest(BitcoinTestFramework): opreturn = "6a" nulldata = fastprune_blockfile_size * "ff" self.generateblock(self.nodes[0], output=f"raw({opreturn}{nulldata})", transactions=[]) + block_count = self.nodes[0].getblockcount() self.stop_node(0) assert (self.nodes[0].chain_path / "blocks" / "blk00000.dat").exists() @@ -73,10 +74,10 @@ class BlockstoreReindexTest(BitcoinTestFramework): pass if undo_immutable: - self.log.info("Attempt to restart and reindex the node with the unwritable block file") - with self.nodes[0].assert_debug_log(expected_msgs=['FlushStateToDisk', 'failed to open file'], unexpected_msgs=[]): - self.nodes[0].assert_start_raises_init_error(extra_args=['-reindex', '-fastprune'], - expected_msg="Error: A fatal internal error occurred, see debug.log for details") + self.log.debug("Attempt to restart and reindex the node with the unwritable block file") + with self.nodes[0].wait_for_debug_log([b"Reindexing finished"]): + self.start_node(0, extra_args=['-reindex', '-fastprune']) + assert block_count == self.nodes[0].getblockcount() undo_immutable() filename.chmod(0o777)