Skip to content

Commit

Permalink
Merge #13106: Simplify semantics of ChainStateFlushed callback
Browse files Browse the repository at this point in the history 
9cb6cdc Simplify semantics of ChainStateFlushed callback (Matt Corallo)
50b6533 scripted-diff: Rename SetBestChain callback ChainStateFlushed (Matt Corallo)

Pull request description:

  Previously, ChainStateFlushed would fire either if a full flush
  completed (which can happen due to memory limits, forced flush, or
  on its own DATABASE_WRITE_INTERVAL timer) *or* on a
  ChainStateFlushed-specific DATABASE_WRITE_INTERVAL timer. This is
  both less clear for clients (as there are no guarantees about a
  flush having actually happened prior to the call), and reults in
  extra flushes not clearly intended by the code. We drop the second
  case, providing a strong guarantee without removing the periodit
  timer-based flushing.

  This is a follow-up to discussion in #11857.

Tree-SHA512: 22ba3a0954d265d28413dbf87040790ca5b439820ee7bbadab14028295ec190de82ce5cd664426c82e58b706dc84278868026fa8d066702eb6e6962c9ace1f8e
  • Loading branch information
@laanwj
laanwj committed May 2, 2018
2 parents 0bc980b + 9cb6cdc commit 598db38
Show file tree
Hide file tree
Showing 7 changed files with 33 additions and 26 deletions.
4 changes: 2 additions & 2 deletions src/index/txindex.cpp
View file
Expand Up @@ -192,7 +192,7 @@ void TxIndex::BlockConnected(const std::shared_ptr<const CBlock>& block, const C
}
}

void TxIndex::SetBestChain(const CBlockLocator& locator)
void TxIndex::ChainStateFlushed(const CBlockLocator& locator)
{
if (!m_synced) {
return;
Expand All @@ -211,7 +211,7 @@ void TxIndex::SetBestChain(const CBlockLocator& locator)
return;
}

// This checks that SetBestChain callbacks are received after BlockConnected. The check may fail
// This checks that ChainStateFlushed callbacks are received after BlockConnected. The check may fail
// immediately after the the sync thread catches up and sets m_synced. Consider the case where
// there is a reorg and the blocks on the stale branch are in the ValidationInterface queue
// backlog even after the sync thread has caught up to the new chain tip. In this unlikely
Expand Down
2 changes: 1 addition & 1 deletion src/index/txindex.h
View file
Expand Up @@ -55,7 +55,7 @@ class TxIndex final : public CValidationInterface
void BlockConnected(const std::shared_ptr<const CBlock>& block, const CBlockIndex* pindex,
const std::vector<CTransactionRef>& txn_conflicted) override;

void SetBestChain(const CBlockLocator& locator) override;
void ChainStateFlushed(const CBlockLocator& locator) override;

public:
/// Constructs the TxIndex, which becomes available to be queried.
Expand Down
18 changes: 7 additions & 11 deletions src/validation.cpp
View file
Expand Up @@ -2066,13 +2066,12 @@ bool static FlushStateToDisk(const CChainParams& chainparams, CValidationState &
LOCK(cs_main);
static int64_t nLastWrite = 0;
static int64_t nLastFlush = 0;
static int64_t nLastSetChain = 0;
std::set<int> setFilesToPrune;
bool fFlushForPrune = false;
bool fDoFullFlush = false;
int64_t nNow = 0;
bool full_flush_completed = false;
try {
{
bool fFlushForPrune = false;
bool fDoFullFlush = false;
LOCK(cs_LastBlockFile);
if (fPruneMode && (fCheckForPruning || nManualPruneHeight > 0) && !fReindex) {
if (nManualPruneHeight > 0) {
Expand All @@ -2089,17 +2088,14 @@ bool static FlushStateToDisk(const CChainParams& chainparams, CValidationState &
}
}
}
nNow = GetTimeMicros();
int64_t nNow = GetTimeMicros();
// Avoid writing/flushing immediately after startup.
if (nLastWrite == 0) {
nLastWrite = nNow;
}
if (nLastFlush == 0) {
nLastFlush = nNow;
}
if (nLastSetChain == 0) {
nLastSetChain = nNow;
}
int64_t nMempoolSizeMax = gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000;
int64_t cacheSize = pcoinsTip->DynamicMemoryUsage();
int64_t nTotalSpace = nCoinCacheUsage + std::max<int64_t>(nMempoolSizeMax - nMempoolUsage, 0);
Expand Down Expand Up @@ -2156,12 +2152,12 @@ bool static FlushStateToDisk(const CChainParams& chainparams, CValidationState &
if (!pcoinsTip->Flush())
return AbortNode(state, "Failed to write to coin database");
nLastFlush = nNow;
full_flush_completed = true;
}
}
if (fDoFullFlush || ((mode == FlushStateMode::ALWAYS || mode == FlushStateMode::PERIODIC) && nNow > nLastSetChain + (int64_t)DATABASE_WRITE_INTERVAL * 1000000)) {
if (full_flush_completed) {
// Update best block in wallet (so we can detect restored wallets).
GetMainSignals().SetBestChain(chainActive.GetLocator());
nLastSetChain = nNow;
GetMainSignals().ChainStateFlushed(chainActive.GetLocator());
}
} catch (const std::runtime_error& e) {
return AbortNode(state, std::string("System error while flushing: ") + e.what());
Expand Down
12 changes: 6 additions & 6 deletions src/validationinterface.cpp
View file
Expand Up @@ -25,7 +25,7 @@ struct MainSignalsInstance {
boost::signals2::signal<void (const std::shared_ptr<const CBlock> &, const CBlockIndex *pindex, const std::vector<CTransactionRef>&)> BlockConnected;
boost::signals2::signal<void (const std::shared_ptr<const CBlock> &)> BlockDisconnected;
boost::signals2::signal<void (const CTransactionRef &)> TransactionRemovedFromMempool;
boost::signals2::signal<void (const CBlockLocator &)> SetBestChain;
boost::signals2::signal<void (const CBlockLocator &)> ChainStateFlushed;
boost::signals2::signal<void (const uint256 &)> Inventory;
boost::signals2::signal<void (int64_t nBestBlockTime, CConnman* connman)> Broadcast;
boost::signals2::signal<void (const CBlock&, const CValidationState&)> BlockChecked;
Expand Down Expand Up @@ -80,7 +80,7 @@ void RegisterValidationInterface(CValidationInterface* pwalletIn) {
g_signals.m_internals->BlockConnected.connect(boost::bind(&CValidationInterface::BlockConnected, pwalletIn, _1, _2, _3));
g_signals.m_internals->BlockDisconnected.connect(boost::bind(&CValidationInterface::BlockDisconnected, pwalletIn, _1));
g_signals.m_internals->TransactionRemovedFromMempool.connect(boost::bind(&CValidationInterface::TransactionRemovedFromMempool, pwalletIn, _1));
g_signals.m_internals->SetBestChain.connect(boost::bind(&CValidationInterface::SetBestChain, pwalletIn, _1));
g_signals.m_internals->ChainStateFlushed.connect(boost::bind(&CValidationInterface::ChainStateFlushed, pwalletIn, _1));
g_signals.m_internals->Inventory.connect(boost::bind(&CValidationInterface::Inventory, pwalletIn, _1));
g_signals.m_internals->Broadcast.connect(boost::bind(&CValidationInterface::ResendWalletTransactions, pwalletIn, _1, _2));
g_signals.m_internals->BlockChecked.connect(boost::bind(&CValidationInterface::BlockChecked, pwalletIn, _1, _2));
Expand All @@ -91,7 +91,7 @@ void UnregisterValidationInterface(CValidationInterface* pwalletIn) {
g_signals.m_internals->BlockChecked.disconnect(boost::bind(&CValidationInterface::BlockChecked, pwalletIn, _1, _2));
g_signals.m_internals->Broadcast.disconnect(boost::bind(&CValidationInterface::ResendWalletTransactions, pwalletIn, _1, _2));
g_signals.m_internals->Inventory.disconnect(boost::bind(&CValidationInterface::Inventory, pwalletIn, _1));
g_signals.m_internals->SetBestChain.disconnect(boost::bind(&CValidationInterface::SetBestChain, pwalletIn, _1));
g_signals.m_internals->ChainStateFlushed.disconnect(boost::bind(&CValidationInterface::ChainStateFlushed, pwalletIn, _1));
g_signals.m_internals->TransactionAddedToMempool.disconnect(boost::bind(&CValidationInterface::TransactionAddedToMempool, pwalletIn, _1));
g_signals.m_internals->BlockConnected.disconnect(boost::bind(&CValidationInterface::BlockConnected, pwalletIn, _1, _2, _3));
g_signals.m_internals->BlockDisconnected.disconnect(boost::bind(&CValidationInterface::BlockDisconnected, pwalletIn, _1));
Expand All @@ -107,7 +107,7 @@ void UnregisterAllValidationInterfaces() {
g_signals.m_internals->BlockChecked.disconnect_all_slots();
g_signals.m_internals->Broadcast.disconnect_all_slots();
g_signals.m_internals->Inventory.disconnect_all_slots();
g_signals.m_internals->SetBestChain.disconnect_all_slots();
g_signals.m_internals->ChainStateFlushed.disconnect_all_slots();
g_signals.m_internals->TransactionAddedToMempool.disconnect_all_slots();
g_signals.m_internals->BlockConnected.disconnect_all_slots();
g_signals.m_internals->BlockDisconnected.disconnect_all_slots();
Expand Down Expand Up @@ -166,9 +166,9 @@ void CMainSignals::BlockDisconnected(const std::shared_ptr<const CBlock> &pblock
});
}

void CMainSignals::SetBestChain(const CBlockLocator &locator) {
void CMainSignals::ChainStateFlushed(const CBlockLocator &locator) {
m_internals->m_schedulerClient.AddToProcessQueue([locator, this] {
m_internals->SetBestChain(locator);
m_internals->ChainStateFlushed(locator);
});
}

Expand Down
15 changes: 13 additions & 2 deletions src/validationinterface.h
View file
Expand Up @@ -99,9 +99,20 @@ class CValidationInterface {
/**
* Notifies listeners of the new active block chain on-disk.
*
* Prior to this callback, any updates are not guaranteed to persist on disk
* (ie clients need to handle shutdown/restart safety by being able to
* understand when some updates were lost due to unclean shutdown).
*
* When this callback is invoked, the validation changes done by any prior
* callback are guaranteed to exist on disk and survive a restart, including
* an unclean shutdown.
*
* Provides a locator describing the best chain, which is likely useful for
* storing current state on disk in client DBs.
*
* Called on a background thread.
*/
virtual void SetBestChain(const CBlockLocator &locator) {}
virtual void ChainStateFlushed(const CBlockLocator &locator) {}
/**
* Notifies listeners about an inventory item being seen on the network.
*
Expand Down Expand Up @@ -157,7 +168,7 @@ class CMainSignals {
void TransactionAddedToMempool(const CTransactionRef &);
void BlockConnected(const std::shared_ptr<const CBlock> &, const CBlockIndex *pindex, const std::shared_ptr<const std::vector<CTransactionRef>> &);
void BlockDisconnected(const std::shared_ptr<const CBlock> &);
void SetBestChain(const CBlockLocator &);
void ChainStateFlushed(const CBlockLocator &);
void Inventory(const uint256 &);
void Broadcast(int64_t nBestBlockTime, CConnman* connman);
void BlockChecked(const CBlock&, const CValidationState&);
Expand Down
6 changes: 3 additions & 3 deletions src/wallet/wallet.cpp
View file
Expand Up @@ -453,7 +453,7 @@ bool CWallet::ChangeWalletPassphrase(const SecureString& strOldWalletPassphrase,
return false;
}

void CWallet::SetBestChain(const CBlockLocator& loc)
void CWallet::ChainStateFlushed(const CBlockLocator& loc)
{
WalletBatch batch(*database);
batch.WriteBestBlock(loc);
Expand Down Expand Up @@ -4038,7 +4038,7 @@ CWallet* CWallet::CreateWalletFromFile(const std::string& name, const fs::path&
return nullptr;
}

walletInstance->SetBestChain(chainActive.GetLocator());
walletInstance->ChainStateFlushed(chainActive.GetLocator());
} else if (gArgs.IsArgSet("-usehd")) {
bool useHD = gArgs.GetBoolArg("-usehd", true);
if (walletInstance->IsHDEnabled() && !useHD) {
Expand Down Expand Up @@ -4180,7 +4180,7 @@ CWallet* CWallet::CreateWalletFromFile(const std::string& name, const fs::path&
walletInstance->ScanForWalletTransactions(pindexRescan, nullptr, reserver, true);
}
LogPrintf(" rescan %15dms\n", GetTimeMillis() - nStart);
walletInstance->SetBestChain(chainActive.GetLocator());
walletInstance->ChainStateFlushed(chainActive.GetLocator());
walletInstance->database->IncrementUpdateCounter();

// Restore wallet transaction metadata after -zapwallettxes=1
Expand Down
2 changes: 1 addition & 1 deletion src/wallet/wallet.h
View file
Expand Up @@ -1013,7 +1013,7 @@ class CWallet final : public CCryptoKeyStore, public CValidationInterface
bool IsAllFromMe(const CTransaction& tx, const isminefilter& filter) const;
CAmount GetCredit(const CTransaction& tx, const isminefilter& filter) const;
CAmount GetChange(const CTransaction& tx) const;
void SetBestChain(const CBlockLocator& loc) override;
void ChainStateFlushed(const CBlockLocator& loc) override;

DBErrors LoadWallet(bool& fFirstRunRet);
DBErrors ZapWalletTx(std::vector<CWalletTx>& vWtx);
Expand Down

0 comments on commit 598db38

Please sign in to comment.