MB-58576 [1/2]: Fix memory tracking in StrictQuotaItemPager

StrictQuotaItemPager - as introduced via MB-53053 for cross-Bucket
frequency counter based eviction (specifically
https://review.couchbase.org/c/kv_engine/+/183324) - constructs its
base class GlobalTask via the ctor taking a Taskable, not the ctor
taking a EventuallyPersistentEngine.

This is problematic as when the task runs, the execution pools do not
switch to the current engine (as GlobalTask::engine is null) - and as
a result any (de)allocations made - say during paging - are not
correctly accounted to the associated bucket - they end up being
accounted to the 'NoBucket'.

The bug is somewhat subtle, as ItemPager has it's /own/ 'engine'
member variable (which is initialised correctly), however given this
shadows the (null) base class one which is used by the ExecutorPool to
setup the current engine, we end up with switched to a null engine.

Fix by adding an ItemPager ctor taking an Engine and using that for
StrictQuotaItemPager.

Change-Id: I4e13d547e4e21f1e67e65bbe06d41f05bbc11ac9
Reviewed-on: https://review.couchbase.org/c/kv_engine/+/196871
Reviewed-by: Trond Norbye <[email protected]>
Tested-by: Build Bot <[email protected]>

Author: daverigby

engines/ep/src/item_pager.cc

@@ -58,6 +58,13 @@ ItemPager::ItemPager(Taskable& t,
       sleepTime(sleepTime) {
 }
 
+ItemPager::ItemPager(EventuallyPersistentEngine& engine,
+                     size_t numConcurrentPagers,
+                     std::chrono::milliseconds sleepTime)
+    : ItemPager(engine.getTaskable(), numConcurrentPagers, sleepTime) {
+    GlobalTask::engine = &engine;
+}
+
 std::optional<VBucketFilter> ItemPager::createVBucketFilter(
         KVBucket& kvBucket, PermittedVBStates acceptedStates) {
     VBucketFilter filter;
@@ -93,11 +100,10 @@ PermittedVBStates ItemPager::getStatesForEviction(EvictionRatios ratios) const {
 StrictQuotaItemPager::StrictQuotaItemPager(EventuallyPersistentEngine& e,
                                            EPStats& st,
                                            size_t numConcurrentPagers)
-    : ItemPager(e.getTaskable(),
+    : ItemPager(e,
                 numConcurrentPagers,
                 std::chrono::milliseconds(
                         e.getConfiguration().getPagerSleepTimeMs())),
-      engine(e),
       stats(st) {
 }
 
@@ -139,7 +145,7 @@ EvictionRatios StrictQuotaItemPager::getEvictionRatios(
 }
 
 ItemPager::PageableMemInfo StrictQuotaItemPager::getPageableMemInfo() const {
-    KVBucket* kvBucket = engine.getKVBucket();
+    KVBucket* kvBucket = engine->getKVBucket();
     auto current = kvBucket->getPageableMemCurrent();
     auto upper = kvBucket->getPageableMemHighWatermark();
     auto lower = kvBucket->getPageableMemLowWatermark();
@@ -183,15 +189,15 @@ bool ItemPager::runInner(bool manuallyNotified) {
 std::function<std::unique_ptr<ItemEvictionStrategy>()>
 StrictQuotaItemPager::getEvictionStrategyFactory(
         EvictionRatios evictionRatios) {
-    const auto& cfg = engine.getConfiguration();
+    const auto& cfg = engine->getConfiguration();
 
     auto strategy = cfg.getItemEvictionStrategy();
 
     if (strategy == "upfront_mfu_only") {
         MFUHistogram activePendingMFUHist;
         MFUHistogram replicaMFUHist;
 
-        KVBucket* kvBucket = engine.getKVBucket();
+        KVBucket* kvBucket = engine->getKVBucket();
         for (auto& vbucket : kvBucket->getVBuckets()) {
             switch (vbucket.getState()) {
             case vbucket_state_active:
@@ -278,7 +284,7 @@ size_t ItemPager::getEvictableBytes(KVBucket& kvBucket,
 }
 
 void StrictQuotaItemPager::schedulePagingVisitors(std::size_t bytesToEvict) {
-    auto* kvBucket = engine.getKVBucket();
+    auto* kvBucket = engine->getKVBucket();
     if (kvBucket->getItemEvictionPolicy() == EvictionPolicy::Value) {
         doEvict = true;
     }

engines/ep/src/item_pager.h

@@ -42,10 +42,16 @@ class Semaphore;
  */
 class ItemPager : public NotifiableTask {
 public:
+    // ctor when ItemPager is not associated with a specific bucket.
     ItemPager(Taskable& t,
               size_t numConcurrentPagers,
               std::chrono::milliseconds sleepTime);
 
+    // ctor when Item pager is associated with a specific bucket.
+    ItemPager(EventuallyPersistentEngine& engine,
+              size_t numConcurrentPagers,
+              std::chrono::milliseconds sleepTime);
+
     bool runInner(bool manuallyNotified) override;
 
     std::chrono::microseconds getSleepTime() const override {
@@ -170,7 +176,6 @@ class StrictQuotaItemPager : public ItemPager {
      */
     void resetPhase();
 
-    EventuallyPersistentEngine& engine;
     EPStats& stats;
 };