MB-31266: Count system events

Update the hash-table stats to include items which
are system-event items, for ephemeral buckets this
enables us to provide an more accurate curr_items
statistic which is not inflated by the system
events.

A deleted system event (tombstone) is still tracked
as a system event, no differentiation of alive/deleted.

For persistent buckets more work is needed and is tracked
as MB-26334.

Change-Id: I8ae4d37f1c4b96c4d7c1edc059ea921e17a0ad5a
Reviewed-on: http://review.couchbase.org/103121
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>

engines/ep/src/ep_engine.cc

@@ -3152,6 +3152,11 @@ ENGINE_ERROR_CODE EventuallyPersistentEngine::doHashStats(const void *cookie,
                 checked_snprintf(
                         buf, sizeof(buf), "vb_%d:mem_size_counted", vbid.get());
                 add_casted_stat(buf, depthVisitor.memUsed, add_stat, cookie);
+
+                checked_snprintf(
+                        buf, sizeof(buf), "vb_%d:num_system_items", vbid.get());
+                add_casted_stat(
+                        buf, vb->ht.getNumSystemItems(), add_stat, cookie);
             } catch (std::exception& error) {
                 EP_LOG_WARN(
                         "StatVBucketVisitor::visitBucket: Failed to build "

engines/ep/src/ep_vb.cc

@@ -267,7 +267,8 @@ void EPVBucket::notifyAllPendingConnsFailed(EventuallyPersistentEngine& e) {
 
 size_t EPVBucket::getNumItems() const {
     if (eviction == VALUE_ONLY) {
-        return ht.getNumInMemoryItems() - ht.getNumDeletedItems();
+        return ht.getNumInMemoryItems() -
+               (ht.getNumDeletedItems() + ht.getNumSystemItems());
     } else {
         return onDiskTotalItems;
     }
@@ -300,6 +301,11 @@ size_t EPVBucket::getNumNonResidentItems() const {
     }
 }
 
+size_t EPVBucket::getNumSystemItems() const {
+    // @todo: MB-26334 need to track system counts for persistent buckets
+    return 0;
+}
+
 ENGINE_ERROR_CODE EPVBucket::statsVKey(const DocKey& key,
                                        const void* cookie,
                                        EventuallyPersistentEngine& engine) {

engines/ep/src/ep_vb.h

@@ -85,6 +85,8 @@ class EPVBucket : public VBucket {
 
     size_t getNumNonResidentItems() const override;
 
+    size_t getNumSystemItems() const override;
+
     ENGINE_ERROR_CODE statsVKey(const DocKey& key,
                                 const void* cookie,
                                 EventuallyPersistentEngine& engine) override;

engines/ep/src/ephemeral_vb.cc

@@ -78,7 +78,12 @@ EphemeralVBucket::EphemeralVBucket(
 }
 
 size_t EphemeralVBucket::getNumItems() const {
-    return ht.getNumInMemoryItems() - ht.getNumDeletedItems();
+    return ht.getNumInMemoryItems() -
+           (ht.getNumDeletedItems() + ht.getNumSystemItems());
+}
+
+size_t EphemeralVBucket::getNumSystemItems() const {
+    return ht.getNumSystemItems();
 }
 
 void EphemeralVBucket::completeStatsVKey(const DocKey& key,

engines/ep/src/ephemeral_vb.h

@@ -78,6 +78,8 @@ class EphemeralVBucket : public VBucket {
         return 0;
     }
 
+    size_t getNumSystemItems() const override;
+
     ENGINE_ERROR_CODE statsVKey(const DocKey& key,
                                 const void* cookie,
                                 EventuallyPersistentEngine& engine) override {

engines/ep/src/hash_table.cc

@@ -418,6 +418,7 @@ HashTable::Statistics::StoredValueProperties::StoredValueProperties(
     isResident = sv->isResident();
     isDeleted = sv->isDeleted();
     isTempItem = sv->isTempItem();
+    isSystemItem = sv->getKey().getCollectionID().isSystem();
 }
 
 HashTable::Statistics::StoredValueProperties HashTable::Statistics::prologue(
@@ -469,7 +470,13 @@ void HashTable::Statistics::epilogue(StoredValueProperties pre,
         numItems.fetch_add(postNonTemp - preNonTemp);
     }
 
-    if (pre.isDeleted != post.isDeleted) {
+    if (pre.isSystemItem != post.isSystemItem) {
+        numSystemItems.fetch_add(post.isSystemItem - pre.isSystemItem);
+    }
+
+    // Don't include system items in the deleted count, numSystemItems will
+    // count both types (a marked deleted system event still has purpose)
+    if (pre.isDeleted != post.isDeleted && !post.isSystemItem) {
         numDeletedItems.fetch_add(post.isDeleted - pre.isDeleted);
     }
 
@@ -958,6 +965,7 @@ std::ostream& operator<<(std::ostream& os, const HashTable& ht) {
        << " numDeleted:" << ht.getNumDeletedItems()
        << " numNonResident:" << ht.getNumInMemoryNonResItems()
        << " numTemp:" << ht.getNumTempItems()
+       << " numSystemItems:" << ht.getNumSystemItems()
        << " values: " << std::endl;
     for (const auto& chain : ht.values) {
         if (chain) {

engines/ep/src/hash_table.h

@@ -246,6 +246,7 @@ class HashTable {
             bool isResident = false;
             bool isDeleted = false;
             bool isTempItem = false;
+            bool isSystemItem = false;
         };
 
         /**
@@ -305,6 +306,10 @@ class HashTable {
             return numTempItems;
         }
 
+        size_t getNumSystemItems() const {
+            return numSystemItems;
+        }
+
         DatatypeCombo getDatatypeCounts() const {
             return datatypeCounts;
         }
@@ -339,6 +344,9 @@ class HashTable {
         /// Count of items where StoredValue::isTempItem() is true.
         cb::NonNegativeCounter<size_t> numTempItems;
 
+        /// Count of items where StoredValue resides in system namespace
+        cb::NonNegativeCounter<size_t> numSystemItems;
+
         /**
          * Number of documents of a given datatype. Includes alive
          * (non-deleted), documents in the HashTable.
@@ -466,6 +474,10 @@ class HashTable {
         return valueStats.getNumItems();
     }
 
+    size_t getNumSystemItems() const {
+        return valueStats.getNumSystemItems();
+    }
+
     /**
      * Sets the frequencyCounterSaturated function to the callback function
      * passed in.

engines/ep/src/vbucket.h

@@ -633,6 +633,11 @@ class VBucket : public std::enable_shared_from_this<VBucket> {
         return ht.getNumTempItems();
     }
 
+    /**
+     * @returns the number of system items stored in this vbucket
+     */
+    virtual size_t getNumSystemItems() const = 0;
+
     void incrRollbackItemCount(uint64_t val) {
         rollbackItemCount.fetch_add(val, std::memory_order_relaxed);
     }

engines/ep/tests/ep_testsuite.cc

@@ -6552,6 +6552,7 @@ static enum test_result test_mb19687_fixed(EngineIface* h) {
               "vb_0:mem_size",
               "vb_0:mem_size_counted",
               "vb_0:min_depth",
+              "vb_0:num_system_items",
               "vb_0:reported",
               "vb_0:resized",
               "vb_0:size",

engines/ep/tests/module_tests/collections/evp_store_collections_dcp_test.cc

@@ -118,6 +118,11 @@ TEST_P(CollectionsDcpParameterizedTest, test_dcp) {
     CollectionsManifest cm(CollectionEntry::meat);
     vb->updateFromManifest({cm});
 
+    // @todo MB-26334: persistent buckets don't track the system event counts
+    if (!persistent()) {
+        EXPECT_EQ(1, vb->getNumSystemItems());
+    }
+
     notifyAndStepToCheckpoint();
 
     VBucketPtr replica = store->getVBucket(replicaVB);
@@ -141,6 +146,11 @@ TEST_P(CollectionsDcpParameterizedTest, test_dcp) {
     // remove meat
     vb->updateFromManifest({cm.remove(CollectionEntry::meat)});
 
+    // The delete collection event still exists
+    if (!persistent()) {
+        EXPECT_EQ(1, vb->getNumSystemItems());
+    }
+
     notifyAndStepToCheckpoint();
 
     // Now step the producer to transfer the collection deletion

engines/ep/tests/module_tests/collections/evp_store_collections_test.cc

@@ -110,6 +110,17 @@ TEST_P(CollectionsParameterizedTest, collections_basic) {
     CollectionsManifest cm(CollectionEntry::meat);
     vb->updateFromManifest({cm});
 
+    // System event not counted
+    // Note: for persistent buckets, that is because
+    // 1) It doesn't go in the hash-table
+    // 2) It will only be accounted for on Full-Evict buckets after flush
+    EXPECT_EQ(1, vb->getNumItems());
+
+    // @todo MB-26334: persistent buckets don't track the system event counts
+    if (!persistent()) {
+        EXPECT_EQ(1, vb->getNumSystemItems());
+    }
+
     // Trigger a flush to disk. Flushes the meat create event and 1 item
     flushVBucketToDiskIfPersistent(vbid, 2);
 

engines/ep/tests/module_tests/hash_table_test.cc

@@ -359,6 +359,7 @@ class HashTableStatsTest : public HashTableTest,
         EXPECT_EQ(0, ht.getNumInMemoryNonResItems());
         EXPECT_EQ(0, ht.getNumTempItems());
         EXPECT_EQ(0, ht.getNumDeletedItems());
+        EXPECT_EQ(0, ht.getNumSystemItems());
         for (const auto& count : ht.getDatatypeCounts()) {
             EXPECT_EQ(0, count);
         }
@@ -378,6 +379,7 @@ class HashTableStatsTest : public HashTableTest,
         EXPECT_EQ(0, ht.getNumInMemoryItems());
         EXPECT_EQ(0, ht.getNumTempItems());
         EXPECT_EQ(0, ht.getNumDeletedItems());
+        EXPECT_EQ(0, ht.getNumSystemItems());
         for (const auto& datatypeCount : ht.getDatatypeCounts()) {
             EXPECT_EQ(0, datatypeCount);
         }
@@ -1044,3 +1046,12 @@ TEST_F(HashTableTest, ItemFreqDecayerVisitorTest) {
            EXPECT_EQ(expectVal, v->getFreqCounterValue());
        }
 }
+
+TEST_F(HashTableTest, SystemEventItem) {
+    HashTable ht(global_stats, makeFactory(true), 128, 1);
+    StoredDocKey key("key", CollectionID::System);
+    store(ht, key);
+    EXPECT_EQ(1, ht.getNumSystemItems());
+    EXPECT_TRUE(del(ht, key));
+    EXPECT_EQ(0, ht.getNumSystemItems());
+}