MB-23829: Revert "MB-22695: Add stats for datatypes" 75/76575/3
authorDave Rigby <daver@couchbase.com>
Mon, 10 Apr 2017 17:45:26 +0000 (17:45 +0000)
committerDave Rigby <daver@couchbase.com>
Mon, 10 Apr 2017 19:50:18 +0000 (19:50 +0000)
Reverting to due null pointer dereference when replacing a non-resident item:

       297      if (v.getDatatype() != itm.getDataType()) {
    -> 298          --datatypeCounts[v.getValue()->getDataType()];
       299          ++datatypeCounts[itm.getDataType()];
       300      }
    (lldb) p v
    (StoredValue) $0 = {
      value = {
        value = 0x0000000000000000
      }

This reverts commit e4606e8f50797e40d3a9f7931c1e45a070f82002.

Change-Id: I5998e2eaadedf897192d0fb8aeb184ac85c4bf8f
Reviewed-on: http://review.couchbase.org/76575
Reviewed-by: Dave Rigby <daver@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>
src/hash_table.cc
src/hash_table.h
src/kv_bucket.cc
src/vb_count_visitor.cc
src/vb_count_visitor.h
tests/ep_testsuite.cc
tests/module_tests/stats_test.cc

index 4079e44..456c0ad 100644 (file)
@@ -46,7 +46,6 @@ HashTable::HashTable(EPStats& st,
                      size_t s, size_t l)
     : maxDeletedRevSeqno(0),
       numTotalItems(0),
-      datatypeCounts(),
       numNonResidentItems(0),
       numDeletedItems(0),
       numEjects(0),
@@ -106,7 +105,6 @@ void HashTable::clear(bool deactivate) {
 
     stats.currentSize.fetch_sub(clearedMemSize - clearedValSize);
 
-    datatypeCounts.fill(0);
     numTotalItems.store(0);
     numItems.store(0);
     numTempItems.store(0);
@@ -292,13 +290,6 @@ MutationStatus HashTable::unlocked_updateStoredValue(
         --numDeletedItems;
     }
 
-    // If the item we are replacing is resident then we need to make sure we
-    // appropriately alter the datatype stats.
-    if (v.getDatatype() != itm.getDataType()) {
-        --datatypeCounts[v.getValue()->getDataType()];
-        ++datatypeCounts[itm.getDataType()];
-    }
-
     /* setValue() will mark v as undeleted if required */
     v.setValue(itm, *this);
     return status;
@@ -325,7 +316,6 @@ StoredValue* HashTable::unlocked_addNewStoredValue(const HashBucketLock& hbl,
     } else {
         ++numItems;
         ++numTotalItems;
-        ++datatypeCounts[v->getDatatype()];
     }
     values[hbl.getBucketNum()] = std::move(v);
 
@@ -371,8 +361,6 @@ void HashTable::unlocked_softDelete(const std::unique_lock<std::mutex>& htLock,
         decrNumNonResidentItems();
     }
 
-    --datatypeCounts[v.getDatatype()];
-
     if (onlyMarkDeleted) {
         v.markDeleted();
     } else {
@@ -444,7 +432,6 @@ StoredValue::UniquePtr HashTable::unlocked_release(
     } else {
         decrNumItems();
         decrNumTotalItems();
-        --datatypeCounts[released->getDatatype()];
         if (released->isDeleted()) {
             --numDeletedItems;
         }
@@ -637,6 +624,7 @@ bool HashTable::unlocked_ejectItem(StoredValue*& vptr,
         throw std::invalid_argument("HashTable::unlocked_ejectItem: "
                 "Unable to delete NULL StoredValue");
     }
+
     if (policy == VALUE_ONLY) {
         bool rv = vptr->ejectValue(*this, policy);
         if (rv) {
@@ -668,7 +656,6 @@ bool HashTable::unlocked_ejectItem(StoredValue*& vptr,
                                            // fully evicted.
             }
             decrNumItems(); // Decrement because the item is fully evicted.
-            --datatypeCounts[vptr->getDatatype()];
             ++numEjects;
             updateMaxDeletedRevSeqno(vptr->getRevSeqno());
 
@@ -705,7 +692,6 @@ bool HashTable::unlocked_restoreValue(
         /* set it back to false as we created a temp item by setting it to true
            when bg fetch is scheduled (full eviction mode). */
         v.setNewCacheItem(false);
-        ++datatypeCounts[itm.getDataType()];
     } else {
         decrNumNonResidentItems();
     }
@@ -736,6 +722,5 @@ void HashTable::unlocked_restoreMeta(const std::unique_lock<std::mutex>& htLock,
         --numTempItems;
         ++numItems;
         ++numNonResidentItems;
-        ++datatypeCounts[v.getDatatype()];
     }
 }
index 54dafe7..55b9f58 100644 (file)
@@ -20,7 +20,6 @@
 #include "config.h"
 #include "storeddockey.h"
 #include "stored-value.h"
-#include <platform/non_negative_counter.h>
 
 class AbstractStoredValueFactory;
 class HashTableStatVisitor;
@@ -583,8 +582,6 @@ public:
 
     std::atomic<uint64_t>     maxDeletedRevSeqno;
     std::atomic<size_t>       numTotalItems;
-    std::array<cb::NonNegativeCounter<size_t>, mcbp::datatype::highest + 1>
-            datatypeCounts;
     cb::NonNegativeCounter<size_t> numNonResidentItems;
     cb::NonNegativeCounter<size_t> numDeletedItems;
     std::atomic<size_t>       numEjects;
index 41ccb6e..c9a8b79 100644 (file)
@@ -1334,18 +1334,6 @@ void KVBucket::appendAggregatedVBucketStats(VBucketCountVisitor& active,
             active.getTotalHLCDriftExceptionCounters().ahead +
                     replica.getTotalHLCDriftExceptionCounters().ahead);
 
-    for (uint8_t ii = 0; ii < active.getNumDatatypes(); ++ii) {
-        std::string name = "ep_active_datatype_";
-        name += mcbp::datatype::to_string(ii);
-        DO_STAT(name.c_str(), active.getDatatypeCount(ii));
-    }
-
-    for (uint8_t ii = 0; ii < replica.getNumDatatypes(); ++ii) {
-        std::string name = "ep_replica_datatype_";
-        name += mcbp::datatype::to_string(ii);
-        DO_STAT(name.c_str(), replica.getDatatypeCount(ii));
-    }
-
 #undef DO_STAT
 }
 
index ce30326..e392b47 100644 (file)
@@ -63,10 +63,5 @@ void VBucketCountVisitor::visitBucket(RCPtr<VBucket>& vb) {
         auto driftExceptionCounters = vb->getHLCDriftExceptionCounters();
         totalHLCDriftExceptionCounters.ahead += driftExceptionCounters.ahead;
         totalHLCDriftExceptionCounters.behind += driftExceptionCounters.behind;
-
-        // Iterate over each datatype combination
-        for (uint8_t ii = 0; ii < datatypeCounts.size(); ++ii) {
-            datatypeCounts[ii] += vb->ht.datatypeCounts[ii];
-        }
     }
 }
index 00a41d4..ad17de5 100644 (file)
@@ -54,7 +54,6 @@ public:
           backfillQueueSize(0),
           pendingWrites(0),
           chkPersistRemaining(0),
-          datatypeCounts{{0}},
           queueAge(0),
           rollbackItemCount(0),
           numHpVBReqs(0),
@@ -155,13 +154,6 @@ public:
         return chkPersistRemaining;
     }
 
-    size_t getDatatypeCount(protocol_binary_datatype_t datatype) const {
-        return datatypeCounts[datatype];
-    }
-    const size_t getNumDatatypes() const {
-        return datatypeCounts.size();
-    }
-
     uint64_t getRollbackItemCount() {
         return rollbackItemCount;
     }
@@ -205,7 +197,6 @@ private:
     size_t backfillQueueSize;
     size_t pendingWrites;
     size_t chkPersistRemaining;
-    std::array<size_t, mcbp::datatype::highest + 1> datatypeCounts;
     uint64_t queueAge;
     uint64_t rollbackItemCount;
     size_t numHpVBReqs;
index 149cccc..11176d3 100644 (file)
@@ -6521,14 +6521,6 @@ static enum test_result test_mb19687_fixed(ENGINE_HANDLE* h,
                 "ep_access_scanner_task_time",
                 "ep_active_ahead_exceptions",
                 "ep_active_behind_exceptions",
-                "ep_active_datatype_json",
-                "ep_active_datatype_json,xattr",
-                "ep_active_datatype_raw",
-                "ep_active_datatype_snappy",
-                "ep_active_datatype_snappy,json",
-                "ep_active_datatype_snappy,json,xattr",
-                "ep_active_datatype_snappy,xattr",
-                "ep_active_datatype_xattr",
                 "ep_active_hlc_drift",
                 "ep_active_hlc_drift_count",
                 "ep_alog_block_size",
@@ -6691,14 +6683,6 @@ static enum test_result test_mb19687_fixed(ENGINE_HANDLE* h,
                 "ep_queue_size",
                 "ep_replica_ahead_exceptions",
                 "ep_replica_behind_exceptions",
-                "ep_replica_datatype_json",
-                "ep_replica_datatype_json,xattr",
-                "ep_replica_datatype_raw",
-                "ep_replica_datatype_snappy",
-                "ep_replica_datatype_snappy,json",
-                "ep_replica_datatype_snappy,json,xattr",
-                "ep_replica_datatype_snappy,xattr",
-                "ep_replica_datatype_xattr",
                 "ep_replica_hlc_drift",
                 "ep_replica_hlc_drift_count",
                 "ep_replication_throttle_cap_pcnt",
index d811a47..c4717bd 100644 (file)
@@ -20,8 +20,6 @@
  */
 
 #include "stats_test.h"
-#include "evp_store_single_threaded_test.h"
-#include "test_helpers.h"
 
 #include <gmock/gmock.h>
 
@@ -59,15 +57,6 @@ std::map<std::string, std::string> StatTest::get_stat(const char* statkey) {
     return stats;
 }
 
-class DatatypeStatTest : public StatTest,
-                         public ::testing::WithParamInterface<std::string> {
-protected:
-    void SetUp() override {
-        config_string += std::string{"item_eviction_policy="} + GetParam();
-        StatTest::SetUp();
-    }
-};
-
 TEST_F(StatTest, vbucket_seqno_stats_test) {
     using namespace testing;
     const std::string vbucket = "vb_" + std::to_string(vbid);
@@ -82,187 +71,3 @@ TEST_F(StatTest, vbucket_seqno_stats_test) {
             Pair(vbucket + ":last_persisted_snap_start", "0"),
             Pair(vbucket + ":last_persisted_snap_end", "0")));
 }
-
-TEST_P(DatatypeStatTest, datatypesInitiallyZero) {
-    // Check that the datatype stats initialise to 0
-    auto vals = get_stat(nullptr);
-    EXPECT_EQ(0, std::stoi(vals["ep_active_datatype_snappy"]));
-    EXPECT_EQ(0, std::stoi(vals["ep_active_datatype_snappy,json"]));
-    EXPECT_EQ(0, std::stoi(vals["ep_active_datatype_snappy,xattr"]));
-    EXPECT_EQ(0, std::stoi(vals["ep_active_datatype_json"]));
-    EXPECT_EQ(0, std::stoi(vals["ep_active_datatype_json,xattr"]));
-    EXPECT_EQ(0, std::stoi(vals["ep_active_datatype_raw"]));
-    EXPECT_EQ(0, std::stoi(vals["ep_active_datatype_xattr"]));
-    EXPECT_EQ(0, std::stoi(vals["ep_active_datatype_snappy,json,xattr"]));
-
-    EXPECT_EQ(0, std::stoi(vals["ep_replica_datatype_snappy"]));
-    EXPECT_EQ(0, std::stoi(vals["ep_replica_datatype_snappy,json"]));
-    EXPECT_EQ(0, std::stoi(vals["ep_replica_datatype_snappy,xattr"]));
-    EXPECT_EQ(0, std::stoi(vals["ep_replica_datatype_json"]));
-    EXPECT_EQ(0, std::stoi(vals["ep_replica_datatype_json,xattr"]));
-    EXPECT_EQ(0, std::stoi(vals["ep_replica_datatype_raw"]));
-    EXPECT_EQ(0, std::stoi(vals["ep_replica_datatype_xattr"]));
-    EXPECT_EQ(0, std::stoi(vals["ep_replica_datatype_snappy,json,xattr"]));
-}
-
-void setDatatypeItem(KVBucket* store,
-                     const void* cookie,
-                     protocol_binary_datatype_t datatype,
-                     std::string name, std::string val = "[0]") {
-    Item item(make_item(
-            0, {name, DocNamespace::DefaultCollection}, val, 0, datatype));
-    store->set(item, cookie);
-}
-
-TEST_P(DatatypeStatTest, datatypeJsonToXattr) {
-    setDatatypeItem(store, cookie, PROTOCOL_BINARY_DATATYPE_JSON, "jsonDoc");
-    auto vals = get_stat(nullptr);
-    EXPECT_EQ(1, std::stoi(vals["ep_active_datatype_json"]));
-
-    // Check that updating an items datatype works
-    setDatatypeItem(store, cookie, PROTOCOL_BINARY_DATATYPE_XATTR, "jsonDoc");
-    vals = get_stat(nullptr);
-
-    EXPECT_EQ(1, std::stoi(vals["ep_active_datatype_xattr"]));
-    EXPECT_EQ(0, std::stoi(vals["ep_active_datatype_json"]));
-}
-
-TEST_P(DatatypeStatTest, datatypeRawStatTest) {
-    setDatatypeItem(store, cookie, 0, "rawDoc");
-    auto vals = get_stat(nullptr);
-    EXPECT_EQ(1, std::stoi(vals["ep_active_datatype_raw"]));
-}
-
-TEST_P(DatatypeStatTest, datatypeXattrStatTest) {
-    setDatatypeItem(store, cookie, PROTOCOL_BINARY_DATATYPE_XATTR, "xattrDoc");
-    auto vals = get_stat(nullptr);
-    EXPECT_EQ(1, std::stoi(vals["ep_active_datatype_xattr"]));
-    // Update the same key with a different value. The datatype stat should
-    // stay the same
-    setDatatypeItem(store, cookie, PROTOCOL_BINARY_DATATYPE_XATTR,
-                    "xattrDoc", "[2]");
-    vals = get_stat(nullptr);
-    EXPECT_EQ(1, std::stoi(vals["ep_active_datatype_xattr"]));
-}
-
-TEST_P(DatatypeStatTest, datatypeCompressedStatTest) {
-    setDatatypeItem(store,
-                    cookie,
-                    PROTOCOL_BINARY_DATATYPE_SNAPPY,
-                    "compressedDoc");
-    auto vals = get_stat(nullptr);
-    EXPECT_EQ(1, std::stoi(vals["ep_active_datatype_snappy"]));
-}
-
-TEST_P(DatatypeStatTest, datatypeCompressedJson) {
-    setDatatypeItem(
-            store,
-            cookie,
-            PROTOCOL_BINARY_DATATYPE_JSON | PROTOCOL_BINARY_DATATYPE_SNAPPY,
-            "jsonCompressedDoc");
-    auto vals = get_stat(nullptr);
-    EXPECT_EQ(1, std::stoi(vals["ep_active_datatype_snappy,json"]));
-}
-
-TEST_P(DatatypeStatTest, datatypeCompressedXattr) {
-    setDatatypeItem(store,
-                    cookie,
-                    PROTOCOL_BINARY_DATATYPE_XATTR |
-                            PROTOCOL_BINARY_DATATYPE_SNAPPY,
-                    "xattrCompressedDoc");
-    auto vals = get_stat(nullptr);
-    EXPECT_EQ(1, std::stoi(vals["ep_active_datatype_snappy,xattr"]));
-}
-
-TEST_P(DatatypeStatTest, datatypeJsonXattr) {
-    setDatatypeItem(
-            store,
-            cookie,
-            PROTOCOL_BINARY_DATATYPE_JSON | PROTOCOL_BINARY_DATATYPE_XATTR,
-            "jsonXattrDoc");
-    auto vals = get_stat(nullptr);
-    EXPECT_EQ(1, std::stoi(vals["ep_active_datatype_json,xattr"]));
-}
-
-TEST_P(DatatypeStatTest, datatypeDeletion) {
-    setDatatypeItem(
-            store,
-            cookie,
-            PROTOCOL_BINARY_DATATYPE_JSON | PROTOCOL_BINARY_DATATYPE_XATTR,
-            "jsonXattrDoc");
-    auto vals = get_stat(nullptr);
-    EXPECT_EQ(1, std::stoi(vals["ep_active_datatype_json,xattr"]));
-    uint64_t cas = 0;
-    store->deleteItem({"jsonXattrDoc", DocNamespace::DefaultCollection},
-                      cas,
-                      0,
-                      cookie,
-                      nullptr,
-                      nullptr,
-                      nullptr);
-    vals = get_stat(nullptr);
-    EXPECT_EQ(0, std::stoi(vals["ep_active_datatype_json,xattr"]));
-}
-
-TEST_P(DatatypeStatTest, datatypeCompressedJsonXattr) {
-    setDatatypeItem(store,
-                    cookie,
-                    PROTOCOL_BINARY_DATATYPE_JSON |
-                            PROTOCOL_BINARY_DATATYPE_SNAPPY |
-                            PROTOCOL_BINARY_DATATYPE_XATTR,
-                    "jsonCompressedXattrDoc");
-    auto vals = get_stat(nullptr);
-    EXPECT_EQ(1, std::stoi(vals["ep_active_datatype_snappy,json,xattr"]));
-}
-
-TEST_P(DatatypeStatTest, datatypeExpireItem) {
-    Item item(make_item(
-            0, {"expiryDoc", DocNamespace::DefaultCollection}, "[0]", 1,
-            PROTOCOL_BINARY_DATATYPE_JSON));
-    store->set(item, cookie);
-    store->get({"expiryDoc", DocNamespace::DefaultCollection}, 0, cookie, NONE);
-    auto vals = get_stat(nullptr);
-
-    //Should be 0, becuase the doc should have expired
-    EXPECT_EQ(0, std::stoi(vals["ep_active_datatype_json"]));
-}
-
-
-TEST_P(DatatypeStatTest, datatypeEviction) {
-    const DocKey key = {"jsonXattrDoc", DocNamespace::DefaultCollection};
-    setDatatypeItem(
-            store,
-            cookie,
-            PROTOCOL_BINARY_DATATYPE_JSON | PROTOCOL_BINARY_DATATYPE_XATTR,
-            "jsonXattrDoc");
-    auto vals = get_stat(nullptr);
-    EXPECT_EQ(1, std::stoi(vals["ep_active_datatype_json,xattr"]));
-    store->flushVBucket(0);
-    const char* msg;
-    store->evictKey(key, 0, &msg);
-    vals = get_stat(nullptr);
-    if (GetParam() == "value_only"){
-        // Should still be 1 as only value is evicted
-        EXPECT_EQ(1, std::stoi(vals["ep_active_datatype_json,xattr"]));
-    } else {
-        // Should be 0 as everything is evicted
-        EXPECT_EQ(0, std::stoi(vals["ep_active_datatype_json,xattr"]));
-    }
-
-    store->get(key, 0, cookie, QUEUE_BG_FETCH);
-    if (GetParam() == "full_eviction") {
-        // Run the bgfetch to restore the item from disk
-        ExTask task = new SingleBGFetcherTask(
-                engine.get(), key, 0, cookie, false, 0, false);
-        task_executor->schedule(task);
-        runNextTask(*task_executor->getLpTaskQ()[READER_TASK_IDX]);
-    }
-    vals = get_stat(nullptr);
-    // The item should be restored to memory, hence added back to the stats
-    EXPECT_EQ(1, std::stoi(vals["ep_active_datatype_json,xattr"]));
-}
-
-INSTANTIATE_TEST_CASE_P(FullAndValueEviction, DatatypeStatTest,
-                        ::testing::Values("value_only", "full_eviction"), []
-                                (const ::testing::TestParamInfo<std::string>&
-                                info) {return info.param;});