Merge pull request #27988 from vespa-engine/balder/prepare-for-better-stats-reset-code

Balder/prepare for better stats reset code

11 files changed, 34 insertions, 69 deletions

diff --git a/storage/src/tests/distributor/simplemaintenancescannertest.cpp b/storage/src/tests/distributor/simplemaintenancescannertest.cpp
index 723b0baa6cd..b5dc72d995b 100644
--- a/storage/src/tests/distributor/simplemaintenancescannertest.cpp
+++ b/storage/src/tests/distributor/simplemaintenancescannertest.cpp
@@ -165,7 +165,7 @@ TEST_F(SimpleMaintenanceScannerTest, pending_maintenance_operation_statistics) {
                               "split bucket: 0, join bucket: 0, "
                               "set bucket state: 0, garbage collection: 0");
     {
-        auto stats(_scanner->getPendingMaintenanceStats());
+        const auto & stats = _scanner->getPendingMaintenanceStats();
         EXPECT_EQ(expectedEmpty, stringifyGlobalPendingStats(stats));
     }
 
@@ -173,7 +173,7 @@ TEST_F(SimpleMaintenanceScannerTest, pending_maintenance_operation_statistics) {
 
     // All mock operations generated have the merge type.
     {
-        auto stats(_scanner->getPendingMaintenanceStats());
+        const auto & stats = _scanner->getPendingMaintenanceStats();
         std::string expected("delete bucket: 0, merge bucket: 2, "
                              "split bucket: 0, join bucket: 0, "
                              "set bucket state: 0, garbage collection: 0");
@@ -182,7 +182,7 @@ TEST_F(SimpleMaintenanceScannerTest, pending_maintenance_operation_statistics) {
 
     _scanner->reset();
     {
-        auto stats(_scanner->getPendingMaintenanceStats());
+        const auto & stats = _scanner->getPendingMaintenanceStats();
         EXPECT_EQ(expectedEmpty, stringifyGlobalPendingStats(stats));
     }
 }
@@ -191,14 +191,14 @@ TEST_F(SimpleMaintenanceScannerTest, per_node_maintenance_stats_are_tracked) {
     addBucketToDb(1);
     addBucketToDb(3);
     {
-        auto stats(_scanner->getPendingMaintenanceStats());
+        const auto & stats = _scanner->getPendingMaintenanceStats();
         NodeMaintenanceStats emptyStats;
         EXPECT_EQ(emptyStats, stats.perNodeStats.forNode(0, makeBucketSpace()));
     }
     ASSERT_TRUE(scanEntireDatabase(2));
     // Mock is currently hardwired to increment movingOut for node 1 and
     // copyingIn for node 2 per bucket iterated (we've got 2).
-    auto stats(_scanner->getPendingMaintenanceStats());
+        const auto & stats = _scanner->getPendingMaintenanceStats();
     {
         NodeMaintenanceStats wantedNode1Stats;
         wantedNode1Stats.movingOut = 2;
diff --git a/storage/src/vespa/storage/distributor/distributor_stripe.cpp b/storage/src/vespa/storage/distributor/distributor_stripe.cpp
index 616fd77fdd7..55e9fe46e51 100644
--- a/storage/src/vespa/storage/distributor/distributor_stripe.cpp
+++ b/storage/src/vespa/storage/distributor/distributor_stripe.cpp
@@ -120,8 +120,7 @@ DistributorStripe::sendReply(const std::shared_ptr<api::StorageReply>& reply)
 }
 
 void DistributorStripe::send_shutdown_abort_reply(const std::shared_ptr<api::StorageMessage>& msg) {
-    api::StorageReply::UP reply(
-            std::dynamic_pointer_cast<api::StorageCommand>(msg)->makeReply());
+    auto reply = std::dynamic_pointer_cast<api::StorageCommand>(msg)->makeReply();
     reply->setResult(api::ReturnCode(api::ReturnCode::ABORTED, "Distributor is shutting down"));
     send_up_with_tracking(std::shared_ptr<api::StorageMessage>(reply.release()));
 }
@@ -179,8 +178,7 @@ DistributorStripe::handle_or_enqueue_message(const std::shared_ptr<api::StorageM
 }
 
 void
-DistributorStripe::handleCompletedMerge(
-        const std::shared_ptr<api::MergeBucketReply>& reply)
+DistributorStripe::handleCompletedMerge(const std::shared_ptr<api::MergeBucketReply>& reply)
 {
     _maintenanceOperationOwner.handleReply(reply);
 }
@@ -236,9 +234,7 @@ DistributorStripe::handleReply(const std::shared_ptr<api::StorageReply>& reply)
 }
 
 bool
-DistributorStripe::generateOperation(
-        const std::shared_ptr<api::StorageMessage>& msg,
-        Operation::SP& operation)
+DistributorStripe::generateOperation(const std::shared_ptr<api::StorageMessage>& msg, Operation::SP& operation)
 {
     return _externalOperationHandler.handleMessage(msg, operation);
 }
@@ -277,7 +273,6 @@ DistributorStripe::getClusterStateBundle() const
 void
 DistributorStripe::enableClusterStateBundle(const lib::ClusterStateBundle& state)
 {
-    lib::Node my_node(lib::NodeType::DISTRIBUTOR, getDistributorIndex());
     lib::ClusterStateBundle oldState = _clusterStateBundle;
     _clusterStateBundle = state;
     propagateClusterStates();
@@ -415,7 +410,6 @@ public:
 
     bool check(uint32_t msgType, uint16_t node, uint8_t pri) override {
         (void) node;
-        (void) pri;
         if (msgType == api::MessageType::SPLITBUCKET_ID && pri <= maxPri) {
             found = true;
             return false;
@@ -428,9 +422,7 @@ public:
 }
 
 void
-DistributorStripe::checkBucketForSplit(document::BucketSpace bucketSpace,
-                                 const BucketDatabase::Entry& e,
-                                 uint8_t priority)
+DistributorStripe::checkBucketForSplit(document::BucketSpace bucketSpace, const BucketDatabase::Entry& e, uint8_t priority)
 {
     if (!getConfig().doInlineSplit()) {
        return;
@@ -440,16 +432,13 @@ DistributorStripe::checkBucketForSplit(document::BucketSpace bucketSpace,
     // appropriate priority.
     SplitChecker checker(priority);
     for (uint32_t i = 0; i < e->getNodeCount(); ++i) {
-        _pendingMessageTracker.checkPendingMessages(e->getNodeRef(i).getNode(),
-                                                    document::Bucket(bucketSpace, e.getBucketId()),
-                                                    checker);
+        _pendingMessageTracker.checkPendingMessages(e->getNodeRef(i).getNode(), document::Bucket(bucketSpace, e.getBucketId()), checker);
         if (checker.found) {
             return;
         }
     }
 
-    Operation::SP operation =
-        _idealStateManager.generateInterceptingSplit(bucketSpace, e, priority);
+    Operation::SP operation = _idealStateManager.generateInterceptingSplit(bucketSpace, e, priority);
 
     if (operation.get()) {
         _maintenanceOperationOwner.start(operation, priority);
@@ -458,8 +447,7 @@ DistributorStripe::checkBucketForSplit(document::BucketSpace bucketSpace,
 
 // TODO STRIPE must be invoked by top-level bucket db updater probably
 void
-DistributorStripe::propagateDefaultDistribution(
-        std::shared_ptr<const lib::Distribution> distribution)
+DistributorStripe::propagateDefaultDistribution(std::shared_ptr<const lib::Distribution> distribution)
 {
     auto global_distr = GlobalBucketSpaceDistributionConverter::convert_to_global(*distribution);
     for (auto* repo : {_bucketSpaceRepo.get(), _readOnlyBucketSpaceRepo.get()}) {
@@ -699,9 +687,7 @@ DistributorStripe::scanNextBucket()
         _scanner->reset();
     } else {
         const auto &distribution(_bucketSpaceRepo->get(scanResult.getBucketSpace()).getDistribution());
-        _bucketDBMetricUpdater.visit(
-                scanResult.getEntry(),
-                distribution.getRedundancy());
+        _bucketDBMetricUpdater.visit(scanResult.getEntry(), distribution.getRedundancy());
     }
     return scanResult;
 }
@@ -823,12 +809,6 @@ DistributorStripe::getActiveIdealStateOperations() const
     return _maintenanceOperationOwner.toString();
 }
 
-std::string
-DistributorStripe::getActiveOperations() const
-{
-    return _operationOwner.toString();
-}
-
 StripeAccessGuard::PendingOperationStats
 DistributorStripe::pending_operation_stats() const
 {
diff --git a/storage/src/vespa/storage/distributor/distributor_stripe.h b/storage/src/vespa/storage/distributor/distributor_stripe.h
index 801efa0ff73..dc9b531573b 100644
--- a/storage/src/vespa/storage/distributor/distributor_stripe.h
+++ b/storage/src/vespa/storage/distributor/distributor_stripe.h
@@ -122,7 +122,6 @@ public:
     StripeAccessGuard::PendingOperationStats pending_operation_stats() const override;
 
     std::string getActiveIdealStateOperations() const;
-    std::string getActiveOperations() const;
 
     framework::ThreadWaitInfo doNonCriticalTick(framework::ThreadIndex);
 
diff --git a/storage/src/vespa/storage/distributor/idealstatemetricsset.cpp b/storage/src/vespa/storage/distributor/idealstatemetricsset.cpp
index d79bf2c4810..d50b2004bf2 100644
--- a/storage/src/vespa/storage/distributor/idealstatemetricsset.cpp
+++ b/storage/src/vespa/storage/distributor/idealstatemetricsset.cpp
@@ -134,7 +134,7 @@ IdealStateMetricSet::IdealStateMetricSet()
 
 IdealStateMetricSet::~IdealStateMetricSet() = default;
 
-void IdealStateMetricSet::setPendingOperations(const std::vector<uint64_t>& newMetrics) {
+void IdealStateMetricSet::setPendingOperations(vespalib::ConstArrayRef<uint64_t> newMetrics) {
     for (uint32_t i = 0; i < IdealStateOperation::OPERATION_COUNT; i++) {
         operations[i]->pending.set(newMetrics[i]);
     }
diff --git a/storage/src/vespa/storage/distributor/idealstatemetricsset.h b/storage/src/vespa/storage/distributor/idealstatemetricsset.h
index 6528ad7dc72..0bbc13d061a 100644
--- a/storage/src/vespa/storage/distributor/idealstatemetricsset.h
+++ b/storage/src/vespa/storage/distributor/idealstatemetricsset.h
@@ -5,6 +5,7 @@
 #include <vespa/metrics/valuemetric.h>
 #include <vespa/metrics/countmetric.h>
 #include <vespa/storage/distributor/operations/idealstate/idealstateoperation.h>
+#include <vespa/vespalib/util/arrayref.h>
 
 namespace storage::distributor {
 
@@ -61,7 +62,7 @@ public:
     IdealStateMetricSet();
     ~IdealStateMetricSet() override;
 
-    void setPendingOperations(const std::vector<uint64_t>& newMetrics);
+    void setPendingOperations(vespalib::ConstArrayRef<uint64_t> newMetrics);
 };
 
 } // storage::distributor
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 47b8aec4aa6..7ac99f5712f 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
@@ -7,16 +7,6 @@ namespace storage::distributor {
 
 const NodeMaintenanceStats NodeMaintenanceStatsTracker::_emptyNodeMaintenanceStats;
 
-void
-NodeMaintenanceStats::merge(const NodeMaintenanceStats& rhs)
-{
-    movingOut  += rhs.movingOut;
-    syncing    += rhs.syncing;
-    copyingIn  += rhs.copyingIn;
-    copyingOut += rhs.copyingOut;
-    total      += rhs.total;
-}
-
 namespace {
 
 void
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 deeef118685..3818dd4bacb 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
@@ -37,7 +37,13 @@ struct NodeMaintenanceStats
         return !(*this == other);
     }
 
-    void merge(const NodeMaintenanceStats& rhs);
+    void merge(const NodeMaintenanceStats& rhs) noexcept {
+        movingOut  += rhs.movingOut;
+        syncing    += rhs.syncing;
+        copyingIn  += rhs.copyingIn;
+        copyingOut += rhs.copyingOut;
+        total      += rhs.total;
+    }
 };
 
 std::ostream& operator<<(std::ostream&, const NodeMaintenanceStats&);
diff --git a/storage/src/vespa/storage/distributor/maintenance/simplemaintenancescanner.cpp b/storage/src/vespa/storage/distributor/maintenance/simplemaintenancescanner.cpp
index cb60e3eb0fc..afcbef32584 100644
--- a/storage/src/vespa/storage/distributor/maintenance/simplemaintenancescanner.cpp
+++ b/storage/src/vespa/storage/distributor/maintenance/simplemaintenancescanner.cpp
@@ -13,22 +13,22 @@ SimpleMaintenanceScanner::SimpleMaintenanceScanner(BucketPriorityDatabase& bucke
       _priorityGenerator(priorityGenerator),
       _bucketSpaceRepo(bucketSpaceRepo),
       _bucketSpaceItr(_bucketSpaceRepo.begin()),
-      _bucketCursor()
+      _bucketCursor(),
+      _pendingMaintenance()
 {
 }
 
 SimpleMaintenanceScanner::~SimpleMaintenanceScanner() = default;
 
 bool
-SimpleMaintenanceScanner::GlobalMaintenanceStats::operator==(const GlobalMaintenanceStats& rhs) const
+SimpleMaintenanceScanner::GlobalMaintenanceStats::operator==(const GlobalMaintenanceStats& rhs) const noexcept
 {
     return pending == rhs.pending;
 }
 
 void
-SimpleMaintenanceScanner::GlobalMaintenanceStats::merge(const GlobalMaintenanceStats& rhs)
+SimpleMaintenanceScanner::GlobalMaintenanceStats::merge(const GlobalMaintenanceStats& rhs) noexcept
 {
-    assert(pending.size() == rhs.pending.size());
     for (size_t i = 0; i < pending.size(); ++i) {
         pending[i] += rhs.pending[i];
     }
@@ -99,8 +99,7 @@ SimpleMaintenanceScanner::prioritizeBucket(const document::Bucket &bucket)
 }
 
 std::ostream&
-operator<<(std::ostream& os,
-           const SimpleMaintenanceScanner::GlobalMaintenanceStats& stats)
+operator<<(std::ostream& os, const SimpleMaintenanceScanner::GlobalMaintenanceStats& stats)
 {
     using MO = MaintenanceOperation;
     os << "delete bucket: "        << stats.pending[MO::DELETE_BUCKET]
diff --git a/storage/src/vespa/storage/distributor/maintenance/simplemaintenancescanner.h b/storage/src/vespa/storage/distributor/maintenance/simplemaintenancescanner.h
index cf6622f03b0..7af61815c31 100644
--- a/storage/src/vespa/storage/distributor/maintenance/simplemaintenancescanner.h
+++ b/storage/src/vespa/storage/distributor/maintenance/simplemaintenancescanner.h
@@ -13,14 +13,14 @@ class SimpleMaintenanceScanner : public MaintenanceScanner
 {
 public:
     struct GlobalMaintenanceStats {
-        std::vector<uint64_t> pending;
+        std::array<uint64_t, MaintenanceOperation::OPERATION_COUNT> pending;
 
-        GlobalMaintenanceStats()
-            : pending(MaintenanceOperation::OPERATION_COUNT)
+        GlobalMaintenanceStats() noexcept
+            : pending()
         { }
 
-        bool operator==(const GlobalMaintenanceStats& rhs) const;
-        void merge(const GlobalMaintenanceStats& rhs);
+        bool operator==(const GlobalMaintenanceStats& rhs) const noexcept;
+        void merge(const GlobalMaintenanceStats& rhs) noexcept;
     };
     struct PendingMaintenanceStats {
         PendingMaintenanceStats();
@@ -47,7 +47,7 @@ public:
                              const DistributorBucketSpaceRepo& bucketSpaceRepo);
     SimpleMaintenanceScanner(const SimpleMaintenanceScanner&) = delete;
     SimpleMaintenanceScanner& operator=(const SimpleMaintenanceScanner&) = delete;
-    ~SimpleMaintenanceScanner();
+    ~SimpleMaintenanceScanner() override;
 
     ScanResult scanNext() override;
     void reset();
diff --git a/storage/src/vespa/storage/distributor/top_level_distributor.cpp b/storage/src/vespa/storage/distributor/top_level_distributor.cpp
index 80c096135fa..a13ad3aeb25 100644
--- a/storage/src/vespa/storage/distributor/top_level_distributor.cpp
+++ b/storage/src/vespa/storage/distributor/top_level_distributor.cpp
@@ -360,15 +360,6 @@ TopLevelDistributor::getBucketSpacesStats() const
     return result;
 }
 
-SimpleMaintenanceScanner::PendingMaintenanceStats
-TopLevelDistributor::pending_maintenance_stats() const {
-    SimpleMaintenanceScanner::PendingMaintenanceStats result;
-    for (const auto& stripe : _stripes) {
-        result.merge(stripe->pending_maintenance_stats());
-    }
-    return result;
-}
-
 void
 TopLevelDistributor::propagateInternalScanMetricsToExternal()
 {
diff --git a/storage/src/vespa/storage/distributor/top_level_distributor.h b/storage/src/vespa/storage/distributor/top_level_distributor.h
index aa3a7b3655d..da3642a9312 100644
--- a/storage/src/vespa/storage/distributor/top_level_distributor.h
+++ b/storage/src/vespa/storage/distributor/top_level_distributor.h
@@ -145,7 +145,6 @@ private:
     std::unordered_map<uint16_t, uint32_t> getMinReplica() const override;
 
     PerNodeBucketSpacesStats getBucketSpacesStats() const override;
-    SimpleMaintenanceScanner::PendingMaintenanceStats pending_maintenance_stats() const;
 
     /**
      * Atomically publish internal metrics to external ideal state metrics.