From 12c3f382ccfc07db3684a1560a8206ba92d99385 Mon Sep 17 00:00:00 2001 From: jamescowens Date: Thu, 9 Apr 2020 13:24:53 -0400 Subject: [PATCH] Optimize bdb to avoid synchronous flush of database during AddToWalletIfInvolvingMe() calls. --- src/db.cpp | 37 +++++++++++++++++--------- src/db.h | 4 ++- src/test/accounting_tests.cpp | 6 ++--- src/wallet.cpp | 50 ++++++++++++++++++++++------------- src/wallet.h | 6 ++--- src/walletdb.cpp | 4 +-- src/walletdb.h | 2 +- 7 files changed, 68 insertions(+), 41 deletions(-) diff --git a/src/db.cpp b/src/db.cpp index 1653099a2c..6e428f08b0 100644 --- a/src/db.cpp +++ b/src/db.cpp @@ -233,14 +233,16 @@ void CDBEnv::lsn_reset(const std::string& strFile) dbenv.lsn_reset(strFile.c_str(),0); } -CDB::CDB(const char *pszFile, const char* pszMode) : +CDB::CDB(const std::string& strFilename, const char* pszMode, bool fFlushOnCloseIn) : pdb(NULL), activeTxn(NULL) { int ret; - if (pszFile == NULL) - return; + + if (strFilename.empty()) + return; fReadOnly = (!strchr(pszMode, '+') && !strchr(pszMode, 'w')); + fFlushOnClose = fFlushOnCloseIn; bool fCreate = strchr(pszMode, 'c'); unsigned int nFlags = DB_THREAD; if (fCreate) @@ -251,7 +253,7 @@ CDB::CDB(const char *pszFile, const char* pszMode) : if (!bitdb.Open(GetDataDir())) throw runtime_error("env open failed"); - strFile = pszFile; + strFile = strFilename; ++bitdb.mapFileUseCount[strFile]; pdb = bitdb.mapDb[strFile]; if (pdb == NULL) @@ -264,11 +266,11 @@ CDB::CDB(const char *pszFile, const char* pszMode) : DbMpoolFile*mpf = pdb->get_mpf(); ret = mpf->set_flags(DB_MPOOL_NOFILE, 1); if (ret != 0) - throw runtime_error(strprintf("CDB() : failed to configure for no temp file backing for database %s", pszFile)); + throw runtime_error(strprintf("CDB() : failed to configure for no temp file backing for database %s", strFile)); } ret = pdb->open(NULL, // Txn pointer - fMockDb ? NULL : pszFile, // Filename + fMockDb ? NULL : strFile.c_str(), // Filename "main", // Logical db name DB_BTREE, // Database type nFlags, // Flags @@ -280,7 +282,7 @@ CDB::CDB(const char *pszFile, const char* pszMode) : pdb = NULL; --bitdb.mapFileUseCount[strFile]; strFile = ""; - throw runtime_error(strprintf("CDB() : can't open database file %s, error %d", pszFile, ret)); + throw runtime_error(strprintf("CDB() : can't open database file %s, error %d", strFile, ret)); } if (fCreate && !Exists(string("version"))) @@ -296,6 +298,19 @@ CDB::CDB(const char *pszFile, const char* pszMode) : } } +void CDB::Flush() +{ + if (activeTxn) + return; + + // Flush database activity from memory pool to disk log + unsigned int nMinutes = 0; + if (fReadOnly) + nMinutes = 1; + + bitdb.dbenv.txn_checkpoint(nMinutes ? GetArg("-dblogsize", 100) * 1024 : 0, nMinutes, 0); +} + void CDB::Close() { if (!pdb) @@ -305,12 +320,8 @@ void CDB::Close() activeTxn = NULL; pdb = NULL; - // Flush database activity from memory pool to disk log - unsigned int nMinutes = 0; - if (fReadOnly) - nMinutes = 1; - - bitdb.dbenv.txn_checkpoint(nMinutes ? GetArg("-dblogsize", 100)*1024 : 0, nMinutes, 0); + if (fFlushOnClose) + Flush(); { LOCK(bitdb.cs_db); diff --git a/src/db.h b/src/db.h index 6de7079993..bdaff79c05 100644 --- a/src/db.h +++ b/src/db.h @@ -99,10 +99,12 @@ class CDB std::string strFile; DbTxn *activeTxn; bool fReadOnly; + bool fFlushOnClose; - explicit CDB(const char* pszFile, const char* pszMode="r+"); + explicit CDB(const std::string &strFilename, const char* pszMode="r+", bool fFlushOnCloseIn = true); ~CDB() { Close(); } public: + void Flush(); void Close(); private: CDB(const CDB&); diff --git a/src/test/accounting_tests.cpp b/src/test/accounting_tests.cpp index 0417b59670..6b227288d9 100755 --- a/src/test/accounting_tests.cpp +++ b/src/test/accounting_tests.cpp @@ -40,7 +40,7 @@ BOOST_AUTO_TEST_CASE(acc_orderupgrade) walletdb.WriteAccountingEntry(ae); wtx.mapValue["comment"] = "z"; - pwalletMain->AddToWallet(wtx); + pwalletMain->AddToWallet(wtx, &walletdb); vpwtx.push_back(&pwalletMain->mapWallet[wtx.GetHash()]); vpwtx[0]->nTimeReceived = (unsigned int)1333333335; vpwtx[0]->nOrderPos = -1; @@ -78,13 +78,13 @@ BOOST_AUTO_TEST_CASE(acc_orderupgrade) wtx.mapValue["comment"] = "y"; --wtx.nLockTime; // Just to change the hash :) - pwalletMain->AddToWallet(wtx); + pwalletMain->AddToWallet(wtx, &walletdb); vpwtx.push_back(&pwalletMain->mapWallet[wtx.GetHash()]); vpwtx[1]->nTimeReceived = (unsigned int)1333333336; wtx.mapValue["comment"] = "x"; --wtx.nLockTime; // Just to change the hash :) - pwalletMain->AddToWallet(wtx); + pwalletMain->AddToWallet(wtx, &walletdb); vpwtx.push_back(&pwalletMain->mapWallet[wtx.GetHash()]); vpwtx[2]->nTimeReceived = (unsigned int)1333333329; vpwtx[2]->nOrderPos = -1; diff --git a/src/wallet.cpp b/src/wallet.cpp index 9e3987509c..c58beb08dd 100644 --- a/src/wallet.cpp +++ b/src/wallet.cpp @@ -383,7 +383,7 @@ CWallet::TxItems CWallet::OrderedTxItems(std::list& acentries, return txOrdered; } -void CWallet::WalletUpdateSpent(const CTransaction &tx, bool fBlock) +void CWallet::WalletUpdateSpent(const CTransaction &tx, bool fBlock, CWalletDB* pwalletdb) { // Anytime a signature is successfully verified, it's proof the outpoint is spent. // Update the wallet spent flag if it doesn't know due to wallet.dat being @@ -402,7 +402,7 @@ void CWallet::WalletUpdateSpent(const CTransaction &tx, bool fBlock) { if (fDebug) LogPrintf("WalletUpdateSpent found spent coin %s gC %s", FormatMoney(wtx.GetCredit()), wtx.GetHash().ToString()); wtx.MarkSpent(txin.prevout.n); - wtx.WriteToDisk(); + wtx.WriteToDisk(pwalletdb); NotifyTransactionChanged(this, txin.prevout.hash, CT_UPDATED); } } @@ -419,7 +419,7 @@ void CWallet::WalletUpdateSpent(const CTransaction &tx, bool fBlock) if (IsMine(txout)) { wtx.MarkUnspent(&txout - &tx.vout[0]); - wtx.WriteToDisk(); + wtx.WriteToDisk(pwalletdb); NotifyTransactionChanged(this, hash, CT_UPDATED); } } @@ -437,7 +437,7 @@ void CWallet::MarkDirty() } } -bool CWallet::AddToWallet(const CWalletTx& wtxIn) +bool CWallet::AddToWallet(const CWalletTx& wtxIn, CWalletDB* pwalletdb) { uint256 hash = wtxIn.GetHash(); { @@ -450,7 +450,7 @@ bool CWallet::AddToWallet(const CWalletTx& wtxIn) if (fInsertedNew) { wtx.nTimeReceived = GetAdjustedTime(); - wtx.nOrderPos = IncOrderPosNext(); + wtx.nOrderPos = IncOrderPosNext(pwalletdb); wtx.nTimeSmart = wtx.nTimeReceived; if (!wtxIn.hashBlock.IsNull()) @@ -527,7 +527,7 @@ bool CWallet::AddToWallet(const CWalletTx& wtxIn) // Write to disk if (fInsertedNew || fUpdated) - if (!wtx.WriteToDisk()) + if (!wtx.WriteToDisk(pwalletdb)) return false; if(!fQtActive) { @@ -550,7 +550,7 @@ bool CWallet::AddToWallet(const CWalletTx& wtxIn) } } // since AddToWallet is called directly for self-originating transactions, check for consumption of own coins - WalletUpdateSpent(wtx, (!wtxIn.hashBlock.IsNull())); + WalletUpdateSpent(wtx, (!wtxIn.hashBlock.IsNull()), pwalletdb); // Notify UI of new or updated transaction NotifyTransactionChanged(this, hash, fInsertedNew ? CT_NEW : CT_UPDATED); @@ -577,16 +577,22 @@ bool CWallet::AddToWalletIfInvolvingMe(const CTransaction& tx, const CBlock* pbl LOCK(cs_wallet); bool fExisted = mapWallet.count(hash); if (fExisted && !fUpdate) return false; + + // Do not flush the wallet here for performance reasons + // this is safe, as in case of a crash, we rescan the necessary blocks on startup. + CWalletDB walletdb(strWalletFile, "r+", false); + if (fExisted || IsMine(tx) || IsFromMe(tx)) { CWalletTx wtx(this,tx); // Get merkle branch if transaction was found in a block if (pblock) wtx.SetMerkleBranch(pblock); - return AddToWallet(wtx); + + return AddToWallet(wtx, &walletdb); } else - WalletUpdateSpent(tx); + WalletUpdateSpent(tx, false, &walletdb); } return false; } @@ -965,9 +971,9 @@ void CWalletTx::AddSupportingTransactions(CTxDB& txdb) reverse(vtxPrev.begin(), vtxPrev.end()); } -bool CWalletTx::WriteToDisk() +bool CWalletTx::WriteToDisk(CWalletDB *pwalletdb) { - return CWalletDB(pwallet->strWalletFile).WriteTx(GetHash(), *this); + return pwalletdb->WriteTx(GetHash(), *this); } // Scan the block chain (starting in pindexStart) for transactions @@ -1042,7 +1048,10 @@ void CWallet::ReacceptWalletTransactions() { LogPrintf("ReacceptWalletTransactions found spent coin %s gC %s", FormatMoney(wtx.GetCredit()), wtx.GetHash().ToString()); wtx.MarkDirty(); - wtx.WriteToDisk(); + + CWalletDB walletdb(strWalletFile); + + wtx.WriteToDisk(&walletdb); } } else @@ -1829,14 +1838,14 @@ bool CWallet::CommitTransaction(CWalletTx& wtxNew, CReserveKey& reservekey) // This is only to keep the database open to defeat the auto-flush for the // duration of this scope. This is the only place where this optimization // maybe makes sense; please don't do it anywhere else. - CWalletDB* pwalletdb = fFileBacked ? new CWalletDB(strWalletFile,"r") : NULL; + CWalletDB* pwalletdb = fFileBacked ? new CWalletDB(strWalletFile,"r+") : NULL; // Take key pair from key pool so it won't be used again reservekey.KeepKey(); // Add tx to wallet, because if it has change it's also ours, // otherwise just for transaction history. - AddToWallet(wtxNew); + AddToWallet(wtxNew, pwalletdb); // Mark old coins as spent set setCoins; @@ -1845,7 +1854,7 @@ bool CWallet::CommitTransaction(CWalletTx& wtxNew, CReserveKey& reservekey) CWalletTx &coin = mapWallet[txin.prevout.hash]; coin.BindWallet(this); coin.MarkSpent(txin.prevout.n); - coin.WriteToDisk(); + coin.WriteToDisk(pwalletdb); NotifyTransactionChanged(this, coin.GetHash(), CT_UPDATED); } @@ -2380,6 +2389,8 @@ void CWallet::FixSpentCoins(int& nMismatchFound, int64_t& nBalanceInQuestion, bo for (map::iterator it = mapWallet.begin(); it != mapWallet.end(); ++it) vCoins.push_back(&(*it).second); + CWalletDB walletdb(strWalletFile); + CTxDB txdb("r"); for (auto const& pcoin : vCoins) { @@ -2398,7 +2409,7 @@ void CWallet::FixSpentCoins(int& nMismatchFound, int64_t& nBalanceInQuestion, bo if (!fCheckOnly) { pcoin->MarkUnspent(n); - pcoin->WriteToDisk(); + pcoin->WriteToDisk(&walletdb); } } else if (IsMine(pcoin->vout[n]) && !pcoin->IsSpent(n) && (txindex.vSpent.size() > n && !txindex.vSpent[n].IsNull())) @@ -2410,7 +2421,7 @@ void CWallet::FixSpentCoins(int& nMismatchFound, int64_t& nBalanceInQuestion, bo if (!fCheckOnly) { pcoin->MarkSpent(n); - pcoin->WriteToDisk(); + pcoin->WriteToDisk(&walletdb); } } } @@ -2424,6 +2435,9 @@ void CWallet::DisableTransaction(const CTransaction &tx) return; // only disconnecting coinstake requires marking input unspent LOCK(cs_wallet); + + CWalletDB walletdb(strWalletFile); + for (auto const& txin : tx.vin) { map::iterator mi = mapWallet.find(txin.prevout.hash); @@ -2433,7 +2447,7 @@ void CWallet::DisableTransaction(const CTransaction &tx) if (txin.prevout.n < prev.vout.size() && IsMine(prev.vout[txin.prevout.n])) { prev.MarkUnspent(txin.prevout.n); - prev.WriteToDisk(); + prev.WriteToDisk(&walletdb); } } } diff --git a/src/wallet.h b/src/wallet.h index ded518efb1..666f892cab 100644 --- a/src/wallet.h +++ b/src/wallet.h @@ -247,10 +247,10 @@ class CWallet : public CCryptoKeyStore TxItems OrderedTxItems(std::list& acentries, std::string strAccount = ""); void MarkDirty(); - bool AddToWallet(const CWalletTx& wtxIn); + bool AddToWallet(const CWalletTx& wtxIn, CWalletDB *pwalletdb); bool AddToWalletIfInvolvingMe(const CTransaction& tx, const CBlock* pblock, bool fUpdate = false, bool fFindBlock = false); bool EraseFromWallet(uint256 hash); - void WalletUpdateSpent(const CTransaction& prevout, bool fBlock = false); + void WalletUpdateSpent(const CTransaction &tx, bool fBlock, CWalletDB* pwalletdb); int ScanForWalletTransactions(CBlockIndex* pindexStart, bool fUpdate = false); void ReacceptWalletTransactions(); void ResendWalletTransactions(bool fForce = false); @@ -823,7 +823,7 @@ class CWalletTx : public CMerkleTx return true; } - bool WriteToDisk(); + bool WriteToDisk(CWalletDB *pwalletdb); int64_t GetTxTime() const; int GetRequestCount() const; diff --git a/src/walletdb.cpp b/src/walletdb.cpp index 938c39855c..33c6509ea9 100644 --- a/src/walletdb.cpp +++ b/src/walletdb.cpp @@ -654,7 +654,7 @@ void ThreadFlushWalletDB(void* parg) map::iterator mi = bitdb.mapFileUseCount.find(strFile); if (mi != bitdb.mapFileUseCount.end()) { - if (fDebug10) LogPrintf("Flushing wallet.dat"); + LogPrint(BCLog::LogFlags::WALLETDB, "Flushing wallet.dat"); nLastFlushed = nWalletDBUpdated; int64_t nStart = GetTimeMillis(); @@ -666,7 +666,7 @@ void ThreadFlushWalletDB(void* parg) // This is handled in CDB::Flush, which has a while loop that also does in the right place what // the intention of the below line was. // bitdb.mapFileUseCount.erase(mi++); - if (fDebug10) LogPrintf("Flushed wallet.dat %" PRId64 "ms", GetTimeMillis() - nStart); + LogPrint(BCLog::LogFlags::WALLETDB, "Flushed wallet.dat %" PRId64 "ms", GetTimeMillis() - nStart); } } } diff --git a/src/walletdb.h b/src/walletdb.h index 2d7d2f17dd..afa1994bf0 100644 --- a/src/walletdb.h +++ b/src/walletdb.h @@ -61,7 +61,7 @@ class CKeyMetadata class CWalletDB : public CDB { public: - CWalletDB(std::string strFilename, const char* pszMode="r+") : CDB(strFilename.c_str(), pszMode) + CWalletDB(const std::string& strFilename, const char* pszMode = "r+", bool fFlushOnClose = true) : CDB(strFilename, pszMode, fFlushOnClose) { } private: