From 8bb65cedbaf4a84d4018cc194985aa02c8a51043 Mon Sep 17 00:00:00 2001 From: Russell Yanofsky Date: Wed, 27 Mar 2019 15:50:03 -0400 Subject: [PATCH] Simplify rewind/commit control flow --- src/index/base.cpp | 59 ++++++++++++++++------------------ src/index/base.h | 15 ++++----- src/index/blockfilterindex.cpp | 15 ++++----- src/index/blockfilterindex.h | 2 +- 4 files changed, 42 insertions(+), 49 deletions(-) diff --git a/src/index/base.cpp b/src/index/base.cpp index 6b8a45a94b8db1..490ba18959b6c9 100644 --- a/src/index/base.cpp +++ b/src/index/base.cpp @@ -95,8 +95,7 @@ void BaseIndex::ThreadSync() int64_t last_locator_write_time = 0; while (true) { if (m_interrupt) { - m_best_block_index = pindex; - Commit(); + InternalCommit(pindex); return; } @@ -104,12 +103,11 @@ void BaseIndex::ThreadSync() LOCK(cs_main); const CBlockIndex* pindex_next = NextSyncBlock(pindex); if (!pindex_next) { - m_best_block_index = pindex; + InternalCommit(pindex); m_synced = true; - Commit(); break; } - if (pindex_next->pprev != pindex && !Rewind(pindex, pindex_next->pprev)) { + if (pindex_next->pprev != pindex && !InternalCommit(pindex_next->pprev, /* rewind= */ true)) { FatalError("%s: Failed to rewind index %s to a previous chain tip", __func__, GetName()); return; @@ -125,9 +123,8 @@ void BaseIndex::ThreadSync() } if (last_locator_write_time + SYNC_LOCATOR_WRITE_INTERVAL < current_time) { - m_best_block_index = pindex; + InternalCommit(pindex); last_locator_write_time = current_time; - Commit(); } CBlock block; @@ -151,35 +148,35 @@ void BaseIndex::ThreadSync() } } -bool BaseIndex::Commit() +bool BaseIndex::InternalCommit(const CBlockIndex* new_tip, bool rewind) { + const CBlockIndex* best_block = m_best_block_index.load(); CDBBatch batch(GetDB()); - if (!Commit(batch) || !GetDB().WriteBatch(batch)) { - return error("%s: Failed to commit latest %s state", __func__, GetName()); + if (new_tip) { + if (rewind) { + assert(best_block->GetAncestor(new_tip->nHeight) == new_tip); + if (!Rewind(batch, best_block, new_tip)) { + return error("%s: Failed to rewind %s tip", __func__, GetName()); + } + } + best_block = new_tip; } - return true; -} -bool BaseIndex::Commit(CDBBatch& batch) -{ - LOCK(cs_main); - GetDB().WriteBestBlock(batch, chainActive.GetLocator(m_best_block_index)); - return true; -} + if (!Commit(batch)) { + return error("%s: Failed to commit latest %s state", __func__, GetName()); + } -bool BaseIndex::Rewind(const CBlockIndex* current_tip, const CBlockIndex* new_tip) -{ - assert(current_tip == m_best_block_index); - assert(current_tip->GetAncestor(new_tip->nHeight) == new_tip); - - // In the case of a reorg, ensure persisted block locator is not stale. - m_best_block_index = new_tip; - if (!Commit()) { - // If commit fails, revert the best block index to avoid corruption. - m_best_block_index = current_tip; - return false; + CBlockLocator locator; + { + LOCK(cs_main); + locator = chainActive.GetLocator(best_block); } + GetDB().WriteBestBlock(batch, locator); + if (!GetDB().WriteBatch(batch)) { + return error("%s: Failed to flush latest %s state", __func__, GetName()); + } + m_best_block_index = best_block; return true; } @@ -210,7 +207,7 @@ void BaseIndex::BlockConnected(const std::shared_ptr& block, const best_block_index->GetBlockHash().ToString()); return; } - if (best_block_index != pindex->pprev && !Rewind(best_block_index, pindex->pprev)) { + if (best_block_index != pindex->pprev && !InternalCommit(pindex->pprev, /* rewind= */ true )) { FatalError("%s: Failed to rewind index %s to a previous chain tip", __func__, GetName()); return; @@ -259,7 +256,7 @@ void BaseIndex::ChainStateFlushed(const CBlockLocator& locator) return; } - Commit(); + InternalCommit(); } bool BaseIndex::BlockUntilSyncedToCurrentChain() diff --git a/src/index/base.h b/src/index/base.h index c9e23404a2f4cb..baad52e2cd1615 100644 --- a/src/index/base.h +++ b/src/index/base.h @@ -55,27 +55,26 @@ class BaseIndex : public CValidationInterface void ThreadSync(); /// Write the current chain block locator and other index state to the DB. - bool Commit(); + bool InternalCommit(const CBlockIndex* new_tip = nullptr, bool rewind = false); -protected: void BlockConnected(const std::shared_ptr& block, const CBlockIndex* pindex, const std::vector& txn_conflicted) override; void ChainStateFlushed(const CBlockLocator& locator) override; +protected: /// Initialize internal state from the database and block index. virtual bool Init(); /// Write update index entries for a newly connected block. virtual bool WriteBlock(const CBlock& block, const CBlockIndex* pindex) { return true; } - /// Virtual method called internally that can be overridden to atomically - /// commit more index state. - virtual bool Commit(CDBBatch& batch); - - /// Rewind index to an earlier chain tip during a chain reorg. The tip must + /// Rewind index to an earlier chain tip during a chain reorg. The tip will /// be an ancestor of the current best block. - virtual bool Rewind(const CBlockIndex* current_tip, const CBlockIndex* new_tip); + virtual bool Rewind(CDBBatch& batch, const CBlockIndex* current_tip, const CBlockIndex* new_tip) { return true; } + + /// Save additional state after new blocks are added or rewound. + virtual bool Commit(CDBBatch& batch) { return true; } virtual DB& GetDB() const = 0; diff --git a/src/index/blockfilterindex.cpp b/src/index/blockfilterindex.cpp index ea7b5a68975200..6aa865f74be634 100644 --- a/src/index/blockfilterindex.cpp +++ b/src/index/blockfilterindex.cpp @@ -133,7 +133,7 @@ bool BlockFilterIndex::Commit(CDBBatch& batch) } batch.Write(DB_FILTER_POS, pos); - return BaseIndex::Commit(batch); + return true; } bool BlockFilterIndex::ReadFilterFromDisk(const FlatFilePos& pos, BlockFilter& filter) const @@ -271,11 +271,10 @@ static bool CopyHeightIndexToHashIndex(CDBIterator& db_it, CDBBatch& batch, return true; } -bool BlockFilterIndex::Rewind(const CBlockIndex* current_tip, const CBlockIndex* new_tip) +bool BlockFilterIndex::Rewind(CDBBatch& batch, const CBlockIndex* current_tip, const CBlockIndex* new_tip) { assert(current_tip->GetAncestor(new_tip->nHeight) == new_tip); - CDBBatch batch(*m_db); std::unique_ptr db_it(m_db->NewIterator()); // During a reorg, we need to copy all filters for blocks that are getting disconnected from the @@ -285,13 +284,11 @@ bool BlockFilterIndex::Rewind(const CBlockIndex* current_tip, const CBlockIndex* return false; } - // The latest filter position gets written in Commit by the call to the BaseIndex::Rewind. - // But since this creates new references to the filter, the position should get updated here - // atomically as well in case Commit fails. + // The latest filter position also gets written later in BlockFilterIndex::Commit. + // But since rewinding creates new references to the filter, the position should get updated here + // as well in case Commit fails. batch.Write(DB_FILTER_POS, m_next_filter_pos); - if (!m_db->WriteBatch(batch)) return false; - - return BaseIndex::Rewind(current_tip, new_tip); + return true; } static bool LookupOne(const CDBWrapper& db, const CBlockIndex* block_index, DBVal& result) diff --git a/src/index/blockfilterindex.h b/src/index/blockfilterindex.h index 9112d2665db6f4..38e5ccb427a7a4 100644 --- a/src/index/blockfilterindex.h +++ b/src/index/blockfilterindex.h @@ -37,7 +37,7 @@ class BlockFilterIndex final : public BaseIndex bool WriteBlock(const CBlock& block, const CBlockIndex* pindex) override; - bool Rewind(const CBlockIndex* current_tip, const CBlockIndex* new_tip) override; + bool Rewind(CDBBatch& batch, const CBlockIndex* current_tip, const CBlockIndex* new_tip) override; BaseIndex::DB& GetDB() const override { return *m_db; }