Better bloom filter metrics

docs/metrics.md

@@ -31,6 +31,7 @@ bucket.batch.objectsadded                | meter     | number of objects added p
 bucket.memory.shared                     | counter   | number of buckets referenced (excluding publish queue)
 bucket.merge-time.level-<X>              | timer     | time to merge two buckets on level <X>
 bucket.snap.merge                        | timer     | time to merge two buckets
+bucketlistDB.bloom.lookups               | meter     | number of bloom filter lookups
 bucketlistDB.bloom.misses                | meter     | number of bloom filter false positives
 bucketlistDB.query.loads                 | meter     | number of BucketListDB load queries
 bucketlistDB.bulk.inflationWinners       | timer     | time to load inflation winners

src/bucket/BucketIndex.cpp

@@ -45,7 +45,8 @@ template <class IndexT> class BucketIndexImpl : public BucketIndex
     IndexT mKeysToOffset{};
     std::streamoff const mPageSize{};
     std::unique_ptr<bloom_filter> mFilter{};
-    medida::Meter& mBloomMisses;
+    medida::Meter& mBloomMissMeter;
+    medida::Meter& mBloomLookupMeter;
 
     BucketIndexImpl(BucketManager const& bm,
                     std::filesystem::path const& filename,
@@ -84,13 +85,16 @@ template <class IndexT> class BucketIndexImpl : public BucketIndex
     }
 
     virtual void markBloomMiss() const override;
+    virtual void markBloomLookup() const override;
 };
 
 template <class IndexT>
 BucketIndexImpl<IndexT>::BucketIndexImpl(BucketManager const& bm,
                                          std::filesystem::path const& filename,
                                          std::streamoff pageSize)
-    : mPageSize(pageSize), mBloomMisses(bm.getBloomMissMeter())
+    : mPageSize(pageSize)
+    , mBloomMissMeter(bm.getBloomMissMeter())
+    , mBloomLookupMeter(bm.getBloomLookupMeter())
 {
     ZoneScoped;
     releaseAssert(!filename.empty());
@@ -114,6 +118,14 @@ BucketIndexImpl<IndexT>::BucketIndexImpl(BucketManager const& bm,
         params.compute_optimal_parameters();
         mFilter = std::make_unique<bloom_filter>(params);
         estimatedIndexEntries = fileSize / mPageSize;
+        CLOG_DEBUG(
+            Bucket,
+            "Bloom filter initialized with params: projected element count "
+            "{} false positive probability: {}, number of hashes: {}, "
+            "table size: {}",
+            params.projected_element_count, params.false_positive_probability,
+            params.optimal_parameters.number_of_hashes,
+            params.optimal_parameters.table_size);
     }
     else
     {
@@ -174,7 +186,7 @@ BucketIndexImpl<IndexT>::BucketIndexImpl(BucketManager const& bm,
 
     CLOG_DEBUG(Bucket, "Indexed {} positions in {}", mKeysToOffset.size(),
                filename.filename());
-    if (estimatedNumElems < count)
+    if (std::is_same<IndexT, RangeIndex>::value && estimatedNumElems < count)
     {
         CLOG_WARNING(Bucket,
                      "Underestimated bloom filter size. Estimated entry "
@@ -313,6 +325,7 @@ BucketIndexImpl<IndexT>::scan(Iterator start, LedgerKey const& k) const
 
     // If the key is not in the bloom filter or in the lower bounded index
     // entry, return nullopt
+    markBloomLookup();
     if ((mFilter && !mFilter->contains(std::hash<stellar::LedgerKey>()(k))) ||
         keyIter == mKeysToOffset.end() || keyNotInIndexEntry(k, keyIter->first))
     {
@@ -372,6 +385,19 @@ template <>
 void
 BucketIndexImpl<BucketIndex::RangeIndex>::markBloomMiss() const
 {
-    mBloomMisses.Mark();
+    mBloomMissMeter.Mark();
+}
+
+template <class IndexT>
+void
+BucketIndexImpl<IndexT>::markBloomLookup() const
+{
+}
+
+template <>
+void
+BucketIndexImpl<BucketIndex::RangeIndex>::markBloomLookup() const
+{
+    mBloomLookupMeter.Mark();
 }
 }

src/bucket/BucketIndex.h

@@ -94,5 +94,6 @@ class BucketIndex : public NonMovableOrCopyable
     virtual Iterator end() const = 0;
 
     virtual void markBloomMiss() const = 0;
+    virtual void markBloomLookup() const = 0;
 };
 }

src/bucket/BucketManager.h

@@ -219,6 +219,7 @@ class BucketManager : NonMovableOrCopyable
     loadInflationWinners(size_t maxWinners, int64_t minBalance) const = 0;
 
     virtual medida::Meter& getBloomMissMeter() const = 0;
+    virtual medida::Meter& getBloomLookupMeter() const = 0;
 
 #ifdef BUILD_TESTS
     // Install a fake/assumed ledger version and bucket list hash to use in next

src/bucket/BucketManagerImpl.cpp

@@ -120,6 +120,8 @@ BucketManagerImpl::BucketManagerImpl(Application& app)
           {"bucketlistDB", "query", "loads"}, "query"))
     , mBucketListDBBloomMisses(app.getMetrics().NewMeter(
           {"bucketlistDB", "bloom", "misses"}, "bloom"))
+    , mBucketListDBBloomLookups(app.getMetrics().NewMeter(
+          {"bucketlistDB", "bloom", "lookups"}, "bloom"))
     // Minimal DB is stored in the buckets dir, so delete it only when
     // mode does not use minimal DB
     , mDeleteEntireBucketDirInDtor(
@@ -941,6 +943,12 @@ BucketManagerImpl::getBloomMissMeter() const
     return mBucketListDBBloomMisses;
 }
 
+medida::Meter&
+BucketManagerImpl::getBloomLookupMeter() const
+{
+    return mBucketListDBBloomLookups;
+}
+
 void
 BucketManagerImpl::calculateSkipValues(LedgerHeader& currentHeader)
 {

src/bucket/BucketManagerImpl.h

@@ -48,6 +48,7 @@ class BucketManagerImpl : public BucketManager
     medida::Counter& mSharedBucketsSize;
     medida::Meter& mBucketListDBQueryMeter;
     medida::Meter& mBucketListDBBloomMisses;
+    medida::Meter& mBucketListDBBloomLookups;
     mutable UnorderedMap<LedgerEntryType, medida::Timer&>
         mBucketListDBPointTimers{};
     mutable UnorderedMap<std::string, medida::Timer&> mBucketListDBBulkTimers{};
@@ -139,6 +140,7 @@ class BucketManagerImpl : public BucketManager
     std::vector<InflationWinner>
     loadInflationWinners(size_t maxWinners, int64_t minBalance) const override;
     medida::Meter& getBloomMissMeter() const override;
+    medida::Meter& getBloomLookupMeter() const override;
 
 #ifdef BUILD_TESTS
     // Install a fake/assumed ledger version and bucket list hash to use in next