Allow freezing bucketlist snapshots to be refreshed by callers manually

stellar · Dec 20, 2024 · 8922ba5 · 8922ba5
1 parent 21f2b39
commit 8922ba5
Show file tree

Hide file tree

Showing 12 changed files with 73 additions and 30 deletions.
diff --git a/src/bucket/BucketListSnapshotBase.cpp b/src/bucket/BucketListSnapshotBase.cpp
@@ -54,7 +54,10 @@ LedgerHeader const&
 SearchableBucketListSnapshotBase<BucketT>::getLedgerHeader()
 {
     releaseAssert(mSnapshot);
-    mSnapshotManager.maybeUpdateSnapshot(mSnapshot, mHistoricalSnapshots);
+    if (mAutoUpdate)
+    {
+        mSnapshotManager.maybeUpdateSnapshot(mSnapshot, mHistoricalSnapshots);
+    }
     return mSnapshot->getLedgerHeader();
 }
 
@@ -63,7 +66,10 @@ LastClosedLedger const&
 SearchableBucketListSnapshotBase<BucketT>::getLastClosedLedger()
 {
     releaseAssert(mSnapshot);
-    mSnapshotManager.maybeUpdateSnapshot(mSnapshot, mHistoricalSnapshots);
+    if (mAutoUpdate)
+    {
+        mSnapshotManager.maybeUpdateSnapshot(mSnapshot, mHistoricalSnapshots);
+    }
     return mSnapshot->getLastClosedLedger();
 }
 
@@ -117,7 +123,10 @@ SearchableBucketListSnapshotBase<BucketT>::load(LedgerKey const& k)
         }
     };
 
-    mSnapshotManager.maybeUpdateSnapshot(mSnapshot, mHistoricalSnapshots);
+    if (mAutoUpdate)
+    {
+        mSnapshotManager.maybeUpdateSnapshot(mSnapshot, mHistoricalSnapshots);
+    }
     if (threadIsMain())
     {
         mSnapshotManager.startPointLoadTimer();
@@ -150,10 +159,12 @@ BucketLevelSnapshot<BucketT>::BucketLevelSnapshot(
 
 template <class BucketT>
 SearchableBucketListSnapshotBase<BucketT>::SearchableBucketListSnapshotBase(
-    BucketSnapshotManager const& snapshotManager)
-    : mSnapshotManager(snapshotManager), mHistoricalSnapshots()
+    BucketSnapshotManager const& snapshotManager, bool autoUpdate)
+    : mSnapshotManager(snapshotManager)
+    , mHistoricalSnapshots()
+    , mAutoUpdate(autoUpdate)
 {
-
+    // Always create a snapshot based on the latest LCL state.
     mSnapshotManager.maybeUpdateSnapshot(mSnapshot, mHistoricalSnapshots);
 }
 

diff --git a/src/bucket/BucketListSnapshotBase.h b/src/bucket/BucketListSnapshotBase.h
@@ -159,15 +159,20 @@ class SearchableBucketListSnapshotBase : public NonMovableOrCopyable
     // Snapshot managed by SnapshotManager
     SnapshotPtrT<BucketT> mSnapshot{};
     std::map<uint32_t, SnapshotPtrT<BucketT>> mHistoricalSnapshots;
+    bool const mAutoUpdate;
 
     // Loops through all buckets, starting with curr at level 0, then snap at
     // level 0, etc. Calls f on each bucket. Exits early if function
     // returns Loop::COMPLETE.
     void loopAllBuckets(std::function<Loop(BucketSnapshotT const&)> f,
                         BucketListSnapshot<BucketT> const& snapshot) const;
 
+    // If `autoUpdate` is true, the snapshot will keep itself consistent with
+    // LCL automatically. If not, callers are expected to refresh the snapshot
+    // manually (this is useful to use cases that a consistent view of the state
+    // for some arbitrary time)
     SearchableBucketListSnapshotBase(
-        BucketSnapshotManager const& snapshotManager);
+        BucketSnapshotManager const& snapshotManager, bool autoUpdate);
 
     std::optional<std::vector<typename BucketT::LoadT>>
     loadKeysInternal(std::set<LedgerKey, LedgerEntryIdCmp> const& inKeys,

diff --git a/src/bucket/BucketManager.cpp b/src/bucket/BucketManager.cpp
@@ -1054,8 +1054,8 @@ BucketManager::startBackgroundEvictionScan(uint32_t ledgerSeq)
     releaseAssert(!mEvictionFuture.valid());
     releaseAssert(mEvictionStatistics);
 
-    auto searchableBL =
-        mSnapshotManager->copySearchableLiveBucketListSnapshot();
+    auto searchableBL = mSnapshotManager->copySearchableLiveBucketListSnapshot(
+        /* autoUpdate */ true);
     auto const& cfg = mApp.getLedgerManager().getSorobanNetworkConfig();
     auto const& sas = cfg.stateArchivalSettings();
 
@@ -1588,7 +1588,8 @@ BucketManager::getSearchableLiveBucketListSnapshot()
     if (!mSearchableBucketListSnapshot)
     {
         mSearchableBucketListSnapshot =
-            mSnapshotManager->copySearchableLiveBucketListSnapshot();
+            mSnapshotManager->copySearchableLiveBucketListSnapshot(
+                /* autoUpdate */ true);
     }
 
     return mSearchableBucketListSnapshot;

diff --git a/src/bucket/BucketSnapshotManager.cpp b/src/bucket/BucketSnapshotManager.cpp
@@ -53,11 +53,12 @@ BucketSnapshotManager::BucketSnapshotManager(
 }
 
 std::shared_ptr<SearchableLiveBucketListSnapshot>
-BucketSnapshotManager::copySearchableLiveBucketListSnapshot() const
+BucketSnapshotManager::copySearchableLiveBucketListSnapshot(
+    bool autoUpdate) const
 {
     // Can't use std::make_shared due to private constructor
     return std::shared_ptr<SearchableLiveBucketListSnapshot>(
-        new SearchableLiveBucketListSnapshot(*this));
+        new SearchableLiveBucketListSnapshot(*this, autoUpdate));
 }
 
 std::shared_ptr<SearchableHotArchiveBucketListSnapshot>

diff --git a/src/bucket/BucketSnapshotManager.h b/src/bucket/BucketSnapshotManager.h
@@ -89,7 +89,7 @@ class BucketSnapshotManager : NonMovableOrCopyable
                           uint32_t numHistoricalLedgers);
 
     std::shared_ptr<SearchableLiveBucketListSnapshot>
-    copySearchableLiveBucketListSnapshot() const;
+    copySearchableLiveBucketListSnapshot(bool autoUpdate) const;
 
     std::shared_ptr<SearchableHotArchiveBucketListSnapshot>
     copySearchableHotArchiveBucketListSnapshot() const;

diff --git a/src/bucket/SearchableBucketList.cpp b/src/bucket/SearchableBucketList.cpp
@@ -61,6 +61,12 @@ SearchableLiveBucketListSnapshot::scanForEviction(
     return result;
 }
 
+void
+SearchableLiveBucketListSnapshot::updateSnapshotToLatest()
+{
+    mSnapshotManager.maybeUpdateSnapshot(mSnapshot, mHistoricalSnapshots);
+}
+
 template <class BucketT>
 std::optional<std::vector<typename BucketT::LoadT>>
 SearchableBucketListSnapshotBase<BucketT>::loadKeysInternal(
@@ -77,7 +83,10 @@ SearchableBucketListSnapshotBase<BucketT>::loadKeysInternal(
         return keys.empty() ? Loop::COMPLETE : Loop::INCOMPLETE;
     };
 
-    mSnapshotManager.maybeUpdateSnapshot(mSnapshot, mHistoricalSnapshots);
+    if (mAutoUpdate)
+    {
+        mSnapshotManager.maybeUpdateSnapshot(mSnapshot, mHistoricalSnapshots);
+    }
 
     if (!ledgerSeq || *ledgerSeq == mSnapshot->getLedgerSeq())
     {
@@ -111,7 +120,10 @@ SearchableLiveBucketListSnapshot::loadPoolShareTrustLinesByAccountAndAsset(
 
     // This query should only be called during TX apply
     releaseAssert(threadIsMain());
-    mSnapshotManager.maybeUpdateSnapshot(mSnapshot, mHistoricalSnapshots);
+    if (mAutoUpdate)
+    {
+        mSnapshotManager.maybeUpdateSnapshot(mSnapshot, mHistoricalSnapshots);
+    }
     releaseAssert(mSnapshot);
 
     LedgerKeySet trustlinesToLoad;
@@ -152,7 +164,10 @@ SearchableLiveBucketListSnapshot::loadInflationWinners(size_t maxWinners,
                                                        int64_t minBalance)
 {
     ZoneScoped;
-    mSnapshotManager.maybeUpdateSnapshot(mSnapshot, mHistoricalSnapshots);
+    if (mAutoUpdate)
+    {
+        mSnapshotManager.maybeUpdateSnapshot(mSnapshot, mHistoricalSnapshots);
+    }
     releaseAssert(mSnapshot);
 
     // This is a legacy query, should only be called by main thread during
@@ -261,14 +276,15 @@ SearchableLiveBucketListSnapshot::loadKeysWithLimits(
 }
 
 SearchableLiveBucketListSnapshot::SearchableLiveBucketListSnapshot(
-    BucketSnapshotManager const& snapshotManager)
-    : SearchableBucketListSnapshotBase<LiveBucket>(snapshotManager)
+    BucketSnapshotManager const& snapshotManager, bool autoUpdate)
+    : SearchableBucketListSnapshotBase<LiveBucket>(snapshotManager, autoUpdate)
 {
 }
 
 SearchableHotArchiveBucketListSnapshot::SearchableHotArchiveBucketListSnapshot(
     BucketSnapshotManager const& snapshotManager)
-    : SearchableBucketListSnapshotBase<HotArchiveBucket>(snapshotManager)
+    : SearchableBucketListSnapshotBase<HotArchiveBucket>(snapshotManager,
+                                                         /* autoUpdate */ true)
 {
 }
 

diff --git a/src/bucket/SearchableBucketList.h b/src/bucket/SearchableBucketList.h
@@ -14,7 +14,7 @@ class SearchableLiveBucketListSnapshot
     : public SearchableBucketListSnapshotBase<LiveBucket>
 {
     SearchableLiveBucketListSnapshot(
-        BucketSnapshotManager const& snapshotManager);
+        BucketSnapshotManager const& snapshotManager, bool autoUpdate);
 
   public:
     std::vector<LedgerEntry>
@@ -35,7 +35,9 @@ class SearchableLiveBucketListSnapshot
                                    StateArchivalSettings const& sas);
 
     friend std::shared_ptr<SearchableLiveBucketListSnapshot>
-    BucketSnapshotManager::copySearchableLiveBucketListSnapshot() const;
+    BucketSnapshotManager::copySearchableLiveBucketListSnapshot(
+        bool autoUpdate) const;
+    void updateSnapshotToLatest();
 };
 
 class SearchableHotArchiveBucketListSnapshot

diff --git a/src/bucket/test/BucketIndexTests.cpp b/src/bucket/test/BucketIndexTests.cpp
@@ -133,7 +133,7 @@ class BucketIndexTest
 
         auto searchableBL = getBM()
                                 .getBucketSnapshotManager()
-                                .copySearchableLiveBucketListSnapshot();
+                                .copySearchableLiveBucketListSnapshot(true);
         auto lk = LedgerEntryKey(canonicalEntry);
 
         auto currentLoadedEntry = searchableBL->load(lk);
@@ -252,7 +252,7 @@ class BucketIndexTest
     {
         auto searchableBL = getBM()
                                 .getBucketSnapshotManager()
-                                .copySearchableLiveBucketListSnapshot();
+                                .copySearchableLiveBucketListSnapshot(true);
 
         // Test bulk load lookup
         auto loadResult =
@@ -279,7 +279,7 @@ class BucketIndexTest
     {
         auto searchableBL = getBM()
                                 .getBucketSnapshotManager()
-                                .copySearchableLiveBucketListSnapshot();
+                                .copySearchableLiveBucketListSnapshot(true);
         for (size_t i = 0; i < n; ++i)
         {
             LedgerKeySet searchSubset;
@@ -319,7 +319,7 @@ class BucketIndexTest
     {
         auto searchableBL = getBM()
                                 .getBucketSnapshotManager()
-                                .copySearchableLiveBucketListSnapshot();
+                                .copySearchableLiveBucketListSnapshot(true);
 
         // Load should return empty vector for keys not in bucket list
         auto keysNotInBL =
@@ -496,7 +496,7 @@ class BucketIndexPoolShareTest : public BucketIndexTest
     {
         auto searchableBL = getBM()
                                 .getBucketSnapshotManager()
-                                .copySearchableLiveBucketListSnapshot();
+                                .copySearchableLiveBucketListSnapshot(true);
         auto loadResult =
             searchableBL->loadPoolShareTrustLinesByAccountAndAsset(
                 mAccountToSearch.accountID, mAssetToSearch);

diff --git a/src/bucket/test/BucketListTests.cpp b/src/bucket/test/BucketListTests.cpp
@@ -1405,7 +1405,8 @@ TEST_CASE_VERSIONS("Searchable BucketListDB snapshots", "[bucketlist]")
     entry.data.claimableBalance().amount = 0;
 
     auto searchableBL =
-        bm.getBucketSnapshotManager().copySearchableLiveBucketListSnapshot();
+        bm.getBucketSnapshotManager().copySearchableLiveBucketListSnapshot(
+            true);
 
     // Update entry every 5 ledgers so we can see bucket merge events
     for (auto ledgerSeq = 1; ledgerSeq < 101; ++ledgerSeq)

diff --git a/src/ledger/LedgerManagerImpl.cpp b/src/ledger/LedgerManagerImpl.cpp
@@ -1258,10 +1258,12 @@ LedgerManagerImpl::getCurrentLedgerStateSnaphot()
 {
     if (!mReadOnlyLedgerStateSnapshot)
     {
+        // Set autoUpdate to false to "freeze" the snapshot
+        // Update manually at the end of ledger close
         mReadOnlyLedgerStateSnapshot =
             mApp.getBucketManager()
                 .getBucketSnapshotManager()
-                .copySearchableLiveBucketListSnapshot();
+                .copySearchableLiveBucketListSnapshot(/* autoUpdate */ false);
     }
     return mReadOnlyLedgerStateSnapshot;
 }
@@ -1296,6 +1298,9 @@ LedgerManagerImpl::advanceLedgerPointers(LedgerHeader const& header,
         bm.getBucketSnapshotManager().updateCurrentSnapshot(
             std::move(liveSnapshot), std::move(hotArchiveSnapshot));
     }
+    // Manually refresh the snapshot now that apply is done
+    // This has to be done *after* snapshot manager updates current snapshot
+    getCurrentLedgerStateSnaphot()->updateSnapshotToLatest();
 }
 
 void

diff --git a/src/ledger/LedgerTxn.cpp b/src/ledger/LedgerTxn.cpp
@@ -3062,7 +3062,7 @@ LedgerTxnRoot::Impl::getSearchableLiveBucketListSnapshot() const
         mSearchableBucketListSnapshot =
             mApp.getBucketManager()
                 .getBucketSnapshotManager()
-                .copySearchableLiveBucketListSnapshot();
+                .copySearchableLiveBucketListSnapshot(/* autoUpdate */ true);
     }
 
     return *mSearchableBucketListSnapshot;

diff --git a/src/main/QueryServer.cpp b/src/main/QueryServer.cpp
@@ -67,7 +67,8 @@ QueryServer::QueryServer(const std::string& address, unsigned short port,
     for (auto pid : workerPids)
     {
         mBucketListSnapshots[pid] = std::move(
-            bucketSnapshotManager.copySearchableLiveBucketListSnapshot());
+            bucketSnapshotManager.copySearchableLiveBucketListSnapshot(
+                /* autoUpdate */ true));
     }
 }