Have WalletBatch automatically flush every 1000 updates

achow101 · achow101 · commit d6576e349e8a · 2019-05-18T12:58:50.000-04:00
Since it now automatically flushes, we don't need to have
UpgradeKeyMetadata count and flush separately
diff --git a/src/wallet/db.cpp b/src/wallet/db.cpp
@@ -614,7 +614,9 @@ void BerkeleyBatch::Flush()
     if (fReadOnly)
         nMinutes = 1;
 
-    env->dbenv->txn_checkpoint(nMinutes ? gArgs.GetArg("-dblogsize", DEFAULT_WALLET_DBLOGSIZE) * 1024 : 0, nMinutes, 0);
+    if (env) { // env is nullptr for dummy databases (i.e. in tests). Don't actually flush if env is nullptr so we don't segfault
+        env->dbenv->txn_checkpoint(nMinutes ? gArgs.GetArg("-dblogsize", DEFAULT_WALLET_DBLOGSIZE) * 1024 : 0, nMinutes, 0);
+    }
 }
 
 void BerkeleyDatabase::IncrementUpdateCounter()
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
@@ -370,7 +370,6 @@ void CWallet::UpgradeKeyMetadata()
     }
 
     std::unique_ptr<WalletBatch> batch = MakeUnique<WalletBatch>(*database);
-    size_t cnt = 0;
     for (auto& meta_pair : mapKeyMetadata) {
         CKeyMetadata& meta = meta_pair.second;
         if (!meta.hd_seed_id.IsNull() && !meta.has_key_origin && meta.hdKeypath != "s") { // If the hdKeypath is "s", that's the seed and it doesn't have a key origin
@@ -393,10 +392,6 @@ void CWallet::UpgradeKeyMetadata()
             CPubKey pubkey;
             if (GetPubKey(meta_pair.first, pubkey)) {
                 batch->WriteKeyMetadata(meta, pubkey, true);
-                if (++cnt % 1000 == 0) {
-                    // avoid creating overlarge in-memory batches in case the wallet contains large amounts of keys
-                    batch.reset(new WalletBatch(*database));
-                }
             }
         }
     }
diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h
@@ -143,9 +143,11 @@ class CKeyMetadata
 };
 
 /** Access to the wallet database.
- * This represents a single transaction at the
- * database. It will be committed when the object goes out of scope.
- * Optionally (on by default) it will flush to disk as well.
+ * Opens the database and provides read and write access to it. Each read and write is its own transaction.
+ * Multiple operation transactions can be started using TxnBegin() and committed using TxnCommit()
+ * Otherwise the transaction will be committed when the object goes out of scope.
+ * Optionally (on by default) it will flush to disk on close.
+ * Every 1000 writes will automatically trigger a flush to disk.
  */
 class WalletBatch
 {
@@ -157,6 +159,9 @@ class WalletBatch
             return false;
         }
         m_database.IncrementUpdateCounter();
+        if (m_database.nUpdateCounter % 1000 == 0) {
+            m_batch.Flush();
+        }
         return true;
     }
 
@@ -167,6 +172,9 @@ class WalletBatch
             return false;
         }
         m_database.IncrementUpdateCounter();
+        if (m_database.nUpdateCounter % 1000 == 0) {
+            m_batch.Flush();
+        }
         return true;
     }
 

Original file line number	Diff line number	Diff line change
`@@ -614,7 +614,9 @@ void BerkeleyBatch::Flush()`
`614`	`614`	`if (fReadOnly)`
`615`	`615`	`nMinutes = 1;`
`616`	`616`
`617`		`- env->dbenv->txn_checkpoint(nMinutes ? gArgs.GetArg("-dblogsize", DEFAULT_WALLET_DBLOGSIZE) * 1024 : 0, nMinutes, 0);`
	`617`	`+ if (env) { // env is nullptr for dummy databases (i.e. in tests). Don't actually flush if env is nullptr so we don't segfault`
	`618`	`+ env->dbenv->txn_checkpoint(nMinutes ? gArgs.GetArg("-dblogsize", DEFAULT_WALLET_DBLOGSIZE) * 1024 : 0, nMinutes, 0);`
	`619`	`+ }`
`618`	`620`	`}`
`619`	`621`
`620`	`622`	`void BerkeleyDatabase::IncrementUpdateCounter()`
Original file line number	Diff line number	Diff line change
`@@ -370,7 +370,6 @@ void CWallet::UpgradeKeyMetadata()`
`370`	`370`	`}`
`371`	`371`
`372`	`372`	`std::unique_ptr<WalletBatch> batch = MakeUnique<WalletBatch>(*database);`
`373`		`- size_t cnt = 0;`
`374`	`373`	`for (auto& meta_pair : mapKeyMetadata) {`
`375`	`374`	`CKeyMetadata& meta = meta_pair.second;`
`376`	`375`	`if (!meta.hd_seed_id.IsNull() && !meta.has_key_origin && meta.hdKeypath != "s") { // If the hdKeypath is "s", that's the seed and it doesn't have a key origin`
`@@ -393,10 +392,6 @@ void CWallet::UpgradeKeyMetadata()`
`393`	`392`	`CPubKey pubkey;`
`394`	`393`	`if (GetPubKey(meta_pair.first, pubkey)) {`
`395`	`394`	`batch->WriteKeyMetadata(meta, pubkey, true);`
`396`		`- if (++cnt % 1000 == 0) {`
`397`		`- // avoid creating overlarge in-memory batches in case the wallet contains large amounts of keys`
`398`		`- batch.reset(new WalletBatch(*database));`
`399`		`- }`
`400`	`395`	`}`
`401`	`396`	`}`
`402`	`397`	`}`