From c0c3934ff0a96879fa4c47bf7265d51549638c8f Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kittywhiskers@users.noreply.github.com> Date: Thu, 18 Jun 2020 16:15:33 -0400 Subject: [PATCH] merge bitcoin#19325: Refactor BerkeleyDatabase to introduce DatabaseBatch abstract class --- src/wallet/bdb.cpp | 5 +++ src/wallet/bdb.h | 84 ++++++++++------------------------------- src/wallet/db.h | 80 +++++++++++++++++++++++++++++++++++++++ src/wallet/walletdb.cpp | 38 +++++++++---------- src/wallet/walletdb.h | 12 +++--- 5 files changed, 129 insertions(+), 90 deletions(-) diff --git a/src/wallet/bdb.cpp b/src/wallet/bdb.cpp index ca96b0c995..2c3c149ce1 100644 --- a/src/wallet/bdb.cpp +++ b/src/wallet/bdb.cpp @@ -849,3 +849,8 @@ bool BerkeleyBatch::HasKey(CDataStream&& key) int ret = pdb->exists(activeTxn, datKey, 0); return ret == 0; } + +std::unique_ptr BerkeleyDatabase::MakeBatch(const char* mode, bool flush_on_close) +{ + return MakeUnique(*this, mode, flush_on_close); +} diff --git a/src/wallet/bdb.h b/src/wallet/bdb.h index 87da9ac5a3..fe534f281f 100644 --- a/src/wallet/bdb.h +++ b/src/wallet/bdb.h @@ -91,6 +91,8 @@ std::shared_ptr GetWalletEnv(const fs::path& wallet_path, s /** Return whether a wallet database is currently loaded. */ bool IsBDBWalletLoaded(const fs::path& wallet_path); +class BerkeleyBatch; + /** An instance of this class represents one database. * For BerkeleyDB this is just a (env, strFile) tuple. **/ @@ -159,6 +161,9 @@ public: /** Database pointer. This is initialized lazily and reset during flushes, so it can be null. */ std::unique_ptr m_db; + /** Make a BerkeleyBatch connected to this database */ + std::unique_ptr MakeBatch(const char* mode, bool flush_on_close); + private: std::string strFile; @@ -170,7 +175,7 @@ private: }; /** RAII class that provides access to a Berkeley database */ -class BerkeleyBatch +class BerkeleyBatch : public DatabaseBatch { /** RAII class that automatically cleanses its data on destruction */ class SafeDbt final @@ -193,10 +198,10 @@ class BerkeleyBatch }; private: - bool ReadKey(CDataStream&& key, CDataStream& value); - bool WriteKey(CDataStream&& key, CDataStream&& value, bool overwrite = true); - bool EraseKey(CDataStream&& key); - bool HasKey(CDataStream&& key); + bool ReadKey(CDataStream&& key, CDataStream& value) override; + bool WriteKey(CDataStream&& key, CDataStream&& value, bool overwrite = true) override; + bool EraseKey(CDataStream&& key) override; + bool HasKey(CDataStream&& key) override; protected: Db* pdb; @@ -209,71 +214,20 @@ protected: public: explicit BerkeleyBatch(BerkeleyDatabase& database, const char* pszMode = "r+", bool fFlushOnCloseIn=true); - ~BerkeleyBatch() { Close(); } + ~BerkeleyBatch() override { Close(); } BerkeleyBatch(const BerkeleyBatch&) = delete; BerkeleyBatch& operator=(const BerkeleyBatch&) = delete; - void Flush(); - void Close(); + void Flush() override; + void Close() override; - template - bool Read(const K& key, T& value) - { - CDataStream ssKey(SER_DISK, CLIENT_VERSION); - ssKey.reserve(1000); - ssKey << key; - - CDataStream ssValue(SER_DISK, CLIENT_VERSION); - if (!ReadKey(std::move(ssKey), ssValue)) return false; - try { - ssValue >> value; - return true; - } catch (const std::exception&) { - return false; - } - } - - template - bool Write(const K& key, const T& value, bool fOverwrite = true) - { - CDataStream ssKey(SER_DISK, CLIENT_VERSION); - ssKey.reserve(1000); - ssKey << key; - - CDataStream ssValue(SER_DISK, CLIENT_VERSION); - ssValue.reserve(10000); - ssValue << value; - - return WriteKey(std::move(ssKey), std::move(ssValue), fOverwrite); - } - - template - bool Erase(const K& key) - { - CDataStream ssKey(SER_DISK, CLIENT_VERSION); - ssKey.reserve(1000); - ssKey << key; - - return EraseKey(std::move(ssKey)); - } - - template - bool Exists(const K& key) - { - CDataStream ssKey(SER_DISK, CLIENT_VERSION); - ssKey.reserve(1000); - ssKey << key; - - return HasKey(std::move(ssKey)); - } - - bool StartCursor(); - bool ReadAtCursor(CDataStream& ssKey, CDataStream& ssValue, bool& complete); - void CloseCursor(); - bool TxnBegin(); - bool TxnCommit(); - bool TxnAbort(); + bool StartCursor() override; + bool ReadAtCursor(CDataStream& ssKey, CDataStream& ssValue, bool& complete) override; + void CloseCursor() override; + bool TxnBegin() override; + bool TxnCommit() override; + bool TxnAbort() override; }; std::string BerkeleyDatabaseVersion(); diff --git a/src/wallet/db.h b/src/wallet/db.h index d633dc9b68..7f48781f45 100644 --- a/src/wallet/db.h +++ b/src/wallet/db.h @@ -6,7 +6,9 @@ #ifndef BITCOIN_WALLET_DB_H #define BITCOIN_WALLET_DB_H +#include #include +#include #include @@ -14,4 +16,82 @@ fs::path WalletDataFilePath(const fs::path& wallet_path); void SplitWalletPath(const fs::path& wallet_path, fs::path& env_directory, std::string& database_filename); +/** RAII class that provides access to a WalletDatabase */ +class DatabaseBatch +{ +private: + virtual bool ReadKey(CDataStream&& key, CDataStream& value) = 0; + virtual bool WriteKey(CDataStream&& key, CDataStream&& value, bool overwrite=true) = 0; + virtual bool EraseKey(CDataStream&& key) = 0; + virtual bool HasKey(CDataStream&& key) = 0; + +public: + explicit DatabaseBatch() {} + virtual ~DatabaseBatch() {} + + DatabaseBatch(const DatabaseBatch&) = delete; + DatabaseBatch& operator=(const DatabaseBatch&) = delete; + + virtual void Flush() = 0; + virtual void Close() = 0; + + template + bool Read(const K& key, T& value) + { + CDataStream ssKey(SER_DISK, CLIENT_VERSION); + ssKey.reserve(1000); + ssKey << key; + + CDataStream ssValue(SER_DISK, CLIENT_VERSION); + if (!ReadKey(std::move(ssKey), ssValue)) return false; + try { + ssValue >> value; + return true; + } catch (const std::exception&) { + return false; + } + } + + template + bool Write(const K& key, const T& value, bool fOverwrite = true) + { + CDataStream ssKey(SER_DISK, CLIENT_VERSION); + ssKey.reserve(1000); + ssKey << key; + + CDataStream ssValue(SER_DISK, CLIENT_VERSION); + ssValue.reserve(10000); + ssValue << value; + + return WriteKey(std::move(ssKey), std::move(ssValue), fOverwrite); + } + + template + bool Erase(const K& key) + { + CDataStream ssKey(SER_DISK, CLIENT_VERSION); + ssKey.reserve(1000); + ssKey << key; + + return EraseKey(std::move(ssKey)); + } + + template + bool Exists(const K& key) + { + CDataStream ssKey(SER_DISK, CLIENT_VERSION); + ssKey.reserve(1000); + ssKey << key; + + return HasKey(std::move(ssKey)); + } + + virtual bool StartCursor() = 0; + virtual bool ReadAtCursor(CDataStream& ssKey, CDataStream& ssValue, bool& complete) = 0; + virtual void CloseCursor() = 0; + virtual bool TxnBegin() = 0; + virtual bool TxnCommit() = 0; + virtual bool TxnAbort() = 0; +}; + #endif // BITCOIN_WALLET_DB_H diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 1d5cf3678b..16aee34868 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -158,8 +158,8 @@ bool WalletBatch::WriteBestBlock(const CBlockLocator& locator) bool WalletBatch::ReadBestBlock(CBlockLocator& locator) { - if (m_batch.Read(DBKeys::BESTBLOCK, locator) && !locator.vHave.empty()) return true; - return m_batch.Read(DBKeys::BESTBLOCK_NOMERKLE, locator); + if (m_batch->Read(DBKeys::BESTBLOCK, locator) && !locator.vHave.empty()) return true; + return m_batch->Read(DBKeys::BESTBLOCK_NOMERKLE, locator); } bool WalletBatch::WriteOrderPosNext(int64_t nOrderPosNext) @@ -169,7 +169,7 @@ bool WalletBatch::WriteOrderPosNext(int64_t nOrderPosNext) bool WalletBatch::ReadPool(int64_t nPool, CKeyPool& keypool) { - return m_batch.Read(std::make_pair(DBKeys::POOL, nPool), keypool); + return m_batch->Read(std::make_pair(DBKeys::POOL, nPool), keypool); } bool WalletBatch::WritePool(int64_t nPool, const CKeyPool& keypool) @@ -190,7 +190,7 @@ bool WalletBatch::WriteMinVersion(int nVersion) bool WalletBatch::ReadCoinJoinSalt(uint256& salt, bool fLegacy) { // TODO: Remove legacy checks after few major releases - return m_batch.Read(std::string(fLegacy ? DBKeys::PRIVATESEND_SALT : DBKeys::COINJOIN_SALT), salt); + return m_batch->Read(std::string(fLegacy ? DBKeys::PRIVATESEND_SALT : DBKeys::COINJOIN_SALT), salt); } bool WalletBatch::WriteCoinJoinSalt(const uint256& salt) @@ -514,14 +514,14 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet) LOCK(pwallet->cs_wallet); try { int nMinVersion = 0; - if (m_batch.Read(DBKeys::MINVERSION, nMinVersion)) { + if (m_batch->Read(DBKeys::MINVERSION, nMinVersion)) { if (nMinVersion > FEATURE_LATEST) return DBErrors::TOO_NEW; pwallet->LoadMinVersion(nMinVersion); } // Get cursor - if (!m_batch.StartCursor()) + if (!m_batch->StartCursor()) { pwallet->WalletLogPrintf("Error getting wallet database cursor\n"); return DBErrors::CORRUPT; @@ -533,13 +533,13 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet) CDataStream ssKey(SER_DISK, CLIENT_VERSION); CDataStream ssValue(SER_DISK, CLIENT_VERSION); bool complete; - bool ret = m_batch.ReadAtCursor(ssKey, ssValue, complete); + bool ret = m_batch->ReadAtCursor(ssKey, ssValue, complete); if (complete) { break; } else if (!ret) { - m_batch.CloseCursor(); + m_batch->CloseCursor(); pwallet->WalletLogPrintf("Error reading next record from wallet database\n"); return DBErrors::CORRUPT; } @@ -573,7 +573,7 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet) } catch (...) { result = DBErrors::CORRUPT; } - m_batch.CloseCursor(); + m_batch->CloseCursor(); if (fNoncriticalErrors && result == DBErrors::LOAD_OK) result = DBErrors::NONCRITICAL_ERROR; @@ -585,7 +585,7 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet) // Last client version to open this wallet, was previously the file version number int last_client = CLIENT_VERSION; - m_batch.Read(DBKeys::VERSION, last_client); + m_batch->Read(DBKeys::VERSION, last_client); int wallet_version = pwallet->GetVersion(); pwallet->WalletLogPrintf("Wallet File Version = %d\n", wallet_version > 0 ? wallet_version : last_client); @@ -606,7 +606,7 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet) return DBErrors::NEED_REWRITE; if (last_client < CLIENT_VERSION) // Update - m_batch.Write(DBKeys::VERSION, CLIENT_VERSION); + m_batch->Write(DBKeys::VERSION, CLIENT_VERSION); if (wss.fAnyUnordered) result = pwallet->ReorderTransactions(); @@ -628,13 +628,13 @@ DBErrors WalletBatch::FindWalletTx(std::vector& vTxHash, std::vectorRead(DBKeys::MINVERSION, nMinVersion)) { if (nMinVersion > FEATURE_LATEST) return DBErrors::TOO_NEW; } // Get cursor - if (!m_batch.StartCursor()) + if (!m_batch->StartCursor()) { LogPrintf("Error getting wallet database cursor\n"); return DBErrors::CORRUPT; @@ -646,11 +646,11 @@ DBErrors WalletBatch::FindWalletTx(std::vector& vTxHash, std::vectorReadAtCursor(ssKey, ssValue, complete); if (complete) { break; } else if (!ret) { - m_batch.CloseCursor(); + m_batch->CloseCursor(); LogPrintf("Error reading next record from wallet database\n"); return DBErrors::CORRUPT; } @@ -671,7 +671,7 @@ DBErrors WalletBatch::FindWalletTx(std::vector& vTxHash, std::vectorCloseCursor(); return result; } @@ -801,17 +801,17 @@ bool WalletBatch::WriteWalletFlags(const uint64_t flags) bool WalletBatch::TxnBegin() { - return m_batch.TxnBegin(); + return m_batch->TxnBegin(); } bool WalletBatch::TxnCommit() { - return m_batch.TxnCommit(); + return m_batch->TxnCommit(); } bool WalletBatch::TxnAbort() { - return m_batch.TxnAbort(); + return m_batch->TxnAbort(); } bool IsWalletLoaded(const fs::path& wallet_path) diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h index 5d343539d6..918559f335 100644 --- a/src/wallet/walletdb.h +++ b/src/wallet/walletdb.h @@ -137,12 +137,12 @@ private: template bool WriteIC(const K& key, const T& value, bool fOverwrite = true) { - if (!m_batch.Write(key, value, fOverwrite)) { + if (!m_batch->Write(key, value, fOverwrite)) { return false; } m_database.IncrementUpdateCounter(); if (m_database.nUpdateCounter % 1000 == 0) { - m_batch.Flush(); + m_batch->Flush(); } return true; } @@ -150,19 +150,19 @@ private: template bool EraseIC(const K& key) { - if (!m_batch.Erase(key)) { + if (!m_batch->Erase(key)) { return false; } m_database.IncrementUpdateCounter(); if (m_database.nUpdateCounter % 1000 == 0) { - m_batch.Flush(); + m_batch->Flush(); } return true; } public: explicit WalletBatch(WalletDatabase& database, const char* pszMode = "r+", bool _fFlushOnClose = true) : - m_batch(database, pszMode, _fFlushOnClose), + m_batch(database.MakeBatch(pszMode, _fFlushOnClose)), m_database(database) { } @@ -230,7 +230,7 @@ public: //! Abort current transaction bool TxnAbort(); private: - BerkeleyBatch m_batch; + std::unique_ptr m_batch; WalletDatabase& m_database; };