Refactor: Skip BloomFilter stats if non-existent

Magma buckets don't enable the VBucket level BloomFilter and cbcollects
for such buckets include the following superfluous output for every
bucket/vbucket which can be skipped.

 vb_0:bloom_filter:                         DOESN'T EXIST
 vb_0:bloom_filter_key_count:               0
 vb_0:bloom_filter_memory:                  0
 vb_0:bloom_filter_size:                    0
 vb_1:bloom_filter:                         DOESN'T EXIST
 vb_1:bloom_filter_key_count:               0
 vb_1:bloom_filter_memory:                  0
 vb_1:bloom_filter_size:                    0
 ...

Change-Id: I9504bb7bf9d3971fb6e058e4e2622657c3f8fcd0
Reviewed-on: https://review.couchbase.org/c/kv_engine/+/189743
Reviewed-by: Trond Norbye <[email protected]>
Tested-by: Jim Walker <[email protected]>

Author: jimwwalker

engines/ep/src/bloomfilter.cc

@@ -91,7 +91,7 @@ bfilter_status_t BloomFilter::getStatus() {
     return status;
 }
 
-std::string BloomFilter::getStatusString() {
+std::string BloomFilter::getStatusString() const {
     switch (status) {
         case BFILTER_DISABLED:
             return "DISABLED";
@@ -135,15 +135,15 @@ bool BloomFilter::maybeKeyExists(const DocKey& key) {
     return true;
 }
 
-size_t BloomFilter::getNumOfKeysInFilter() {
+size_t BloomFilter::getNumOfKeysInFilter() const {
     if (status == BFILTER_COMPACTING || status == BFILTER_ENABLED) {
         return keyCounter;
     } else {
         return 0;
     }
 }
 
-size_t BloomFilter::getFilterSize() {
+size_t BloomFilter::getFilterSize() const {
     if (status == BFILTER_COMPACTING || status == BFILTER_ENABLED) {
         return filterSize;
     } else {

engines/ep/src/bloomfilter.h

@@ -36,15 +36,15 @@ class BloomFilter {
 
     void setStatus(bfilter_status_t to);
     bfilter_status_t getStatus();
-    std::string getStatusString();
+    std::string getStatusString() const;
 
     void addKey(const DocKey& key);
     bool maybeKeyExists(const DocKey& key);
 
-    size_t getNumOfKeysInFilter();
+    size_t getNumOfKeysInFilter() const;
 
     /// @returns the filter size (in bits).
-    size_t getFilterSize();
+    size_t getFilterSize() const;
     size_t getNoOfHashes() const;
 
     /// @returns the filter memory footprint in bytes.

engines/ep/src/vbucket.cc

@@ -1290,6 +1290,27 @@ size_t VBucket::getFilterMemoryFootprint() {
     return memFootprint;
 }
 
+void VBucket::addBloomFilterStats(const AddStatFn& add_stat, CookieIface& c) {
+    std::lock_guard<std::mutex> lh(bfMutex);
+    if (bFilter) {
+        addBloomFilterStats_UNLOCKED(add_stat, c, *bFilter);
+    } else if (tempFilter) {
+        addBloomFilterStats_UNLOCKED(add_stat, c, *tempFilter);
+    }
+}
+
+void VBucket::addBloomFilterStats_UNLOCKED(const AddStatFn& add_stat,
+                                           CookieIface& c,
+                                           const BloomFilter& filter) {
+    addStat("bloom_filter", filter.getStatusString(), add_stat, c);
+    addStat("bloom_filter_size", filter.getFilterSize(), add_stat, c);
+    addStat("bloom_filter_key_count",
+            filter.getNumOfKeysInFilter(),
+            add_stat,
+            c);
+    addStat("bloom_filter_memory", filter.getMemoryFootprint(), add_stat, c);
+}
+
 VBNotifyCtx VBucket::queueItem(queued_item& item, const VBQueueItemCtx& ctx) {
     // Set queue time to now. Why not in the ctor of the Item? We only need to
     // do this in certain places for new items as it's used to determine how
@@ -3323,10 +3344,7 @@ void VBucket::_addStats(VBucketStatsDetailLevel detail,
 
         addStat("uuid", failovers->getLatestUUID(), add_stat, c);
         addStat("purge_seqno", getPurgeSeqno(), add_stat, c);
-        addStat("bloom_filter", getFilterStatusString().data(), add_stat, c);
-        addStat("bloom_filter_size", getFilterSize(), add_stat, c);
-        addStat("bloom_filter_key_count", getNumOfKeysInFilter(), add_stat, c);
-        addStat("bloom_filter_memory", getFilterMemoryFootprint(), add_stat, c);
+        addBloomFilterStats(add_stat, c);
         addStat("rollback_item_count", getRollbackItemCount(), add_stat, c);
         addStat("hp_vb_req_size", getHighPriorityChkSize(), add_stat, c);
         addStat("might_contain_xattrs", mightContainXattrs(), add_stat, c);

engines/ep/src/vbucket.h

@@ -2181,6 +2181,13 @@ class VBucket : public std::enable_shared_from_this<VBucket> {
                  const AddStatFn& add_stat,
                  CookieIface& c);
 
+    // helper function to add stats for the current bloom filter
+    void addBloomFilterStats(const AddStatFn& add_stat, CookieIface& c);
+
+    void addBloomFilterStats_UNLOCKED(const AddStatFn& add_stat,
+                                      CookieIface& c,
+                                      const BloomFilter& filter);
+
     /* This member holds the eviction policy used */
     const EvictionPolicy eviction;
 

engines/ep/tests/ep_testsuite.cc

@@ -6289,10 +6289,6 @@ static enum test_result test_mb19687_fixed(EngineIface* h) {
             {"vbucket", {"vb_0"}},
             {"vbucket-details 0",
              {"vb_0",
-              "vb_0:bloom_filter",
-              "vb_0:bloom_filter_key_count",
-              "vb_0:bloom_filter_memory",
-              "vb_0:bloom_filter_size",
               "vb_0:drift_ahead_threshold",
               "vb_0:drift_ahead_threshold_exceeded",
               "vb_0:drift_behind_threshold",
@@ -7280,6 +7276,15 @@ static enum test_result test_mb19687_fixed(EngineIface* h) {
                           "ep_magma_key_tree_data_block_size",
                           "ep_magma_key_tree_index_block_size"});
         }
+    } else if (isPersistentBucket(h)) {
+        // bloom filter stats only appear when enabled (currently all persistent
+        // except magma)
+        auto& vb_details = statsKeys.at("vbucket-details 0");
+        vb_details.insert(vb_details.end(),
+                          {"vb_0:bloom_filter",
+                           "vb_0:bloom_filter_key_count",
+                           "vb_0:bloom_filter_memory",
+                           "vb_0:bloom_filter_size"});
     }
 
     if (isEphemeralBucket(h)) {