Skip to content

Commit 2b2aef9

Browse files
authored
Better behavior when BucketListDB account cache disabled (#4656)
# Description This improves behavior when the account cache is disabled in BucketListDB. Previously, `BUCKETLIST_DB_MEMORY_FOR_CACHING == 0` was still valid, but we would still create a random eviction cache object that had no capacity. This change makes is such that we don't construct the cache at all in this case. I've also added an explicit test. # 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)
2 parents c29c06e + ec2a3fe commit 2b2aef9

File tree

2 files changed

+31
-6
lines changed

2 files changed

+31
-6
lines changed

src/bucket/LiveBucketIndex.cpp

+6-6
Original file line numberDiff line numberDiff line change
@@ -103,16 +103,16 @@ LiveBucketIndex::maybeInitializeCache(size_t totalBucketListAccountsSizeBytes,
103103
mDiskIndex->getBucketEntryCounters().entryTypeCounts.at(
104104
LedgerEntryTypeAndDurability::ACCOUNT);
105105

106-
// Nothing to cache
107-
if (accountsInThisBucket == 0)
108-
{
109-
return;
110-
}
111-
112106
// Convert from MB to bytes, max size for entire BucketList cache
113107
auto maxBucketListBytesToCache =
114108
cfg.BUCKETLIST_DB_MEMORY_FOR_CACHING * 1024 * 1024;
115109

110+
// Nothing to cache. or cache is disabled
111+
if (accountsInThisBucket == 0 || maxBucketListBytesToCache == 0)
112+
{
113+
return;
114+
}
115+
116116
std::unique_lock<std::shared_mutex> lock(mCacheMutex);
117117
if (totalBucketListAccountsSizeBytes < maxBucketListBytesToCache)
118118
{

src/bucket/test/BucketIndexTests.cpp

+25
Original file line numberDiff line numberDiff line change
@@ -637,6 +637,31 @@ TEST_CASE("key-value lookup", "[bucket][bucketindex]")
637637

638638
TEST_CASE("bl cache", "[bucket][bucketindex]")
639639
{
640+
SECTION("disable cache")
641+
{
642+
Config cfg(getTestConfig());
643+
cfg.BUCKETLIST_DB_INDEX_CUTOFF = 0;
644+
cfg.BUCKETLIST_DB_MEMORY_FOR_CACHING = 0;
645+
646+
auto test = BucketIndexTest(cfg);
647+
test.buildGeneralTest(/*isCacheTest=*/true);
648+
test.run();
649+
650+
auto& liveBL = test.getBM().getLiveBucketList();
651+
for (auto i = 0; i < LiveBucketList::kNumLevels; ++i)
652+
{
653+
auto level = liveBL.getLevel(i);
654+
REQUIRE(level.getCurr()->getMaxCacheSize() == 0);
655+
REQUIRE(level.getSnap()->getMaxCacheSize() == 0);
656+
}
657+
658+
// We shouldn't meter anything when cache is disabled
659+
auto& hitMeter = test.getBM().getCacheHitMeter();
660+
auto& missMeter = test.getBM().getCacheMissMeter();
661+
REQUIRE(hitMeter.count() == 0);
662+
REQUIRE(missMeter.count() == 0);
663+
}
664+
640665
auto runCacheTest = [](size_t cacheSizeMb, auto checkCacheSize,
641666
double expectedHitRate) {
642667
// Use disk index for all levels so each bucket has a cache

0 commit comments

Comments
 (0)