Skip to content

Commit fa1ac28

Browse files
laanwjlaanwj
committed
Merge #9951: Wallet database handling abstractions/simplifications
911a480 wallet: Add comment describing the various classes in walletdb.h (Wladimir J. van der Laan) 69d2e9b wallet: Make IsDummy private in CWalletDBWrapper (Wladimir J. van der Laan) 3323281 wallet: CWalletDB CDB composition not inheritance (Wladimir J. van der Laan) be9e1a9 wallet: Reduce references to global bitdb environment (Wladimir J. van der Laan) 071c955 wallet: Get rid of fFileBacked (Wladimir J. van der Laan) 71afe3c wallet: Introduce database handle wrapper (Wladimir J. van der Laan) Tree-SHA512: e4e72953c61a2f6995d609a32f8ed8e18cab9a92bc9e193d46a1d1f06d9daa5c6da6fce2867d4e3ba4fc0439141901a3d35f246486f0fa8f59587786379dfcbd
2 parents 342b9bc + 911a480 commit fa1ac28

File tree

9 files changed

+355
-242
lines changed

9 files changed

+355
-242
lines changed

‎src/qt/test/wallettests.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,8 @@ void WalletTests::walletTests()
8686
TestChain100Setup test;
8787
test.CreateAndProcessBlock({}, GetScriptForRawPubKey(test.coinbaseKey.GetPubKey()));
8888
bitdb.MakeMock();
89-
CWallet wallet("wallet_test.dat");
89+
std::unique_ptr<CWalletDBWrapper> dbw(new CWalletDBWrapper(&bitdb, "wallet_test.dat"));
90+
CWallet wallet(std::move(dbw));
9091
bool firstRun;
9192
wallet.LoadWallet(firstRun);
9293
{

‎src/wallet/db.cpp

Lines changed: 94 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -359,31 +359,34 @@ void CDBEnv::CheckpointLSN(const std::string& strFile)
359359
}
360360

361361

362-
CDB::CDB(const std::string& strFilename, const char* pszMode, bool fFlushOnCloseIn) : pdb(NULL), activeTxn(NULL)
362+
CDB::CDB(CWalletDBWrapper& dbw, const char* pszMode, bool fFlushOnCloseIn) : pdb(NULL), activeTxn(NULL)
363363
{
364364
int ret;
365365
fReadOnly = (!strchr(pszMode, '+') && !strchr(pszMode, 'w'));
366366
fFlushOnClose = fFlushOnCloseIn;
367-
if (strFilename.empty())
367+
env = dbw.env;
368+
if (dbw.IsDummy()) {
368369
return;
370+
}
371+
const std::string &strFilename = dbw.strFile;
369372

370373
bool fCreate = strchr(pszMode, 'c') != NULL;
371374
unsigned int nFlags = DB_THREAD;
372375
if (fCreate)
373376
nFlags |= DB_CREATE;
374377

375378
{
376-
LOCK(bitdb.cs_db);
377-
if (!bitdb.Open(GetDataDir()))
379+
LOCK(env->cs_db);
380+
if (!env->Open(GetDataDir()))
378381
throw std::runtime_error("CDB: Failed to open database environment.");
379382

380383
strFile = strFilename;
381-
++bitdb.mapFileUseCount[strFile];
382-
pdb = bitdb.mapDb[strFile];
384+
++env->mapFileUseCount[strFile];
385+
pdb = env->mapDb[strFile];
383386
if (pdb == NULL) {
384-
pdb = new Db(bitdb.dbenv, 0);
387+
pdb = new Db(env->dbenv, 0);
385388

386-
bool fMockDb = bitdb.IsMock();
389+
bool fMockDb = env->IsMock();
387390
if (fMockDb) {
388391
DbMpoolFile* mpf = pdb->get_mpf();
389392
ret = mpf->set_flags(DB_MPOOL_NOFILE, 1);
@@ -401,7 +404,7 @@ CDB::CDB(const std::string& strFilename, const char* pszMode, bool fFlushOnClose
401404
if (ret != 0) {
402405
delete pdb;
403406
pdb = NULL;
404-
--bitdb.mapFileUseCount[strFile];
407+
--env->mapFileUseCount[strFile];
405408
strFile = "";
406409
throw std::runtime_error(strprintf("CDB: Error %d, can't open database %s", ret, strFilename));
407410
}
@@ -413,7 +416,7 @@ CDB::CDB(const std::string& strFilename, const char* pszMode, bool fFlushOnClose
413416
fReadOnly = fTmp;
414417
}
415418

416-
bitdb.mapDb[strFile] = pdb;
419+
env->mapDb[strFile] = pdb;
417420
}
418421
}
419422
}
@@ -428,7 +431,7 @@ void CDB::Flush()
428431
if (fReadOnly)
429432
nMinutes = 1;
430433

431-
bitdb.dbenv->txn_checkpoint(nMinutes ? GetArg("-dblogsize", DEFAULT_WALLET_DBLOGSIZE) * 1024 : 0, nMinutes, 0);
434+
env->dbenv->txn_checkpoint(nMinutes ? GetArg("-dblogsize", DEFAULT_WALLET_DBLOGSIZE) * 1024 : 0, nMinutes, 0);
432435
}
433436

434437
void CDB::Close()
@@ -444,8 +447,8 @@ void CDB::Close()
444447
Flush();
445448

446449
{
447-
LOCK(bitdb.cs_db);
448-
--bitdb.mapFileUseCount[strFile];
450+
LOCK(env->cs_db);
451+
--env->mapFileUseCount[strFile];
449452
}
450453
}
451454

@@ -472,23 +475,28 @@ bool CDBEnv::RemoveDb(const std::string& strFile)
472475
return (rc == 0);
473476
}
474477

475-
bool CDB::Rewrite(const std::string& strFile, const char* pszSkip)
478+
bool CDB::Rewrite(CWalletDBWrapper& dbw, const char* pszSkip)
476479
{
480+
if (dbw.IsDummy()) {
481+
return true;
482+
}
483+
CDBEnv *env = dbw.env;
484+
const std::string& strFile = dbw.strFile;
477485
while (true) {
478486
{
479-
LOCK(bitdb.cs_db);
480-
if (!bitdb.mapFileUseCount.count(strFile) || bitdb.mapFileUseCount[strFile] == 0) {
487+
LOCK(env->cs_db);
488+
if (!env->mapFileUseCount.count(strFile) || env->mapFileUseCount[strFile] == 0) {
481489
// Flush log data to the dat file
482-
bitdb.CloseDb(strFile);
483-
bitdb.CheckpointLSN(strFile);
484-
bitdb.mapFileUseCount.erase(strFile);
490+
env->CloseDb(strFile);
491+
env->CheckpointLSN(strFile);
492+
env->mapFileUseCount.erase(strFile);
485493

486494
bool fSuccess = true;
487495
LogPrintf("CDB::Rewrite: Rewriting %s...\n", strFile);
488496
std::string strFileRes = strFile + ".rewrite";
489497
{ // surround usage of db with extra {}
490-
CDB db(strFile.c_str(), "r");
491-
Db* pdbCopy = new Db(bitdb.dbenv, 0);
498+
CDB db(dbw, "r");
499+
Db* pdbCopy = new Db(env->dbenv, 0);
492500

493501
int ret = pdbCopy->open(NULL, // Txn pointer
494502
strFileRes.c_str(), // Filename
@@ -531,17 +539,17 @@ bool CDB::Rewrite(const std::string& strFile, const char* pszSkip)
531539
}
532540
if (fSuccess) {
533541
db.Close();
534-
bitdb.CloseDb(strFile);
542+
env->CloseDb(strFile);
535543
if (pdbCopy->close(0))
536544
fSuccess = false;
537545
delete pdbCopy;
538546
}
539547
}
540548
if (fSuccess) {
541-
Db dbA(bitdb.dbenv, 0);
549+
Db dbA(env->dbenv, 0);
542550
if (dbA.remove(strFile.c_str(), NULL, 0))
543551
fSuccess = false;
544-
Db dbB(bitdb.dbenv, 0);
552+
Db dbB(env->dbenv, 0);
545553
if (dbB.rename(strFileRes.c_str(), NULL, strFile.c_str(), 0))
546554
fSuccess = false;
547555
}
@@ -596,16 +604,21 @@ void CDBEnv::Flush(bool fShutdown)
596604
}
597605
}
598606

599-
bool CDB::PeriodicFlush(std::string strFile)
607+
bool CDB::PeriodicFlush(CWalletDBWrapper& dbw)
600608
{
609+
if (dbw.IsDummy()) {
610+
return true;
611+
}
601612
bool ret = false;
613+
CDBEnv *env = dbw.env;
614+
const std::string& strFile = dbw.strFile;
602615
TRY_LOCK(bitdb.cs_db,lockDb);
603616
if (lockDb)
604617
{
605618
// Don't do this if any databases are in use
606619
int nRefCount = 0;
607-
std::map<std::string, int>::iterator mit = bitdb.mapFileUseCount.begin();
608-
while (mit != bitdb.mapFileUseCount.end())
620+
std::map<std::string, int>::iterator mit = env->mapFileUseCount.begin();
621+
while (mit != env->mapFileUseCount.end())
609622
{
610623
nRefCount += (*mit).second;
611624
mit++;
@@ -614,17 +627,17 @@ bool CDB::PeriodicFlush(std::string strFile)
614627
if (nRefCount == 0)
615628
{
616629
boost::this_thread::interruption_point();
617-
std::map<std::string, int>::iterator mi = bitdb.mapFileUseCount.find(strFile);
618-
if (mi != bitdb.mapFileUseCount.end())
630+
std::map<std::string, int>::iterator mi = env->mapFileUseCount.find(strFile);
631+
if (mi != env->mapFileUseCount.end())
619632
{
620633
LogPrint(BCLog::DB, "Flushing %s\n", strFile);
621634
int64_t nStart = GetTimeMillis();
622635

623636
// Flush wallet file so it's self contained
624-
bitdb.CloseDb(strFile);
625-
bitdb.CheckpointLSN(strFile);
637+
env->CloseDb(strFile);
638+
env->CheckpointLSN(strFile);
626639

627-
bitdb.mapFileUseCount.erase(mi++);
640+
env->mapFileUseCount.erase(mi++);
628641
LogPrint(BCLog::DB, "Flushed %s %dms\n", strFile, GetTimeMillis() - nStart);
629642
ret = true;
630643
}
@@ -633,3 +646,52 @@ bool CDB::PeriodicFlush(std::string strFile)
633646

634647
return ret;
635648
}
649+
650+
bool CWalletDBWrapper::Rewrite(const char* pszSkip)
651+
{
652+
return CDB::Rewrite(*this, pszSkip);
653+
}
654+
655+
bool CWalletDBWrapper::Backup(const std::string& strDest)
656+
{
657+
if (IsDummy()) {
658+
return false;
659+
}
660+
while (true)
661+
{
662+
{
663+
LOCK(env->cs_db);
664+
if (!env->mapFileUseCount.count(strFile) || env->mapFileUseCount[strFile] == 0)
665+
{
666+
// Flush log data to the dat file
667+
env->CloseDb(strFile);
668+
env->CheckpointLSN(strFile);
669+
env->mapFileUseCount.erase(strFile);
670+
671+
// Copy wallet file
672+
fs::path pathSrc = GetDataDir() / strFile;
673+
fs::path pathDest(strDest);
674+
if (fs::is_directory(pathDest))
675+
pathDest /= strFile;
676+
677+
try {
678+
fs::copy_file(pathSrc, pathDest, fs::copy_option::overwrite_if_exists);
679+
LogPrintf("copied %s to %s\n", strFile, pathDest.string());
680+
return true;
681+
} catch (const fs::filesystem_error& e) {
682+
LogPrintf("error copying %s to %s - %s\n", strFile, pathDest.string(), e.what());
683+
return false;
684+
}
685+
}
686+
}
687+
MilliSleep(100);
688+
}
689+
return false;
690+
}
691+
692+
void CWalletDBWrapper::Flush(bool shutdown)
693+
{
694+
if (!IsDummy()) {
695+
env->Flush(shutdown);
696+
}
697+
}

‎src/wallet/db.h

Lines changed: 53 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,52 @@ class CDBEnv
8686

8787
extern CDBEnv bitdb;
8888

89+
/** An instance of this class represents one database.
90+
* For BerkeleyDB this is just a (env, strFile) tuple.
91+
**/
92+
class CWalletDBWrapper
93+
{
94+
friend class CDB;
95+
public:
96+
/** Create dummy DB handle */
97+
CWalletDBWrapper(): env(nullptr)
98+
{
99+
}
100+
101+
/** Create DB handle to real database */
102+
CWalletDBWrapper(CDBEnv *env_in, const std::string &strFile_in):
103+
env(env_in), strFile(strFile_in)
104+
{
105+
}
106+
107+
/** Rewrite the entire database on disk, with the exception of key pszSkip if non-zero
108+
*/
109+
bool Rewrite(const char* pszSkip=nullptr);
110+
111+
/** Back up the entire database to a file.
112+
*/
113+
bool Backup(const std::string& strDest);
114+
115+
/** Get a name for this database, for debugging etc.
116+
*/
117+
std::string GetName() const { return strFile; }
118+
119+
/** Make sure all changes are flushed to disk.
120+
*/
121+
void Flush(bool shutdown);
122+
123+
private:
124+
/** BerkeleyDB specific */
125+
CDBEnv *env;
126+
std::string strFile;
127+
128+
/** Return whether this database handle is a dummy for testing.
129+
* Only to be used at a low level, application should ideally not care
130+
* about this.
131+
*/
132+
bool IsDummy() { return env == nullptr; }
133+
};
134+
89135

90136
/** RAII class that provides access to a Berkeley database */
91137
class CDB
@@ -96,18 +142,19 @@ class CDB
96142
DbTxn* activeTxn;
97143
bool fReadOnly;
98144
bool fFlushOnClose;
145+
CDBEnv *env;
99146

100-
explicit CDB(const std::string& strFilename, const char* pszMode = "r+", bool fFlushOnCloseIn=true);
147+
public:
148+
explicit CDB(CWalletDBWrapper& dbw, const char* pszMode = "r+", bool fFlushOnCloseIn=true);
101149
~CDB() { Close(); }
102150

103-
public:
104151
void Flush();
105152
void Close();
106153
static bool Recover(const std::string& filename, void *callbackDataIn, bool (*recoverKVcallback)(void* callbackData, CDataStream ssKey, CDataStream ssValue));
107154

108155
/* flush the wallet passively (TRY_LOCK)
109156
ideal to be called periodically */
110-
static bool PeriodicFlush(std::string strFile);
157+
static bool PeriodicFlush(CWalletDBWrapper& dbw);
111158
/* verifies the database environment */
112159
static bool VerifyEnvironment(const std::string& walletFile, const fs::path& dataDir, std::string& errorStr);
113160
/* verifies the database file */
@@ -117,7 +164,7 @@ class CDB
117164
CDB(const CDB&);
118165
void operator=(const CDB&);
119166

120-
protected:
167+
public:
121168
template <typename K, typename T>
122169
bool Read(const K& key, T& value)
123170
{
@@ -156,7 +203,7 @@ class CDB
156203
bool Write(const K& key, const T& value, bool fOverwrite = true)
157204
{
158205
if (!pdb)
159-
return false;
206+
return true;
160207
if (fReadOnly)
161208
assert(!"Write called on database in read-only mode");
162209

@@ -310,7 +357,7 @@ class CDB
310357
return Write(std::string("version"), nVersion);
311358
}
312359

313-
bool static Rewrite(const std::string& strFile, const char* pszSkip = NULL);
360+
bool static Rewrite(CWalletDBWrapper& dbw, const char* pszSkip = NULL);
314361
};
315362

316363
#endif // BITCOIN_WALLET_DB_H

‎src/wallet/rpcwallet.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2078,7 +2078,7 @@ UniValue walletpassphrase(const JSONRPCRequest& request)
20782078

20792079
int64_t nSleepTime = request.params[1].get_int64();
20802080
pwallet->nRelockTime = GetTime() + nSleepTime;
2081-
RPCRunLater(strprintf("lockwallet(%s)", pwallet->strWalletFile), boost::bind(LockWallet, pwallet), nSleepTime);
2081+
RPCRunLater(strprintf("lockwallet(%s)", pwallet->GetName()), boost::bind(LockWallet, pwallet), nSleepTime);
20822082

20832083
return NullUniValue;
20842084
}

‎src/wallet/test/wallet_test_fixture.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,8 @@ WalletTestingSetup::WalletTestingSetup(const std::string& chainName):
1414
bitdb.MakeMock();
1515

1616
bool fFirstRun;
17-
pwalletMain = new CWallet("wallet_test.dat");
17+
std::unique_ptr<CWalletDBWrapper> dbw(new CWalletDBWrapper(&bitdb, "wallet_test.dat"));
18+
pwalletMain = new CWallet(std::move(dbw));
1819
pwalletMain->LoadWallet(fFirstRun);
1920
RegisterValidationInterface(pwalletMain);
2021

0 commit comments

Comments
 (0)