Cleanup

Author: SirTyson

src/bucket/BucketList.cpp

@@ -63,7 +63,7 @@ BucketLevel::getNext()
 void
 BucketLevel::setNext(FutureBucket const& fb)
 {
-    assertThreadIsMain();
+    releaseAssert(threadIsMain());
     mNextCurr = fb;
 }
 
@@ -82,7 +82,7 @@ BucketLevel::getSnap() const
 void
 BucketLevel::setCurr(std::shared_ptr<Bucket> b)
 {
-    assertThreadIsMain();
+    releaseAssert(threadIsMain());
     mNextCurr.clear();
     mCurr = b;
 }
@@ -117,7 +117,7 @@ BucketList::shouldMergeWithEmptyCurr(uint32_t ledger, uint32_t level)
 void
 BucketLevel::setSnap(std::shared_ptr<Bucket> b)
 {
-    assertThreadIsMain();
+    releaseAssert(threadIsMain());
     mSnap = b;
 }
 

src/bucket/BucketListSnapshot.cpp

@@ -16,7 +16,7 @@ namespace stellar
 BucketListSnapshot::BucketListSnapshot(BucketList const& bl, uint32_t ledgerSeq)
     : mLedgerSeq(ledgerSeq)
 {
-    assertThreadIsMain();
+    releaseAssert(threadIsMain());
 
     for (uint32_t i = 0; i < BucketList::kNumLevels; ++i)
     {
@@ -158,7 +158,7 @@ SearchableBucketListSnapshot::loadPoolShareTrustLinesByAccountAndAsset(
     ZoneScoped;
 
     // This query should only be called during TX apply
-    assertThreadIsMain();
+    releaseAssert(threadIsMain());
     mSnapshotManager.maybeUpdateSnapshot(mSnapshot);
 
     LedgerKeySet trustlinesToLoad;
@@ -194,7 +194,7 @@ SearchableBucketListSnapshot::loadInflationWinners(size_t maxWinners,
 
     // This is a legacy query, should only be called by main thread during
     // catchup
-    assertThreadIsMain();
+    releaseAssert(threadIsMain());
     auto timer = mSnapshotManager.recordBulkLoadMetrics("inflationWinners", 0)
                      .TimeScope();
 

src/bucket/BucketManagerImpl.h

@@ -43,6 +43,10 @@ class BucketManagerImpl : public BucketManager
     std::unique_ptr<TmpDirManager> mTmpDirManager;
     std::unique_ptr<TmpDir> mWorkDir;
     std::map<Hash, std::shared_ptr<Bucket>> mSharedBuckets;
+
+    // Lock for managing raw Bucket files or the bucket directory. This lock is
+    // only required for file access, but is not required for logical changes to
+    // the BucketList (i.e. addBatch).
     mutable std::recursive_mutex mBucketMutex;
     std::unique_ptr<std::string> mLockedBucketDir;
     medida::Meter& mBucketObjectInsertBatch;

src/bucket/BucketSnapshotManager.cpp

@@ -24,7 +24,7 @@ BucketSnapshotManager::BucketSnapshotManager(
     , mBloomLookups(
           mMetrics.NewMeter({"bucketlistDB", "bloom", "lookups"}, "bloom"))
 {
-    assertThreadIsMain();
+    releaseAssert(threadIsMain());
 }
 
 std::unique_ptr<SearchableBucketListSnapshot>
@@ -41,7 +41,7 @@ BucketSnapshotManager::recordBulkLoadMetrics(std::string const& label,
 {
     // For now, only keep metrics for the main thread. We can decide on what
     // metrics make sense when more background services are added later.
-    assertThreadIsMain();
+    releaseAssert(threadIsMain());
 
     if (numEntries != 0)
     {
@@ -63,7 +63,7 @@ BucketSnapshotManager::getPointLoadTimer(LedgerEntryType t) const
 {
     // For now, only keep metrics for the main thread. We can decide on what
     // metrics make sense when more background services are added later.
-    assertThreadIsMain();
+    releaseAssert(threadIsMain());
 
     auto iter = mPointTimers.find(t);
     if (iter == mPointTimers.end())
@@ -84,6 +84,9 @@ BucketSnapshotManager::maybeUpdateSnapshot(
     if (!snapshot ||
         snapshot->getLedgerSeq() != mCurrentSnapshot->getLedgerSeq())
     {
+        // Should only update with a newer snapshot
+        releaseAssert(!snapshot || snapshot->getLedgerSeq() <
+                                       mCurrentSnapshot->getLedgerSeq());
         snapshot = std::make_unique<BucketListSnapshot>(*mCurrentSnapshot);
     }
 }
@@ -93,7 +96,7 @@ BucketSnapshotManager::updateCurrentSnapshot(
     std::unique_ptr<BucketListSnapshot const>&& newSnapshot)
 {
     releaseAssert(newSnapshot);
-    assertThreadIsMain();
+    releaseAssert(threadIsMain());
     std::lock_guard<std::recursive_mutex> lock(mSnapshotMutex);
     releaseAssert(!mCurrentSnapshot || newSnapshot->getLedgerSeq() >=
                                            mCurrentSnapshot->getLedgerSeq());

src/ledger/LedgerTxn.cpp

@@ -2782,7 +2782,6 @@ LedgerTxnRoot::Impl::commitChild(EntryIterator iter,
     // Clearing the cache does not throw
     mBestOffers.clear();
     mEntryCache.clear();
-    mSearchableBucketListSnapshot.reset();
 
     // std::unique_ptr<...>::reset does not throw
     mTransaction.reset();