Encapsulate BDB environment inside new CDBEnv class #1293

jgarzik commented at 12:23 AM on May 14, 2012: contributor

Cleans up and organizes several scattered functions and variables related to the BDB env. Class CDBInit() existed to provide a guaranteed-via-C++-destructor cleanup of the db environment.

A formal CDBEnv class provides all of this inside a single wrapper.

NOTES:

Other db.cpp-related operations might be candidates for CDBEnv inclusion. This current commit was chosen as a good starting point, only encapsulating clearly-dbenv-related items.
Further wrappers may easily encapsulate remaining direct bitdb.dbenv accesses, though it hardly seems worth it.
CDBEnv is a starting point for easy cross-database transactions. BDB supports this, but bitcoin codebase hardcodes txn<->database association into CDB.

jgarzik commented at 5:17 AM on May 14, 2012: contributor

Rebased on top of #1295 (check return values of TxnBegin, TxnCommit)

jgarzik commented at 4:53 PM on May 14, 2012: contributor

DEPENDENCY UPDATE: this sits on top of #1300, in addition to #1295

in src/walletdb.cpp:None in c7ab7e596e outdated

  12 | @@ -13,7 +13,6 @@
  13 |  
  14 |  static uint64 nAccountingEntryNumber = 0;
  15 |  
  16 | -extern CCriticalSection cs_db;
  17 |  extern map<string, int> mapFileUseCount;

sipa commented at 9:57 PM on May 14, 2012:

Does mapFileUseCount need to remain global?

jgarzik commented at 10:17 PM on May 14, 2012:

No, that and mapFiles[] could be moved into CDBEnv as well.

jgarzik commented at 2:55 AM on May 15, 2012: contributor

Moved mapDb and mapFileUseCount into class CDBEnv. mapFileUseCount had to remain public for the moment, until some wrappers may be written that eliminates the need for walletdb and CDB::Rewrite() to directly access these internals.

It doesn't help that we use this reference counting scheme sometimes, and other times we wish to immediately close regardless of reference count.

jgarzik commented at 7:14 PM on May 19, 2012: contributor

This seems stable in testing, though the encapsulation isn't the best. Some of the mapFileUseCount[] stuff is rather delicate, and relies on implicit behavior. Best left to complete the encapsulation of that to a separate commit, IMO. Once done, that will fit naturally inside CDBEnv::OpenDb(), which would be split out from CDB::CDB().

Reorganize(): remove spurious TxnAbort()

If Reorganize() fails, then its caller, CBlock::SetBestChain(),
will call TxnAbort().

Redundant TxnAbort() calls are harmless.  The second will return an
error return value, with no other side effects.  TxnAbort() return
values are generally never checked.  The impact is nil.

b52a270538

Encapsulate BDB environment inside new CDBEnv class

Cleans up and organizes several scattered functions and variables related to
the BDB env.  Class CDBInit() existed to provide a
guaranteed-via-C++-destructor cleanup of the db environment.

A formal CDBEnv class provides all of this inside a single wrapper.

cd9696fc97

Create CDBEnv::TxnBegin(), and use it in CDB::TxnBegin() 24b57e3c6a

Remove unused nested BDB transaction support 8b1202c52c

CDB::CDB: properly initialize activeTxn to NULL 94e34fa0ad

Further CDBEnv encapsulation work. ffe8b77a61

sipa commented at 9:00 PM on May 22, 2012: member

ACK

jgarzik merged this on May 22, 2012

jgarzik closed this on May 22, 2012

jgarzik deleted the branch on Aug 24, 2014

lateminer referenced this in commit 2385b047d3 on Jan 22, 2019

lateminer referenced this in commit 28e0048b3e on May 6, 2020

lateminer referenced this in commit 185194bd7e on May 6, 2020

DrahtBot locked this on Sep 8, 2021