Changed random eviction cache to accounts only

Author: SirTyson

docs/stellar-core_example.cfg

@@ -235,13 +235,12 @@ MAX_DEX_TX_OPERATIONS_IN_TX_SET = 0
 # 0, indiviudal index is always used. Default page size 16 kb.
 BUCKETLIST_DB_INDEX_PAGE_SIZE_EXPONENT = 14
 
-# BUCKETLIST_DB_CACHED_PERCENT (Integer) default 10
-# Percentage of entries cached by BucketListDB when Bucket size is larger
-# than BUCKETLIST_DB_INDEX_CUTOFF. Note that this value does not impact
-# Buckets smaller than BUCKETLIST_DB_INDEX_CUTOFF, as they are always
-# completely held in memory. Roughly speaking, RAM usage for BucketList
-# cache == BucketListSize * (BUCKETLIST_DB_CACHED_PERCENT / 100).
-BUCKETLIST_DB_CACHED_PERCENT = 10
+# BUCKETLIST_DB_MEMORY_FOR_CACHING (Integer) default 3000
+# Memory used for caching entries by BucketListDB when Bucket size is
+# larger than BUCKETLIST_DB_INDEX_CUTOFF, in MB. Note that this value does
+# not impact Buckets smaller than BUCKETLIST_DB_INDEX_CUTOFF, as they are
+# always completely held in memory. If set to 0, caching is disabled.
+BUCKETLIST_DB_MEMORY_FOR_CACHING = 3000
 
 # BUCKETLIST_DB_INDEX_CUTOFF (Integer) default 100
 # Size, in MB, determining whether a bucket should have an individual

src/bucket/BucketIndexUtils.cpp

@@ -19,8 +19,7 @@ namespace stellar
 std::streamoff
 getPageSizeFromConfig(Config const& cfg)
 {
-    if (cfg.BUCKETLIST_DB_INDEX_PAGE_SIZE_EXPONENT == 0 ||
-        cfg.BUCKETLIST_DB_CACHED_PERCENT == 100)
+    if (cfg.BUCKETLIST_DB_INDEX_PAGE_SIZE_EXPONENT == 0)
     {
         return 0;
     }

src/bucket/BucketIndexUtils.h

@@ -98,13 +98,17 @@ std::streamoff getPageSizeFromConfig(Config const& cfg);
 // Builds index for given bucketfile. This is expensive (> 20 seconds
 // for the largest buckets) and should only be called once. Constructs a
 // DiskIndex or InMemoryIndex depending on config and Bucket size.
+// Note: Constructor does not initialize the cache for live bucket indexes,
+// as this must be done when the Bucket is being added to the BucketList
 template <class BucketT>
 std::unique_ptr<typename BucketT::IndexT const>
 createIndex(BucketManager& bm, std::filesystem::path const& filename,
             Hash const& hash, asio::io_context& ctx, SHA256* hasher);
 
 // Loads index from given file. If file does not exist or if saved
 // index does not have expected version or pageSize, return null
+// Note: Constructor does not initialize the cache for live bucket indexes,
+// as this must be done when the Bucket is being added to the BucketList
 template <class BucketT>
 std::unique_ptr<typename BucketT::IndexT const>
 loadIndex(BucketManager const& bm, std::filesystem::path const& filename,

src/bucket/BucketManager.cpp

@@ -1291,6 +1291,9 @@ BucketManager::assumeState(HistoryArchiveState const& has,
         mLiveBucketList->getLevel(i).setNext(nextFuture);
     }
 
+    mLiveBucketList->maybeInitializeCaches(
+        mConfig.maxAccountsInBucketListCache());
+
     if (restartMerges)
     {
         mLiveBucketList->restartMerges(mApp, maxProtocolVersion,

src/bucket/LiveBucket.cpp

@@ -431,6 +431,15 @@ LiveBucket::getBucketVersion() const
     return it.getMetadata().ledgerVersion;
 }
 
+void
+LiveBucket::maybeInitializeCache(size_t bucketListTotalAccounts,
+                                 size_t maxBucketListAccountsToCache) const
+{
+    releaseAssert(mIndex);
+    mIndex->maybeInitializeCache(bucketListTotalAccounts,
+                                 maxBucketListAccountsToCache);
+}
+
 BucketEntryCounters const&
 LiveBucket::getBucketEntryCounters() const
 {

src/bucket/LiveBucket.h

@@ -120,6 +120,9 @@ class LiveBucket : public BucketBase<LiveBucket, LiveBucketIndex>,
 
     uint32_t getBucketVersion() const;
 
+    void maybeInitializeCache(size_t bucketListTotalAccounts,
+                              size_t maxBucketListAccountsToCache) const;
+
     BucketEntryCounters const& getBucketEntryCounters() const;
 
     friend class LiveBucketSnapshot;

src/bucket/LiveBucketIndex.cpp

@@ -5,6 +5,7 @@
 #include "bucket/LiveBucketIndex.h"
 #include "bucket/BucketIndexUtils.h"
 #include "bucket/BucketManager.h"
+#include "bucket/BucketUtils.h"
 #include "bucket/DiskIndex.h"
 #include "util/Fs.h"
 #include "util/GlobalChecks.h"
@@ -65,21 +66,6 @@ LiveBucketIndex::LiveBucketIndex(BucketManager& bm,
                    pageSize, filename);
         mDiskIndex = std::make_unique<DiskIndex<LiveBucket>>(
             bm, filename, pageSize, hash, ctx, hasher);
-
-        auto percentCached = bm.getConfig().BUCKETLIST_DB_CACHED_PERCENT;
-        if (percentCached > 0)
-        {
-            auto const& counters = mDiskIndex->getBucketEntryCounters();
-            auto cacheSize = (counters.numEntries() * percentCached) / 100;
-
-            // Minimum cache size of 100 if we are going to cache a non-zero
-            // number of entries
-            // We don't want to reserve here, since caches only live as long as
-            // the lifetime of the Bucket and fill relatively slowly
-            mCache = std::make_unique<CacheT>(std::max<size_t>(cacheSize, 100),
-                                              /*separatePRNG=*/false,
-                                              /*reserve=*/false);
-        }
     }
 }
 
@@ -95,6 +81,44 @@ LiveBucketIndex::LiveBucketIndex(BucketManager const& bm, Archive& ar,
     releaseAssertOrThrow(pageSize != 0);
 }
 
+void
+LiveBucketIndex::maybeInitializeCache(size_t bucketListTotalAccounts,
+                                      size_t maxBucketListAccountsToCache) const
+{
+    // Everything is already in memory, no need for a redundant cache, or we've
+    // already initialized this cache
+    if (mInMemoryIndex || mCache)
+    {
+        return;
+    }
+
+    releaseAssert(mDiskIndex);
+
+    auto accountsInThisBucket =
+        mDiskIndex->getBucketEntryCounters().entryTypeCounts.at(
+            LedgerEntryTypeAndDurability::ACCOUNT);
+
+    // Nothing to cache
+    if (accountsInThisBucket == 0)
+    {
+        return;
+    }
+
+    if (bucketListTotalAccounts < maxBucketListAccountsToCache)
+    {
+        // We can cache the entire bucket
+        mCache = std::make_unique<CacheT>(bucketListTotalAccounts);
+    }
+    else
+    {
+        double percentAccountsInBucket =
+            static_cast<double>(accountsInThisBucket) / bucketListTotalAccounts;
+        auto cacheSize = static_cast<size_t>(bucketListTotalAccounts *
+                                             percentAccountsInBucket);
+        mCache = std::make_unique<CacheT>(cacheSize);
+    }
+}
+
 LiveBucketIndex::IterT
 LiveBucketIndex::begin() const
 {
@@ -133,7 +157,7 @@ LiveBucketIndex::markBloomMiss() const
 std::shared_ptr<BucketEntry const>
 LiveBucketIndex::getCachedEntry(LedgerKey const& k) const
 {
-    if (shouldUseCache())
+    if (shouldUseCache() && isCachedType(k))
     {
         std::shared_lock<std::shared_mutex> lock(mCacheMutex);
         auto cachePtr = mCache->maybeGet(k);
@@ -262,6 +286,12 @@ LiveBucketIndex::getBucketEntryCounters() const
     return mInMemoryIndex->getBucketEntryCounters();
 }
 
+bool
+LiveBucketIndex::isCachedType(LedgerKey const& lk)
+{
+    return lk.type() == ACCOUNT;
+}
+
 void
 LiveBucketIndex::maybeAddToCache(
     std::shared_ptr<BucketEntry const> const& entry) const
@@ -270,6 +300,10 @@ LiveBucketIndex::maybeAddToCache(
     {
         releaseAssertOrThrow(entry);
         auto k = getBucketLedgerKey(*entry);
+        if (!isCachedType(k))
+        {
+            return;
+        }
 
         // If we are adding an entry to the cache, we must have missed it
         // earlier.

src/bucket/LiveBucketIndex.h

@@ -59,13 +59,13 @@ class LiveBucketIndex : public NonMovableOrCopyable
         RandomEvictionCache<LedgerKey, std::shared_ptr<BucketEntry const>>;
 
   private:
-    std::unique_ptr<DiskIndex<LiveBucket> const> mDiskIndex;
-    std::unique_ptr<InMemoryIndex const> mInMemoryIndex;
-    std::unique_ptr<CacheT> mCache;
+    std::unique_ptr<DiskIndex<LiveBucket> const> mDiskIndex{};
+    std::unique_ptr<InMemoryIndex const> mInMemoryIndex{};
 
     // The indexes themselves are thread safe, as they are immutable after
     // construction. The cache is not, all accesses must first acquire this
     // mutex.
+    mutable std::unique_ptr<CacheT> mCache{};
     mutable std::shared_mutex mCacheMutex;
 
     medida::Meter& mCacheHitMeter;
@@ -89,6 +89,11 @@ class LiveBucketIndex : public NonMovableOrCopyable
         return mDiskIndex && mCache;
     }
 
+    // Because we already have an LTX level cache at apply time, we only need to
+    // cache entry types that are used during TX validation and flooding.
+    // Currently, that is only ACCOUNT.
+    static bool isCachedType(LedgerKey const& lk);
+
     // Returns nullptr if cache is not enabled or entry not found
     std::shared_ptr<BucketEntry const> getCachedEntry(LedgerKey const& k) const;
 
@@ -97,14 +102,24 @@ class LiveBucketIndex : public NonMovableOrCopyable
     inline static const uint32_t BUCKET_INDEX_VERSION = 5;
 
     // Constructor for creating new index from Bucketfile
+    // Note: Constructor does not initialize the cache
     LiveBucketIndex(BucketManager& bm, std::filesystem::path const& filename,
                     Hash const& hash, asio::io_context& ctx, SHA256* hasher);
 
     // Constructor for loading pre-existing index from disk
+    // Note: Constructor does not initialize the cache
     template <class Archive>
     LiveBucketIndex(BucketManager const& bm, Archive& ar,
                     std::streamoff pageSize);
 
+    // Initializes the random eviction cache if it has not already been
+    // initialized. The random eviction cache itself has an entry limit, but we
+    // expose a memory limit in the validator config. To account for this, we
+    // check what percentage of the total number of accounts are in this bucket
+    // and allocate space accordingly.
+    void maybeInitializeCache(size_t bucketListTotalAccounts,
+                              size_t maxBucketListAccountsToCache) const;
+
     // Returns true if LedgerEntryType not supported by BucketListDB
     static bool typeNotSupported(LedgerEntryType t);
 

src/bucket/LiveBucketList.cpp

@@ -21,6 +21,9 @@ LiveBucketList::addBatch(Application& app, uint32_t currLedger,
     ZoneScoped;
     addBatchInternal(app, currLedger, currLedgerProtocol, initEntries,
                      liveEntries, deadEntries);
+
+    // Initialize caches for any new buckets we might have added
+    maybeInitializeCaches(app.getConfig().maxAccountsInBucketListCache());
 }
 
 BucketEntryCounters
@@ -41,6 +44,30 @@ LiveBucketList::sumBucketEntryCounters() const
     return counters;
 }
 
+void
+LiveBucketList::maybeInitializeCaches(size_t maxBucketListAccountsToCache) const
+{
+    auto blCounters = sumBucketEntryCounters();
+    size_t totalAccounts =
+        blCounters.entryTypeCounts.at(LedgerEntryTypeAndDurability::ACCOUNT);
+    for (uint32_t i = 0; i < kNumLevels; ++i)
+    {
+        auto curr = mLevels[i].getCurr();
+        if (!curr->isEmpty())
+        {
+            curr->maybeInitializeCache(totalAccounts,
+                                       maxBucketListAccountsToCache);
+        }
+
+        auto snap = mLevels[i].getSnap();
+        if (!snap->isEmpty())
+        {
+            snap->maybeInitializeCache(totalAccounts,
+                                       maxBucketListAccountsToCache);
+        }
+    }
+}
+
 void
 LiveBucketList::updateStartingEvictionIterator(EvictionIterator& iter,
                                                uint32_t firstScanLevel,

src/bucket/LiveBucketList.h

@@ -50,6 +50,10 @@ class LiveBucketList : public BucketListBase<LiveBucket>
                   std::vector<LedgerKey> const& deadEntries);
 
     BucketEntryCounters sumBucketEntryCounters() const;
+
+    // Initializes any uninitialized caches in the BucketIndex. Should be called
+    // after every time buckets may have changed in the LiveBucketList.
+    void maybeInitializeCaches(size_t maxBucketListAccountsToCache) const;
 };
 
 }

src/bucket/readme.md

@@ -97,8 +97,8 @@ lookup speed and memory overhead. The following configuration flags control thes
     on startup. Defaults to true, should only be set to false for testing purposes.
     Validators do not currently support persisted indexes. If NODE_IS_VALIDATOR=true,
     this value is ignored and indexes are never persisted.
-- `BUCKETLIST_DB_CACHED_PERCENT`
-  - Percentage of entries cached by BucketListDB when Bucket size is larger
-    than `BUCKETLIST_DB_INDEX_CUTOFF`. Note that this value does not impact
+- `BUCKETLIST_DB_MEMORY_FOR_CACHING`
+  - Memory used for caching entries by BucketListDB when Bucket size is larger
+    than `BUCKETLIST_DB_INDEX_CUTOFF`, in MB. Note that this value does not impact
     Buckets smaller than `BUCKETLIST_DB_INDEX_CUTOFF`, as they are always
     completely held in memory.

src/bucket/test/BucketIndexTests.cpp

@@ -63,15 +63,20 @@ class BucketIndexTest
     }
 
     void
-    buildBucketList(std::function<void(std::vector<LedgerEntry>&)> f)
+    buildBucketList(std::function<void(std::vector<LedgerEntry>&)> f,
+                    bool accountOnly = false)
     {
         uint32_t ledger = 0;
         do
         {
             ++ledger;
             std::vector<LedgerEntry> entries =
-                LedgerTestUtils::generateValidLedgerEntriesWithExclusions(
-                    {CONFIG_SETTING}, 10);
+                accountOnly
+                    ? LedgerTestUtils::
+                          generateValidUniqueLedgerEntriesWithTypes({ACCOUNT},
+                                                                    10)
+                    : LedgerTestUtils::generateValidLedgerEntriesWithExclusions(
+                          {CONFIG_SETTING}, 10);
             f(entries);
             closeLedger(*mApp);
         } while (!LiveBucketList::levelShouldSpill(ledger, mLevelsToBuild - 1));
@@ -92,7 +97,7 @@ class BucketIndexTest
     }
 
     virtual void
-    buildGeneralTest()
+    buildGeneralTest(bool accountOnly = false)
     {
         auto f = [&](std::vector<LedgerEntry> const& entries) {
             // Sample ~4% of entries
@@ -109,7 +114,7 @@ class BucketIndexTest
                 {}, entries, {});
         };
 
-        buildBucketList(f);
+        buildBucketList(f, accountOnly);
     }
 
     void
@@ -513,7 +518,7 @@ class BucketIndexPoolShareTest : public BucketIndexTest
     }
 
     virtual void
-    buildGeneralTest() override
+    buildGeneralTest(bool accountOnly = false) override
     {
         buildTest(false);
     }
@@ -581,12 +586,11 @@ TEST_CASE("bl cache", "[bucket][bucketindex]")
     Config cfg(getTestConfig());
 
     // Use disk index for all levels and cache all entries
-    // Note: Setting this to 100% will actually switch to in-memory index
-    cfg.BUCKETLIST_DB_CACHED_PERCENT = 99;
     cfg.BUCKETLIST_DB_INDEX_CUTOFF = 0;
+    cfg.BUCKETLIST_DB_MEMORY_FOR_CACHING = 5'000;
 
     auto test = BucketIndexTest(cfg);
-    test.buildGeneralTest();
+    test.buildGeneralTest(/*accountOnly=*/true);
     test.run(/*testCache=*/true);
 }