Changed random eviction cache to accounts only (#4650)

# Description

This change adds a configurable memory based cap to the LiveBucketList
random eviction cache. It also modifies the cache such that only
accounts are stored.

Following initial testing, the random eviction cache improved TX
flooding performance, but had virtually no effect on apply time. This
makes sense, since apply time already leverages the ltx cache and only
makes at most one disk read per ledger entry. TX flooding on the other
hand has no such queue and constantly re reads entries off disk. Given
that we already have an ltx cache for apply time, it makes sense to only
cache the entry types that are used by transaction flooding (i.e.
`ACCOUNT`).

My initial default setting is to cache 3 GB. Currently accounts take up
about 2.5 GB in the BucketList, so this allows us to cache all accounts
with some room for growth, and 3 GB seems like a modest increase in RAM
requirements for validators.

# Checklist
- [x] Reviewed the
[contributing](https://github.com/stellar/stellar-core/blob/master/CONTRIBUTING.md#submitting-changes)
document
- [x] Rebased on top of master (no merge commits)
- [x] Ran `clang-format` v8.0.0 (via `make format` or the Visual Studio
extension)
- [x] Compiles
- [x] Ran all tests
- [ ] If change impacts performance, include supporting evidence per the
[performance
document](https://github.com/stellar/stellar-core/blob/master/performance-eval/performance-eval.md)

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,8 @@ BucketManager::assumeState(HistoryArchiveState const& has,
         mLiveBucketList->getLevel(i).setNext(nextFuture);
     }
 
+    mLiveBucketList->maybeInitializeCaches(mConfig);
+
     if (restartMerges)
     {
         mLiveBucketList->restartMerges(mApp, maxProtocolVersion,

src/bucket/BucketUtils.cpp

@@ -348,6 +348,18 @@ BucketEntryCounters::numEntries() const
     return num;
 }
 
+BucketEntryCounters::BucketEntryCounters()
+{
+    for (uint32_t type =
+             static_cast<uint32_t>(LedgerEntryTypeAndDurability::ACCOUNT);
+         type < static_cast<uint32_t>(LedgerEntryTypeAndDurability::NUM_TYPES);
+         ++type)
+    {
+        entryTypeCounts[static_cast<LedgerEntryTypeAndDurability>(type)] = 0;
+        entryTypeSizes[static_cast<LedgerEntryTypeAndDurability>(type)] = 0;
+    }
+}
+
 template void
 BucketEntryCounters::count<LiveBucket>(LiveBucket::EntryT const& be);
 template void BucketEntryCounters::count<HotArchiveBucket>(

src/bucket/BucketUtils.h

@@ -208,6 +208,8 @@ struct BucketEntryCounters
     {
         ar(entryTypeCounts, entryTypeSizes);
     }
+
+    BucketEntryCounters();
 };
 
 template <class BucketT>

src/bucket/LiveBucket.cpp

@@ -308,6 +308,17 @@ LiveBucket::apply(Application& app) const
     }
     counters.logInfo("direct", 0, app.getClock().now());
 }
+
+size_t
+LiveBucket::getMaxCacheSize() const
+{
+    if (!isEmpty())
+    {
+        return getIndex().getMaxCacheSize();
+    }
+
+    return 0;
+}
 #endif // BUILD_TESTS
 
 std::optional<std::pair<std::streamoff, std::streamoff>>
@@ -431,6 +442,14 @@ LiveBucket::getBucketVersion() const
     return it.getMetadata().ledgerVersion;
 }
 
+void
+LiveBucket::maybeInitializeCache(size_t totalBucketListAccountsSizeBytes,
+                                 Config const& cfg) const
+{
+    releaseAssert(mIndex);
+    mIndex->maybeInitializeCache(totalBucketListAccountsSizeBytes, cfg);
+}
+
 BucketEntryCounters const&
 LiveBucket::getBucketEntryCounters() const
 {

src/bucket/LiveBucket.h

@@ -83,6 +83,7 @@ class LiveBucket : public BucketBase<LiveBucket, LiveBucketIndex>,
     // entry in the database (respectively; if the entry is dead (a
     // tombstone), deletes the corresponding entry in the database.
     void apply(Application& app) const;
+    size_t getMaxCacheSize() const;
 #endif
 
     // Returns [lowerBound, upperBound) of file offsets for all offers in the
@@ -120,6 +121,13 @@ class LiveBucket : public BucketBase<LiveBucket, LiveBucketIndex>,
 
     uint32_t getBucketVersion() const;
 
+    // Initializes the random eviction cache if it has not already been
+    // initialized. totalBucketListAccountsSizeBytes is the total size, in
+    // bytes, of all BucketEntries in the BucketList that hold ACCOUNT entries,
+    // including INIT, LIVE and DEAD entries.
+    void maybeInitializeCache(size_t totalBucketListAccountsSizeBytes,
+                              Config const& cfg) 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,77 @@ LiveBucketIndex::LiveBucketIndex(BucketManager const& bm, Archive& ar,
     releaseAssertOrThrow(pageSize != 0);
 }
 
+void
+LiveBucketIndex::maybeInitializeCache(size_t totalBucketListAccountsSizeBytes,
+                                      Config const& cfg) const
+{
+    // Everything is already in memory, no need for a redundant cache.
+    if (mInMemoryIndex)
+    {
+        return;
+    }
+
+    // Cache is already initialized
+    if (std::shared_lock<std::shared_mutex> lock(mCacheMutex); mCache)
+    {
+        return;
+    }
+
+    releaseAssert(mDiskIndex);
+
+    auto accountsInThisBucket =
+        mDiskIndex->getBucketEntryCounters().entryTypeCounts.at(
+            LedgerEntryTypeAndDurability::ACCOUNT);
+
+    // Nothing to cache
+    if (accountsInThisBucket == 0)
+    {
+        return;
+    }
+
+    // Convert from MB to bytes, max size for entire BucketList cache
+    auto maxBucketListBytesToCache =
+        cfg.BUCKETLIST_DB_MEMORY_FOR_CACHING * 1024 * 1024;
+
+    std::unique_lock<std::shared_mutex> lock(mCacheMutex);
+    if (totalBucketListAccountsSizeBytes < maxBucketListBytesToCache)
+    {
+        // We can cache the entire bucket
+        mCache = std::make_unique<CacheT>(accountsInThisBucket);
+    }
+    else
+    {
+        // The random eviction cache has an entry limit, but we expose a memory
+        // limit in the validator config. We can't do an exact 1 to 1 mapping
+        // because account entries have different sizes.
+        //
+        // First we take the fraction of the total BucketList size that this
+        // bucket occupies to figure out how much memory to allocate. Then we
+        // use the average account size to convert that to an entry count for
+        // the cache.
+
+        auto accountBytesInThisBucket =
+            mDiskIndex->getBucketEntryCounters().entryTypeSizes.at(
+                LedgerEntryTypeAndDurability::ACCOUNT);
+
+        double fractionOfTotalBucketListBytes =
+            static_cast<double>(accountBytesInThisBucket) /
+            totalBucketListAccountsSizeBytes;
+
+        size_t bytesAvailableForBucketCache = static_cast<size_t>(
+            maxBucketListBytesToCache * fractionOfTotalBucketListBytes);
+
+        double averageAccountSize =
+            static_cast<double>(accountBytesInThisBucket) /
+            accountsInThisBucket;
+
+        auto accountsToCache = static_cast<size_t>(
+            bytesAvailableForBucketCache / averageAccountSize);
+
+        mCache = std::make_unique<CacheT>(accountsToCache);
+    }
+}
+
 LiveBucketIndex::IterT
 LiveBucketIndex::begin() const
 {
@@ -133,7 +190,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 +319,24 @@ LiveBucketIndex::getBucketEntryCounters() const
     return mInMemoryIndex->getBucketEntryCounters();
 }
 
+bool
+LiveBucketIndex::shouldUseCache() const
+{
+    if (mDiskIndex)
+    {
+        std::shared_lock<std::shared_mutex> lock(mCacheMutex);
+        return mCache != nullptr;
+    }
+
+    return false;
+}
+
+bool
+LiveBucketIndex::isCachedType(LedgerKey const& lk)
+{
+    return lk.type() == ACCOUNT;
+}
+
 void
 LiveBucketIndex::maybeAddToCache(
     std::shared_ptr<BucketEntry const> const& entry) const
@@ -270,6 +345,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.
@@ -308,6 +387,18 @@ LiveBucketIndex::operator==(LiveBucketIndex const& in) const
 
     return true;
 }
+
+size_t
+LiveBucketIndex::getMaxCacheSize() const
+{
+    if (shouldUseCache())
+    {
+        std::shared_lock<std::shared_mutex> lock(mCacheMutex);
+        return mCache->maxSize();
+    }
+
+    return 0;
+}
 #endif
 
 template LiveBucketIndex::LiveBucketIndex(BucketManager const& bm,

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;
@@ -83,11 +83,12 @@ class LiveBucketIndex : public NonMovableOrCopyable
         return std::get<InMemoryIndex::IterT>(iter);
     }
 
-    bool
-    shouldUseCache() const
-    {
-        return mDiskIndex && mCache;
-    }
+    bool shouldUseCache() const;
+
+    // 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 +98,28 @@ 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.
+    //
+    // bucketListTotalAccountSizeBytes is the total size, in bytes, of all
+    // BucketEntries in the BucketList that hold ACCOUNT entries, including
+    // INIT, LIVE and DEAD entries.
+    void maybeInitializeCache(size_t bucketListTotalAccountSizeBytes,
+                              Config const& cfg) const;
+
     // Returns true if LedgerEntryType not supported by BucketListDB
     static bool typeNotSupported(LedgerEntryType t);
 
@@ -131,8 +146,7 @@ class LiveBucketIndex : public NonMovableOrCopyable
     void markBloomMiss() const;
 #ifdef BUILD_TESTS
     bool operator==(LiveBucketIndex const& in) const;
-
-    void clearCache() const;
+    size_t getMaxCacheSize() const;
 #endif
 };
 }