diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index a81099a26c..c0a25572fc 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -371,6 +371,23 @@ bool BlockManager::LoadBlockIndexDB(const Consensus::Params& consensus_params) return true; } +void BlockManager::ScanAndUnlinkAlreadyPrunedFiles() +{ + AssertLockHeld(::cs_main); + if (!m_have_pruned) { + return; + } + + std::set block_files_to_prune; + for (int file_number = 0; file_number < m_last_blockfile; file_number++) { + if (m_blockfile_info[file_number].nSize == 0) { + block_files_to_prune.insert(file_number); + } + } + + UnlinkPrunedFiles(block_files_to_prune); +} + const CBlockIndex* BlockManager::GetLastCheckpoint(const CCheckpointData& data) { const MapCheckpoints& checkpoints = data.mapCheckpoints; @@ -556,11 +573,14 @@ uint64_t BlockManager::CalculateCurrentUsage() void UnlinkPrunedFiles(const std::set& setFilesToPrune) { + std::error_code ec; for (std::set::iterator it = setFilesToPrune.begin(); it != setFilesToPrune.end(); ++it) { FlatFilePos pos(*it, 0); - fs::remove(BlockFileSeq().FileName(pos)); - fs::remove(UndoFileSeq().FileName(pos)); - LogPrint(BCLog::BLOCKSTORE, "Prune: %s deleted blk/rev (%05u)\n", __func__, *it); + const bool removed_blockfile{fs::remove(BlockFileSeq().FileName(pos), ec)}; + const bool removed_undofile{fs::remove(UndoFileSeq().FileName(pos), ec)}; + if (removed_blockfile || removed_undofile) { + LogPrint(BCLog::BLOCKSTORE, "Prune: %s deleted blk/rev (%05u)\n", __func__, *it); + } } } diff --git a/src/node/blockstorage.h b/src/node/blockstorage.h index b6007897df..09aff7f08b 100644 --- a/src/node/blockstorage.h +++ b/src/node/blockstorage.h @@ -154,6 +154,13 @@ public: bool WriteBlockIndexDB() EXCLUSIVE_LOCKS_REQUIRED(::cs_main); bool LoadBlockIndexDB(const Consensus::Params& consensus_params) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + /** + * Remove any pruned block & undo files that are still on disk. + * This could happen on some systems if the file was still being read while unlinked, + * or if we crash before unlinking. + */ + void ScanAndUnlinkAlreadyPrunedFiles() EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + CBlockIndex* AddToBlockIndex(const CBlockHeader& block, CBlockIndex*& best_header) EXCLUSIVE_LOCKS_REQUIRED(cs_main); /** Create a new block index entry for a given block hash */ CBlockIndex* InsertBlockIndex(const uint256& hash) EXCLUSIVE_LOCKS_REQUIRED(cs_main); diff --git a/src/test/blockmanager_tests.cpp b/src/test/blockmanager_tests.cpp index dd7c32cc46..d5825d0d7e 100644 --- a/src/test/blockmanager_tests.cpp +++ b/src/test/blockmanager_tests.cpp @@ -12,6 +12,8 @@ using node::BlockManager; using node::BLOCK_SERIALIZATION_HEADER_SIZE; +using node::MAX_BLOCKFILE_SIZE; +using node::OpenBlockFile; // use BasicTestingSetup here for the data directory configuration, setup, and cleanup BOOST_FIXTURE_TEST_SUITE(blockmanager_tests, BasicTestingSetup) @@ -39,4 +41,45 @@ BOOST_AUTO_TEST_CASE(blockmanager_find_block_pos) BOOST_CHECK_EQUAL(actual.nPos, BLOCK_SERIALIZATION_HEADER_SIZE + ::GetSerializeSize(params->GenesisBlock(), CLIENT_VERSION) + BLOCK_SERIALIZATION_HEADER_SIZE); } +BOOST_FIXTURE_TEST_CASE(blockmanager_scan_unlink_already_pruned_files, TestChain100Setup) +{ + // Cap last block file size, and mine new block in a new block file. + const auto& chainman = Assert(m_node.chainman); + auto& blockman = chainman->m_blockman; + const CBlockIndex* old_tip{WITH_LOCK(chainman->GetMutex(), return chainman->ActiveChain().Tip())}; + WITH_LOCK(chainman->GetMutex(), blockman.GetBlockFileInfo(old_tip->GetBlockPos().nFile)->nSize = MAX_BLOCKFILE_SIZE); + CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())); + + // Prune the older block file, but don't unlink it + int file_number; + { + LOCK(chainman->GetMutex()); + file_number = old_tip->GetBlockPos().nFile; + blockman.PruneOneBlockFile(file_number); + } + + const FlatFilePos pos(file_number, 0); + + // Check that the file is not unlinked after ScanAndUnlinkAlreadyPrunedFiles + // if m_have_pruned is not yet set + WITH_LOCK(chainman->GetMutex(), blockman.ScanAndUnlinkAlreadyPrunedFiles()); + BOOST_CHECK(!AutoFile(OpenBlockFile(pos, true)).IsNull()); + + // Check that the file is unlinked after ScanAndUnlinkAlreadyPrunedFiles + // once m_have_pruned is set + blockman.m_have_pruned = true; + WITH_LOCK(chainman->GetMutex(), blockman.ScanAndUnlinkAlreadyPrunedFiles()); + BOOST_CHECK(AutoFile(OpenBlockFile(pos, true)).IsNull()); + + // Check that calling with already pruned files doesn't cause an error + WITH_LOCK(chainman->GetMutex(), blockman.ScanAndUnlinkAlreadyPrunedFiles()); + + // Check that the new tip file has not been removed + const CBlockIndex* new_tip{WITH_LOCK(chainman->GetMutex(), return chainman->ActiveChain().Tip())}; + BOOST_CHECK_NE(old_tip, new_tip); + const int new_file_number{WITH_LOCK(chainman->GetMutex(), return new_tip->GetBlockPos().nFile)}; + const FlatFilePos new_pos(new_file_number, 0); + BOOST_CHECK(!AutoFile(OpenBlockFile(new_pos, true)).IsNull()); +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/src/validation.cpp b/src/validation.cpp index 1357de3c01..64e4d148de 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -4323,6 +4323,8 @@ bool ChainstateManager::LoadBlockIndex() bool ret = m_blockman.LoadBlockIndexDB(GetConsensus()); if (!ret) return false; + m_blockman.ScanAndUnlinkAlreadyPrunedFiles(); + std::vector vSortedByHeight{m_blockman.GetAllBlockIndices()}; std::sort(vSortedByHeight.begin(), vSortedByHeight.end(), CBlockIndexHeightOnlyComparator()); diff --git a/test/functional/feature_remove_pruned_files_on_startup.py b/test/functional/feature_remove_pruned_files_on_startup.py new file mode 100755 index 0000000000..ca0e5ace9f --- /dev/null +++ b/test/functional/feature_remove_pruned_files_on_startup.py @@ -0,0 +1,54 @@ +#!/usr/bin/env python3 +# Copyright (c) 2022 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +"""Test removing undeleted pruned blk files on startup.""" + +import os +from test_framework.test_framework import BitcoinTestFramework + +class FeatureRemovePrunedFilesOnStartupTest(BitcoinTestFramework): + def set_test_params(self): + self.num_nodes = 1 + self.extra_args = [["-fastprune", "-prune=1"]] + + def mine_batches(self, blocks): + n = blocks // 250 + for _ in range(n): + self.generate(self.nodes[0], 250) + self.generate(self.nodes[0], blocks % 250) + self.sync_blocks() + + def run_test(self): + blk0 = os.path.join(self.nodes[0].datadir, self.nodes[0].chain, 'blocks', 'blk00000.dat') + rev0 = os.path.join(self.nodes[0].datadir, self.nodes[0].chain, 'blocks', 'rev00000.dat') + blk1 = os.path.join(self.nodes[0].datadir, self.nodes[0].chain, 'blocks', 'blk00001.dat') + rev1 = os.path.join(self.nodes[0].datadir, self.nodes[0].chain, 'blocks', 'rev00001.dat') + self.mine_batches(800) + fo1 = os.open(blk0, os.O_RDONLY) + fo2 = os.open(rev1, os.O_RDONLY) + fd1 = os.fdopen(fo1) + fd2 = os.fdopen(fo2) + self.nodes[0].pruneblockchain(600) + + # Windows systems will not remove files with an open fd + if os.name != 'nt': + assert not os.path.exists(blk0) + assert not os.path.exists(rev0) + assert not os.path.exists(blk1) + assert not os.path.exists(rev1) + else: + assert os.path.exists(blk0) + assert not os.path.exists(rev0) + assert not os.path.exists(blk1) + assert os.path.exists(rev1) + + # Check that the files are removed on restart once the fds are closed + fd1.close() + fd2.close() + self.restart_node(0) + assert not os.path.exists(blk0) + assert not os.path.exists(rev1) + +if __name__ == '__main__': + FeatureRemovePrunedFilesOnStartupTest().main() diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 26ebce039b..1d560706c2 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -342,6 +342,7 @@ BASE_SCRIPTS = [ 'p2p_permissions.py', 'feature_blocksdir.py', 'wallet_startup.py', + 'feature_remove_pruned_files_on_startup.py', 'p2p_i2p_ports.py', 'p2p_i2p_sessions.py', 'feature_config_args.py',