MB-40531: [BP] Change order of vbuckets paging visitor visits

Backport of http://review.couchbase.org/c/kv_engine/+/137340/

Testing after http://review.couchbase.org/c/kv_engine/+/133197
uncovered unexpected increases in cache miss ratio for a working set
which fits within the quota comfortably. This occurred after some time,
despite the workload reading the same keys in a loop.

Investigation discovered a large disparity in per-vbucket residency
(see MB for info).

Vbuckets with a lower vbid could be <20% resident while other vbuckets
remained fully resident. The fully resident vbs logically contain
resident cold data which has never been evicted despite never being
accessed.

This patch changes the order the paging visitor visits vbuckets. They
are now visited:

 * replicas before other vbucket states
 * from highest pageable memory to lowest (within a given vb state)

Testing demonstrates a lower standard deviation of the distribution of
per-vbucket residency ratios.

Change-Id: Ia8a0fa368821518a25659da03852076b70ac478a
Reviewed-on: http://review.couchbase.org/c/kv_engine/+/137666
Well-Formed: Build Bot <[email protected]>
Reviewed-by: Dave Rigby <[email protected]>
Tested-by: James Harrison <[email protected]>

Author: jameseh96

engines/ep/src/ep_vb.cc

@@ -448,6 +448,14 @@ bool EPVBucket::eligibleToPageOut(const HashTable::HashBucketLock& lh,
     return v.eligibleForEviction(eviction);
 }
 
+size_t EPVBucket::getPageableMemUsage() {
+    if (eviction == FULL_EVICTION) {
+        return ht.getItemMemory();
+    } else {
+        return ht.getItemMemory() - ht.getMetadataMemory();
+    }
+}
+
 void EPVBucket::queueBackfillItem(queued_item& qi,
                                   const GenerateBySeqno generateBySeqno) {
     LockHolder lh(backfill.mutex);

engines/ep/src/ep_vb.h

@@ -96,6 +96,8 @@ class EPVBucket : public VBucket {
     bool eligibleToPageOut(const HashTable::HashBucketLock& lh,
                            const StoredValue& v) const override;
 
+    size_t getPageableMemUsage() override;
+
     bool areDeletedItemsAlwaysResident() const override;
 
     void addStats(bool details, ADD_STAT add_stat, const void* c) override;

engines/ep/src/ephemeral_vb.cc

@@ -128,6 +128,17 @@ bool EphemeralVBucket::eligibleToPageOut(const HashTable::HashBucketLock& lh,
     return true;
 }
 
+size_t EphemeralVBucket::getPageableMemUsage() {
+    if (getState() == vbucket_state_replica) {
+        // Ephemeral buckets are not backed by disk - nothing can be evicted
+        // from a replica as deleting from replicas would cause inconsistency
+        // with the active. When the active vb evicts items deletions will be
+        // streamed to the replica.
+        return 0;
+    }
+    return ht.getItemMemory();
+}
+
 bool EphemeralVBucket::areDeletedItemsAlwaysResident() const {
     // Ephemeral buckets do keep all deleted items resident in memory.
     // (We have nowhere else to store them, given there is no disk).

engines/ep/src/ephemeral_vb.h

@@ -102,6 +102,8 @@ class EphemeralVBucket : public VBucket {
     bool eligibleToPageOut(const HashTable::HashBucketLock& lh,
                            const StoredValue& v) const override;
 
+    size_t getPageableMemUsage() override;
+
     bool areDeletedItemsAlwaysResident() const override;
 
     void addStats(bool details, ADD_STAT add_stat, const void* c) override;

engines/ep/src/fakes/fake_executorpool.h

@@ -184,8 +184,10 @@ class CheckedExecutor : public ExecutorThread {
         run();
     }
 
-    void runCurrentTask() {
+    std::string runCurrentTask() {
+        auto name = getTaskName();
         run();
+        return name;
     }
 
     ProcessClock::time_point completeCurrentTask() {

engines/ep/src/item_pager.cc

@@ -300,6 +300,33 @@ PagingVisitor::PagingVisitor(KVBucket& s,
         }
     }
 
+    std::function<bool(const VBucket::id_type&, const VBucket::id_type&)>
+    PagingVisitor::getVBucketComparator() const {
+        // Get the pageable mem used of each vb _once_ and cache it.
+        // Fetching these values repeatedly in the comparator could cause issues
+        // as the values can change _during_ a given sort call.
+
+        std::map<VBucket::id_type, size_t> pageableMemUsed;
+
+        for (const auto& vbid : store.getVBuckets().getBuckets()) {
+            auto vb = store.getVBucket(vbid);
+            pageableMemUsed[vbid] = vb ? vb->getPageableMemUsage() : 0;
+        }
+
+        // Capture initializers are C++14
+        return [pageableMemUsed, this](const VBucket::id_type& a,
+                                       const VBucket::id_type& b) mutable {
+            auto vbA = store.getVBucket(a);
+            auto vbB = store.getVBucket(b);
+            bool aReplica = vbA && vbA->getState() == vbucket_state_replica;
+            bool bReplica = vbB && vbB->getState() == vbucket_state_replica;
+            // sort replicas before all other vbucket states, then sort by
+            // pageableMemUsed
+            return std::make_pair(aReplica, pageableMemUsed[a]) >
+                   std::make_pair(bReplica, pageableMemUsed[b]);
+        };
+    }
+
     void PagingVisitor::update() {
         store.deleteExpiredItems(expired, ExpireBy::Pager);
 

engines/ep/src/paging_visitor.h

@@ -82,6 +82,9 @@ class PagingVisitor : public VBucketVisitor, public HashTableVisitor {
 
     void complete() override;
 
+    std::function<bool(const uint16_t&, const uint16_t&)> getVBucketComparator()
+            const override;
+
     /**
      * Get the number of items ejected during the visit.
      */

engines/ep/src/vbucket.h

@@ -1032,6 +1032,15 @@ class VBucket : public std::enable_shared_from_this<VBucket> {
     virtual bool eligibleToPageOut(const HashTable::HashBucketLock& lh,
                                    const StoredValue& v) const = 0;
 
+    /**
+     * Check how much memory could be reclaimed if every resident item
+     * were evicted.
+     * Note, the real amount of reclaimable memory will be lower than this,
+     * as certain items (prepares, dirty items, system events,
+     * deletes for ephemeral) cannot be evicted.
+     */
+    virtual size_t getPageableMemUsage() = 0;
+
     /**
      * Add an item in the store
      *

engines/ep/tests/mock/mock_paging_visitor.h

@@ -21,6 +21,8 @@
 
 #include <string>
 
+#include <gmock/gmock.h>
+
 /**
  * Mock PagingVisitor class.  Provide access to ItemEviction data structure.
  */
@@ -52,6 +54,11 @@ class MockPagingVisitor : public PagingVisitor {
                         agePercentage,
                         freqCounterAgeThreshold,
                         evictionPolicy) {
+        using namespace testing;
+        ON_CALL(*this, visitBucket(_))
+                .WillByDefault(Invoke([this](VBucketPtr& vb) {
+                    PagingVisitor::visitBucket(vb);
+                }));
     }
 
     ItemEviction& getItemEviction() {
@@ -69,4 +76,6 @@ class MockPagingVisitor : public PagingVisitor {
     void setCurrentBucket(VBucketPtr _currentBucket) {
         currentBucket = _currentBucket;
     }
+
+    MOCK_METHOD1(visitBucket, void(VBucketPtr&));
 };

engines/ep/tests/module_tests/evp_store_single_threaded_test.cc

@@ -54,12 +54,13 @@ ProcessClock::time_point SingleThreadedKVBucketTest::runNextTask(
     return executor.completeCurrentTask();
 }
 
-ProcessClock::time_point SingleThreadedKVBucketTest::runNextTask(TaskQueue& taskQ) {
+std::pair<ProcessClock::time_point, std::string>
+SingleThreadedKVBucketTest::runNextTask(TaskQueue& taskQ) {
     CheckedExecutor executor(task_executor, taskQ);
 
     // Run the task
-    executor.runCurrentTask();
-    return executor.completeCurrentTask();
+    auto name = executor.runCurrentTask();
+    return {executor.completeCurrentTask(), name};
 }
 
 void SingleThreadedKVBucketTest::SetUp() {