Merge pull request #27989 from vespa-engine/balder/faster-bucketdb-metrics

Move where possible

12 files changed, 59 insertions, 68 deletions

diff --git a/storage/src/tests/distributor/bucketdbmetricupdatertest.cpp b/storage/src/tests/distributor/bucketdbmetricupdatertest.cpp
index c4536c6fa2c..4d04e3ca51a 100644
--- a/storage/src/tests/distributor/bucketdbmetricupdatertest.cpp
+++ b/storage/src/tests/distributor/bucketdbmetricupdatertest.cpp
@@ -7,7 +7,6 @@
 #include <vespa/vespalib/util/memoryusage.h>
 #include <vespa/vespalib/gtest/gtest.h>
 #include <string>
-#include <sstream>
 
 namespace storage::distributor {
 
@@ -16,19 +15,16 @@ using namespace ::testing;
 
 struct BucketDBMetricUpdaterTest : Test {
     void visitBucketWith2Copies1Trusted(BucketDBMetricUpdater& metricUpdater);
-    void visitBucketWith2CopiesBothTrusted(
-            BucketDBMetricUpdater& metricUpdater);
+    void visitBucketWith2CopiesBothTrusted(BucketDBMetricUpdater& metricUpdater);
     void visitBucketWith1Copy(BucketDBMetricUpdater& metricUpdater);
 
-    using NodeToReplicasMap = std::unordered_map<uint16_t, uint32_t>;
+    using NodeToReplicasMap = MinReplicaMap;
     NodeToReplicasMap replicaStatsOf(BucketDBMetricUpdater& metricUpdater);
 
     BucketDBMetricUpdaterTest();
 };
 
-BucketDBMetricUpdaterTest::BucketDBMetricUpdaterTest()
-{
-}
+BucketDBMetricUpdaterTest::BucketDBMetricUpdaterTest() = default;
 
 namespace {
 
@@ -38,8 +34,6 @@ void addNode(BucketInfo& info, uint16_t node, uint32_t crc) {
     info.addNode(BucketCopy(1234, node, apiInfo), order);
 }
 
-using Trusted = bool;
-
 BucketInfo
 makeInfo(uint32_t copy0Crc)
 {
@@ -271,8 +265,7 @@ TEST_F(BucketDBMetricUpdaterTest, complete_round_clears_working_state) {
 
 // Replicas on nodes 0 and 1.
 void
-BucketDBMetricUpdaterTest::visitBucketWith2Copies1Trusted(
-        BucketDBMetricUpdater& metricUpdater)
+BucketDBMetricUpdaterTest::visitBucketWith2Copies1Trusted(BucketDBMetricUpdater& metricUpdater)
 {
     BucketInfo info;
     addNode(info, 0, 100);
@@ -283,8 +276,7 @@ BucketDBMetricUpdaterTest::visitBucketWith2Copies1Trusted(
 
 // Replicas on nodes 0 and 2.
 void
-BucketDBMetricUpdaterTest::visitBucketWith2CopiesBothTrusted(
-        BucketDBMetricUpdater& metricUpdater)
+BucketDBMetricUpdaterTest::visitBucketWith2CopiesBothTrusted(BucketDBMetricUpdater& metricUpdater)
 {
     BucketInfo info;
     addNode(info, 0, 200);
diff --git a/storage/src/tests/distributor/distributor_host_info_reporter_test.cpp b/storage/src/tests/distributor/distributor_host_info_reporter_test.cpp
index 6dfab5abc21..a72dfec2d94 100644
--- a/storage/src/tests/distributor/distributor_host_info_reporter_test.cpp
+++ b/storage/src/tests/distributor/distributor_host_info_reporter_test.cpp
@@ -14,7 +14,7 @@ namespace storage::distributor {
 
 using End = vespalib::JsonStream::End;
 using File = vespalib::File;
-using MinReplicaStats = std::unordered_map<uint16_t, uint32_t>;
+using MinReplicaStats = MinReplicaMap;
 using Object = vespalib::JsonStream::Object;
 using PerNodeBucketSpacesStats = BucketSpacesStatsProvider::PerNodeBucketSpacesStats;
 using BucketSpacesStats = BucketSpacesStatsProvider::BucketSpacesStats;
@@ -36,7 +36,7 @@ struct MockedMinReplicaProvider : MinReplicaProvider
     MinReplicaStats minReplica;
 
     ~MockedMinReplicaProvider() override;
-    std::unordered_map<uint16_t, uint32_t> getMinReplica() const override {
+    MinReplicaMap getMinReplica() const override {
         return minReplica;
     }
 };
diff --git a/storage/src/tests/distributor/top_level_distributor_test.cpp b/storage/src/tests/distributor/top_level_distributor_test.cpp
index dad6f477d83..94f8821f9c8 100644
--- a/storage/src/tests/distributor/top_level_distributor_test.cpp
+++ b/storage/src/tests/distributor/top_level_distributor_test.cpp
@@ -92,7 +92,7 @@ struct TopLevelDistributorTest : Test, TopLevelDistributorTestUtil {
         return _distributor->getBucketSpacesStats();
     }
 
-    std::unordered_map<uint16_t, uint32_t> distributor_min_replica_stats() {
+    MinReplicaMap distributor_min_replica_stats() {
         return _distributor->getMinReplica();
     }
 
@@ -504,7 +504,7 @@ void assert_invalid_bucket_stats_for_all_spaces(
     ASSERT_FALSE(space_iter->second.valid());
 }
 
-void assert_min_replica_stats_zeroed(const std::unordered_map<uint16_t, uint32_t>& stats, uint16_t node_index) {
+void assert_min_replica_stats_zeroed(const MinReplicaMap & stats, uint16_t node_index) {
     auto iter = stats.find(node_index);
     ASSERT_TRUE(iter != stats.cend());
     EXPECT_EQ(iter->second, 0);
diff --git a/storage/src/vespa/storage/distributor/bucketdb/bucketdbmetricupdater.cpp b/storage/src/vespa/storage/distributor/bucketdb/bucketdbmetricupdater.cpp
index fc6c957b737..dfcbbf63946 100644
--- a/storage/src/vespa/storage/distributor/bucketdb/bucketdbmetricupdater.cpp
+++ b/storage/src/vespa/storage/distributor/bucketdb/bucketdbmetricupdater.cpp
@@ -6,19 +6,26 @@
 
 namespace storage::distributor {
 
-BucketDBMetricUpdater::Stats::Stats()
+BucketDBMetricUpdater::Stats::Stats() noexcept
     : _docCount(0),
       _byteCount(0),
       _tooFewCopies(0),
       _tooManyCopies(0),
       _noTrusted(0),
-      _totalBuckets(0)
+      _totalBuckets(0),
+      _mutable_db_mem_usage(),
+      _read_only_db_mem_usage(),
+      _minBucketReplica()
 {
 }
 
 BucketDBMetricUpdater::Stats::Stats(const Stats &rhs) = default;
+BucketDBMetricUpdater::Stats & BucketDBMetricUpdater::Stats::operator=(const Stats &rhs) = default;
+BucketDBMetricUpdater::Stats::Stats(Stats &&rhs) noexcept = default;
+BucketDBMetricUpdater::Stats & BucketDBMetricUpdater::Stats::operator=(Stats &&rhs) noexcept = default;
+BucketDBMetricUpdater::Stats::~Stats() = default;
 
-BucketDBMetricUpdater::BucketDBMetricUpdater()
+BucketDBMetricUpdater::BucketDBMetricUpdater() noexcept
     : _workingStats(),
       _lastCompleteStats(),
       _replicaCountingMode(ReplicaCountingMode::TRUSTED),
@@ -35,8 +42,7 @@ BucketDBMetricUpdater::resetStats()
 }
 
 void
-BucketDBMetricUpdater::visit(const BucketDatabase::Entry& entry,
-                             uint32_t redundancy)
+BucketDBMetricUpdater::visit(const BucketDatabase::Entry& entry, uint32_t redundancy)
 {
     if (entry->getNodeCount() == 0) {
         // We used to have an assert on >0 but that caused some crashes, see
@@ -90,9 +96,7 @@ BucketDBMetricUpdater::visit(const BucketDatabase::Entry& entry,
 }
 
 void
-BucketDBMetricUpdater::updateMinReplicationStats(
-        const BucketDatabase::Entry& entry,
-        uint32_t trustedCopies)
+BucketDBMetricUpdater::updateMinReplicationStats(const BucketDatabase::Entry& entry, uint32_t trustedCopies)
 {
     auto& minBucketReplica = _workingStats._minBucketReplica;
     for (uint32_t i = 0; i < entry->getNodeCount(); i++) {
@@ -103,9 +107,9 @@ BucketDBMetricUpdater::updateMinReplicationStats(
         // sync across each other.
         // Regardless of counting mode we still have to take the minimum
         // replica count across all buckets present on any given node.
-        const uint32_t countedReplicas(
-                (_replicaCountingMode == ReplicaCountingMode::TRUSTED)
-                 ? trustedCopies : entry->getNodeCount());
+        const uint32_t countedReplicas = (_replicaCountingMode == ReplicaCountingMode::TRUSTED)
+                 ? trustedCopies
+                 : entry->getNodeCount();
         auto it = minBucketReplica.find(node);
         if (it == minBucketReplica.end()) {
             minBucketReplica[node] = countedReplicas;
@@ -118,17 +122,18 @@ BucketDBMetricUpdater::updateMinReplicationStats(
 void
 BucketDBMetricUpdater::completeRound(bool resetWorkingStats)
 {
-    _lastCompleteStats = _workingStats;
+
     _hasCompleteStats = true;
     if (resetWorkingStats) {
+        _lastCompleteStats = std::move(_workingStats);
         resetStats();
+    } else {
+        _lastCompleteStats = _workingStats;
     }
 }
 
 void
-BucketDBMetricUpdater::Stats::propagateMetrics(
-        IdealStateMetricSet& idealStateMetrics,
-        DistributorMetricSet& distributorMetrics)
+BucketDBMetricUpdater::Stats::propagateMetrics(IdealStateMetricSet& idealStateMetrics, DistributorMetricSet& distributorMetrics) const
 {
     distributorMetrics.docsStored.set(_docCount);
     distributorMetrics.bytesStored.set(_byteCount);
diff --git a/storage/src/vespa/storage/distributor/bucketdb/bucketdbmetricupdater.h b/storage/src/vespa/storage/distributor/bucketdb/bucketdbmetricupdater.h
index 2edb86cbaa2..366c2f2dc41 100644
--- a/storage/src/vespa/storage/distributor/bucketdb/bucketdbmetricupdater.h
+++ b/storage/src/vespa/storage/distributor/bucketdb/bucketdbmetricupdater.h
@@ -2,10 +2,11 @@
 
 #pragma once
 
+#include <vespa/storage/distributor/min_replica_provider.h>
 #include <vespa/storage/bucketdb/bucketdatabase.h>
 #include <vespa/storage/config/config-stor-distributormanager.h>
 #include <vespa/vespalib/util/memoryusage.h>
-#include <unordered_map>
+#include <vespa/vespalib/stllike/hash_map.h>
 
 namespace storage::distributor {
 
@@ -25,11 +26,12 @@ public:
         vespalib::MemoryUsage _mutable_db_mem_usage;
         vespalib::MemoryUsage _read_only_db_mem_usage;
 
-        Stats();
+        Stats() noexcept;
+        Stats(Stats &&rhs) noexcept;
+        Stats & operator=(Stats &&rhs) noexcept;
         Stats(const Stats &rhs);
-        ~Stats() = default;
-
-        Stats &operator=(const Stats &rhs) = default;
+        Stats & operator=(const Stats &rhs);
+        ~Stats();
 
         /**
          * For each node N, look at all the buckets that have or should have a
@@ -47,24 +49,24 @@ public:
          * Note: If no buckets have been found for a node, that node is not in
          * this map.
          */
-        std::unordered_map<uint16_t, uint32_t> _minBucketReplica;
+        MinReplicaMap _minBucketReplica;
 
         /**
          * Propagate state values to the appropriate metric values.
          */
-        void propagateMetrics(IdealStateMetricSet&, DistributorMetricSet&);
+        void propagateMetrics(IdealStateMetricSet&, DistributorMetricSet&) const;
     };
 
     using ReplicaCountingMode = vespa::config::content::core::StorDistributormanagerConfig::MinimumReplicaCountingMode;
 
 private:
-    Stats _workingStats;
-    Stats _lastCompleteStats;
+    Stats               _workingStats;
+    Stats               _lastCompleteStats;
     ReplicaCountingMode _replicaCountingMode;
-    bool _hasCompleteStats;
+    bool                _hasCompleteStats;
 
 public:
-    BucketDBMetricUpdater();
+    BucketDBMetricUpdater() noexcept;
     ~BucketDBMetricUpdater();
 
     void setMinimumReplicaCountingMode(ReplicaCountingMode mode) noexcept {
@@ -91,11 +93,11 @@ public:
     /**
      * Returns true iff completeRound() has been called at least once.
      */
-    bool hasCompletedRound() const {
+    bool hasCompletedRound() const noexcept {
         return _hasCompleteStats;
     }
 
-    Stats getLastCompleteStats() const {
+    const Stats & getLastCompleteStats() const noexcept {
         return _lastCompleteStats;
     }
 
diff --git a/storage/src/vespa/storage/distributor/distributor_host_info_reporter.cpp b/storage/src/vespa/storage/distributor/distributor_host_info_reporter.cpp
index bb7e573c980..46c9a526a8d 100644
--- a/storage/src/vespa/storage/distributor/distributor_host_info_reporter.cpp
+++ b/storage/src/vespa/storage/distributor/distributor_host_info_reporter.cpp
@@ -3,15 +3,9 @@
 #include "bucket_spaces_stats_provider.h"
 #include "distributor_host_info_reporter.h"
 #include "min_replica_provider.h"
-#include "pendingmessagetracker.h"
-
 #include <set>
 
-using std::set;
-using std::unordered_map;
-
-namespace storage {
-namespace distributor {
+namespace storage::distributor {
 
 using BucketSpacesStats = BucketSpacesStatsProvider::BucketSpacesStats;
 using PerNodeBucketSpacesStats = BucketSpacesStatsProvider::PerNodeBucketSpacesStats;
@@ -48,10 +42,10 @@ writeBucketSpacesStats(vespalib::JsonStream& stream,
 
 void
 outputStorageNodes(vespalib::JsonStream& output,
-                   const unordered_map<uint16_t, uint32_t>& minReplica,
+                   const MinReplicaMap & minReplica,
                    const PerNodeBucketSpacesStats& bucketSpacesStats)
 {
-    set<uint16_t> nodes;
+    std::set<uint16_t> nodes;
     for (const auto& element : minReplica) {
         nodes.insert(element.first);
     }
@@ -104,6 +98,5 @@ DistributorHostInfoReporter::report(vespalib::JsonStream& output)
     output << End();
 }
 
-} // distributor
-} // storage
+}
 
diff --git a/storage/src/vespa/storage/distributor/distributor_stripe.cpp b/storage/src/vespa/storage/distributor/distributor_stripe.cpp
index 55e9fe46e51..502901d27ea 100644
--- a/storage/src/vespa/storage/distributor/distributor_stripe.cpp
+++ b/storage/src/vespa/storage/distributor/distributor_stripe.cpp
@@ -550,7 +550,7 @@ void DistributorStripe::startExternalOperations() {
     _fetchedMessages.clear();
 }
 
-std::unordered_map<uint16_t, uint32_t>
+MinReplicaMap
 DistributorStripe::getMinReplica() const
 {
     std::lock_guard guard(_metricLock);
diff --git a/storage/src/vespa/storage/distributor/distributor_stripe.h b/storage/src/vespa/storage/distributor/distributor_stripe.h
index dc9b531573b..8664330f4e8 100644
--- a/storage/src/vespa/storage/distributor/distributor_stripe.h
+++ b/storage/src/vespa/storage/distributor/distributor_stripe.h
@@ -218,7 +218,7 @@ private:
     /**
      * Return a copy of the latest min replica data, see MinReplicaProvider.
      */
-    std::unordered_map<uint16_t, uint32_t> getMinReplica() const override;
+    MinReplicaMap getMinReplica() const override;
 
     PerNodeBucketSpacesStats getBucketSpacesStats() const override;
 
diff --git a/storage/src/vespa/storage/distributor/min_replica_provider.cpp b/storage/src/vespa/storage/distributor/min_replica_provider.cpp
index f503672af39..52780b99948 100644
--- a/storage/src/vespa/storage/distributor/min_replica_provider.cpp
+++ b/storage/src/vespa/storage/distributor/min_replica_provider.cpp
@@ -5,8 +5,7 @@
 namespace storage::distributor {
 
 void
-merge_min_replica_stats(std::unordered_map<uint16_t, uint32_t>& dest,
-                        const std::unordered_map<uint16_t, uint32_t>& src)
+merge_min_replica_stats(MinReplicaMap & dest, const MinReplicaMap & src)
 {
     for (const auto& entry : src) {
         auto node_index = entry.first;
diff --git a/storage/src/vespa/storage/distributor/min_replica_provider.h b/storage/src/vespa/storage/distributor/min_replica_provider.h
index a4374b906fe..75d3a150d21 100644
--- a/storage/src/vespa/storage/distributor/min_replica_provider.h
+++ b/storage/src/vespa/storage/distributor/min_replica_provider.h
@@ -1,11 +1,12 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 #pragma once
 
-#include <stdint.h>
-#include <unordered_map>
+#include <vespa/vespalib/stllike/hash_map.h>
 
 namespace storage::distributor {
 
+using MinReplicaMap = vespalib::hash_map<uint16_t, uint32_t>;
+
 class MinReplicaProvider
 {
 public:
@@ -17,11 +18,10 @@ public:
      * Can be called at any time after registration from another thread context
      * and the call must thus be thread safe and data race free.
      */
-    virtual std::unordered_map<uint16_t, uint32_t> getMinReplica() const = 0;
+    virtual MinReplicaMap getMinReplica() const = 0;
 };
 
-void merge_min_replica_stats(std::unordered_map<uint16_t, uint32_t>& dest,
-                             const std::unordered_map<uint16_t, uint32_t>& src);
+void merge_min_replica_stats(MinReplicaMap & dest, const MinReplicaMap & src);
 
 }
 
diff --git a/storage/src/vespa/storage/distributor/top_level_distributor.cpp b/storage/src/vespa/storage/distributor/top_level_distributor.cpp
index a13ad3aeb25..f957af5362e 100644
--- a/storage/src/vespa/storage/distributor/top_level_distributor.cpp
+++ b/storage/src/vespa/storage/distributor/top_level_distributor.cpp
@@ -340,10 +340,10 @@ TopLevelDistributor::propagate_default_distribution_thread_unsafe(
     }
 }
 
-std::unordered_map<uint16_t, uint32_t>
+MinReplicaMap
 TopLevelDistributor::getMinReplica() const
 {
-    std::unordered_map<uint16_t, uint32_t> result;
+    MinReplicaMap result;
     for (const auto& stripe : _stripes) {
         merge_min_replica_stats(result, stripe->getMinReplica());
     }
diff --git a/storage/src/vespa/storage/distributor/top_level_distributor.h b/storage/src/vespa/storage/distributor/top_level_distributor.h
index da3642a9312..278a68f72c6 100644
--- a/storage/src/vespa/storage/distributor/top_level_distributor.h
+++ b/storage/src/vespa/storage/distributor/top_level_distributor.h
@@ -142,7 +142,7 @@ private:
     /**
      * Return a copy of the latest min replica data, see MinReplicaProvider.
      */
-    std::unordered_map<uint16_t, uint32_t> getMinReplica() const override;
+    MinReplicaMap getMinReplica() const override;
 
     PerNodeBucketSpacesStats getBucketSpacesStats() const override;