Add total bucket counter to NodeMaintenanceStats.

7 files changed, 54 insertions, 22 deletions

diff --git a/storage/src/tests/distributor/distributortest.cpp b/storage/src/tests/distributor/distributortest.cpp
index cc1c9df0509..33549226b98 100644
--- a/storage/src/tests/distributor/distributortest.cpp
+++ b/storage/src/tests/distributor/distributortest.cpp
@@ -98,7 +98,7 @@ protected:
     void entering_recovery_mode_resets_bucket_space_stats();
     void leaving_recovery_mode_immediately_sends_getnodestate_replies();
 
-    void assertBucketSpaceStats(size_t expBucketPending, uint16_t node, const vespalib::string &bucketSpace,
+    void assertBucketSpaceStats(size_t expBucketPending, size_t expBucketTotal, uint16_t node, const vespalib::string &bucketSpace,
                                 const BucketSpacesStatsProvider::PerNodeBucketSpacesStats &stats);
     std::vector<document::BucketSpace> _bucketSpaces;
 
@@ -639,28 +639,31 @@ Distributor_Test::mergeStatsAreAccumulatedDuringDatabaseIteration()
         NodeMaintenanceStats wanted;
         wanted.syncing = 1;
         wanted.copyingOut = 2;
+        wanted.total = 3;
         CPPUNIT_ASSERT_EQUAL(wanted, stats.perNodeStats.forNode(0, makeBucketSpace()));
     }
     {
         NodeMaintenanceStats wanted;
         wanted.movingOut = 1;
+        wanted.total = 1;
         CPPUNIT_ASSERT_EQUAL(wanted, stats.perNodeStats.forNode(1, makeBucketSpace()));
     }
     {
         NodeMaintenanceStats wanted;
         wanted.syncing = 1;
         wanted.copyingIn = 2;
+        wanted.total = 1;
         CPPUNIT_ASSERT_EQUAL(wanted, stats.perNodeStats.forNode(2, makeBucketSpace()));
     }
     auto bucketStats = _distributor->getBucketSpacesStats();
     CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(3), bucketStats.size());
-    assertBucketSpaceStats(1, 0, "default", bucketStats);
-    assertBucketSpaceStats(0, 1, "default", bucketStats);
-    assertBucketSpaceStats(3, 2, "default", bucketStats);
+    assertBucketSpaceStats(1, 3, 0, "default", bucketStats);
+    assertBucketSpaceStats(0, 1, 1, "default", bucketStats);
+    assertBucketSpaceStats(3, 1, 2, "default", bucketStats);
 }
 
 void
-Distributor_Test::assertBucketSpaceStats(size_t expBucketPending, uint16_t node, const vespalib::string &bucketSpace,
+Distributor_Test::assertBucketSpaceStats(size_t expBucketPending, size_t expBucketTotal, uint16_t node, const vespalib::string &bucketSpace,
                                          const BucketSpacesStatsProvider::PerNodeBucketSpacesStats &stats)
 {
     auto nodeItr = stats.find(node);
@@ -669,7 +672,7 @@ Distributor_Test::assertBucketSpaceStats(size_t expBucketPending, uint16_t node,
     auto bucketSpaceItr = nodeItr->second.find(bucketSpace);
     CPPUNIT_ASSERT(bucketSpaceItr != nodeItr->second.end());
     CPPUNIT_ASSERT(bucketSpaceItr->second.valid());
-    CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(0), bucketSpaceItr->second.bucketsTotal());
+    CPPUNIT_ASSERT_EQUAL(expBucketTotal, bucketSpaceItr->second.bucketsTotal());
     CPPUNIT_ASSERT_EQUAL(expBucketPending, bucketSpaceItr->second.bucketsPending());
 }
 
@@ -694,11 +697,13 @@ Distributor_Test::statsGeneratedForPreemptedOperations()
     {
         NodeMaintenanceStats wanted;
         wanted.syncing = 1;
+        wanted.total = 1;
         CPPUNIT_ASSERT_EQUAL(wanted, stats.perNodeStats.forNode(0, makeBucketSpace()));
     }
     {
         NodeMaintenanceStats wanted;
         wanted.syncing = 1;
+        wanted.total = 1;
         CPPUNIT_ASSERT_EQUAL(wanted, stats.perNodeStats.forNode(1, makeBucketSpace()));
     }
 }
diff --git a/storage/src/tests/distributor/nodemaintenancestatstrackertest.cpp b/storage/src/tests/distributor/nodemaintenancestatstrackertest.cpp
index 5663db78e12..ae0d0bc4478 100644
--- a/storage/src/tests/distributor/nodemaintenancestatstrackertest.cpp
+++ b/storage/src/tests/distributor/nodemaintenancestatstrackertest.cpp
@@ -26,7 +26,7 @@ class NodeMaintenanceStatsTrackerTest : public CppUnit::TestFixture
     void statsAreTrackedPerNode();
     void statsAreTrackedPerBucketSpace();
     void assertEmptyBucketStats(BucketSpace bucketSpace, const NodeMaintenanceStatsTracker& tracker);
-    void assertBucketStats(uint64_t expMovingOut, uint64_t expSyncing, uint64_t expCopyingIn, uint64_t expCopyingOut,
+    void assertBucketStats(uint64_t expMovingOut, uint64_t expSyncing, uint64_t expCopyingIn, uint64_t expCopyingOut, uint64_t expTotal,
                            BucketSpace bucketSpace, const NodeMaintenanceStatsTracker& tracker);
 };
 
@@ -112,25 +112,30 @@ NodeMaintenanceStatsTrackerTest::statsAreTrackedPerBucketSpace()
     BucketSpace fooSpace(3);
     BucketSpace barSpace(5);
 
+    tracker.incTotal(0, fooSpace);
     tracker.incMovingOut(0, fooSpace);
-    assertBucketStats(1, 0, 0, 0, fooSpace, tracker);
+    assertBucketStats(1, 0, 0, 0, 1, fooSpace, tracker);
     assertEmptyBucketStats(barSpace, tracker);
 
+    tracker.incTotal(0, barSpace);
     tracker.incMovingOut(0, barSpace);
-    assertBucketStats(1, 0, 0, 0, fooSpace, tracker);
-    assertBucketStats(1, 0, 0, 0, barSpace, tracker);
+    assertBucketStats(1, 0, 0, 0, 1, fooSpace, tracker);
+    assertBucketStats(1, 0, 0, 0, 1, barSpace, tracker);
 
+    tracker.incTotal(0, fooSpace);
     tracker.incSyncing(0, fooSpace);
-    assertBucketStats(1, 1, 0, 0, fooSpace, tracker);
-    assertBucketStats(1, 0, 0, 0, barSpace, tracker);
+    assertBucketStats(1, 1, 0, 0, 2, fooSpace, tracker);
+    assertBucketStats(1, 0, 0, 0, 1, barSpace, tracker);
 
+    tracker.incTotal(0, fooSpace);
     tracker.incCopyingIn(0, fooSpace);
-    assertBucketStats(1, 1, 1, 0, fooSpace, tracker);
-    assertBucketStats(1, 0, 0, 0, barSpace, tracker);
+    assertBucketStats(1, 1, 1, 0, 3, fooSpace, tracker);
+    assertBucketStats(1, 0, 0, 0, 1, barSpace, tracker);
 
+    tracker.incTotal(0, fooSpace);
     tracker.incCopyingOut(0, fooSpace);
-    assertBucketStats(1, 1, 1, 1, fooSpace, tracker);
-    assertBucketStats(1, 0, 0, 0, barSpace, tracker);
+    assertBucketStats(1, 1, 1, 1, 4, fooSpace, tracker);
+    assertBucketStats(1, 0, 0, 0, 1, barSpace, tracker);
 }
 
 void
@@ -146,10 +151,11 @@ NodeMaintenanceStatsTrackerTest::assertBucketStats(uint64_t expMovingOut,
                                                    uint64_t expSyncing,
                                                    uint64_t expCopyingIn,
                                                    uint64_t expCopyingOut,
+                                                   uint64_t expTotal,
                                                    BucketSpace bucketSpace,
                                                    const NodeMaintenanceStatsTracker& tracker)
 {
-    NodeMaintenanceStats expStats(expMovingOut, expSyncing, expCopyingIn, expCopyingOut);
+    NodeMaintenanceStats expStats(expMovingOut, expSyncing, expCopyingIn, expCopyingOut, expTotal);
     CPPUNIT_ASSERT_EQUAL(expStats, tracker.forNode(0, bucketSpace));
 }
 
diff --git a/storage/src/vespa/storage/distributor/distributor.cpp b/storage/src/vespa/storage/distributor/distributor.cpp
index 28b9fb7f0ef..f09dce2a41e 100644
--- a/storage/src/vespa/storage/distributor/distributor.cpp
+++ b/storage/src/vespa/storage/distributor/distributor.cpp
@@ -692,7 +692,7 @@ namespace {
 BucketSpaceStats
 toBucketSpaceStats(const NodeMaintenanceStats &stats)
 {
-    return BucketSpaceStats(0, stats.syncing + stats.copyingIn);
+    return BucketSpaceStats(stats.total, stats.syncing + stats.copyingIn);
 }
 
 BucketSpacesStatsProvider::PerNodeBucketSpacesStats
diff --git a/storage/src/vespa/storage/distributor/maintenance/node_maintenance_stats_tracker.cpp b/storage/src/vespa/storage/distributor/maintenance/node_maintenance_stats_tracker.cpp
index 3a65d2d5336..9d999681407 100644
--- a/storage/src/vespa/storage/distributor/maintenance/node_maintenance_stats_tracker.cpp
+++ b/storage/src/vespa/storage/distributor/maintenance/node_maintenance_stats_tracker.cpp
@@ -15,6 +15,7 @@ operator<<(std::ostream& os, const NodeMaintenanceStats& stats)
        << ",syncing="    << stats.syncing
        << ",copyingIn="  << stats.copyingIn
        << ",copyingOut=" << stats.copyingOut
+       << ",total="      << stats.total
        << ")";
     return os;
 }
diff --git a/storage/src/vespa/storage/distributor/maintenance/node_maintenance_stats_tracker.h b/storage/src/vespa/storage/distributor/maintenance/node_maintenance_stats_tracker.h
index 891e6fd7fa7..11f7b53ba20 100644
--- a/storage/src/vespa/storage/distributor/maintenance/node_maintenance_stats_tracker.h
+++ b/storage/src/vespa/storage/distributor/maintenance/node_maintenance_stats_tracker.h
@@ -15,20 +15,22 @@ struct NodeMaintenanceStats
     uint64_t syncing;
     uint64_t copyingIn;
     uint64_t copyingOut;
+    uint64_t total;
 
     NodeMaintenanceStats()
-        : movingOut(0), syncing(0), copyingIn(0), copyingOut(0)
+        : movingOut(0), syncing(0), copyingIn(0), copyingOut(0), total(0)
     {}
 
-    NodeMaintenanceStats(uint64_t movingOut_, uint64_t syncing_, uint64_t copyingIn_, uint64_t copyingOut_)
-        : movingOut(movingOut_), syncing(syncing_), copyingIn(copyingIn_), copyingOut(copyingOut_)
+    NodeMaintenanceStats(uint64_t movingOut_, uint64_t syncing_, uint64_t copyingIn_, uint64_t copyingOut_, uint64_t total_)
+        : movingOut(movingOut_), syncing(syncing_), copyingIn(copyingIn_), copyingOut(copyingOut_), total(total_)
     {}
 
     bool operator==(const NodeMaintenanceStats& other) const noexcept {
         return (movingOut == other.movingOut
                 && syncing == other.syncing
                 && copyingIn == other.copyingIn
-                && copyingOut == other.copyingOut);
+                && copyingOut == other.copyingOut
+                && total == other.total);
     }
 };
 
@@ -63,6 +65,10 @@ public:
         ++_stats[node][bucketSpace].copyingOut;
     }
 
+    void incTotal(uint16_t node, document::BucketSpace bucketSpace) {
+        ++_stats[node][bucketSpace].total;
+    }
+
     /**
      * Returned statistics for a given node index and bucket space, or all zero statistics
      * if none have been recorded yet
diff --git a/storage/src/vespa/storage/distributor/maintenance/simplemaintenancescanner.cpp b/storage/src/vespa/storage/distributor/maintenance/simplemaintenancescanner.cpp
index 870dcc25a4b..799b3641c64 100644
--- a/storage/src/vespa/storage/distributor/maintenance/simplemaintenancescanner.cpp
+++ b/storage/src/vespa/storage/distributor/maintenance/simplemaintenancescanner.cpp
@@ -37,6 +37,7 @@ SimpleMaintenanceScanner::scanNext()
             _bucketCursor = document::BucketId();
             continue;
         }
+        countBucket(_bucketSpaceItr->first, entry.getBucketInfo());
         prioritizeBucket(document::Bucket(_bucketSpaceItr->first, entry.getBucketId()));
         _bucketCursor = entry.getBucketId();
         return ScanResult::createNotDone(_bucketSpaceItr->first, entry);
@@ -52,6 +53,17 @@ SimpleMaintenanceScanner::reset()
 }
 
 void
+SimpleMaintenanceScanner::countBucket(document::BucketSpace bucketSpace, const BucketInfo &info)
+{
+    NodeMaintenanceStatsTracker &perNodeStats = _pendingMaintenance.perNodeStats;
+    uint32_t nodeCount = info.getNodeCount();
+    for (uint32_t i = 0; i < nodeCount; ++i) {
+        const BucketCopy &node = info.getNodeRef(i);
+        perNodeStats.incTotal(node.getNode(), bucketSpace);
+    }
+}
+
+void
 SimpleMaintenanceScanner::prioritizeBucket(const document::Bucket &bucket)
 {
     MaintenancePriorityAndType pri(_priorityGenerator.prioritize(bucket, _pendingMaintenance.perNodeStats));
diff --git a/storage/src/vespa/storage/distributor/maintenance/simplemaintenancescanner.h b/storage/src/vespa/storage/distributor/maintenance/simplemaintenancescanner.h
index f4ad53957e9..5b316201036 100644
--- a/storage/src/vespa/storage/distributor/maintenance/simplemaintenancescanner.h
+++ b/storage/src/vespa/storage/distributor/maintenance/simplemaintenancescanner.h
@@ -35,6 +35,8 @@ private:
     DistributorBucketSpaceRepo::BucketSpaceMap::const_iterator _bucketSpaceItr;
     document::BucketId _bucketCursor;
     PendingMaintenanceStats _pendingMaintenance;
+
+    void countBucket(document::BucketSpace bucketSpace, const BucketInfo &info);
 public:
     SimpleMaintenanceScanner(BucketPriorityDatabase& bucketPriorityDb,
                              const MaintenancePriorityGenerator& priorityGenerator,