Only update bucket DB memory statistics at certain intervals

B-tree/datastore stats can be expensive to sample, so don't do
this after every full DB iteration. For now, wait at least 30s.

3 files changed, 73 insertions, 11 deletions

diff --git a/storage/src/tests/distributor/distributortest.cpp b/storage/src/tests/distributor/distributortest.cpp
index 559784121a6..421fd2cb4b6 100644
--- a/storage/src/tests/distributor/distributortest.cpp
+++ b/storage/src/tests/distributor/distributortest.cpp
@@ -12,6 +12,7 @@
 #include <vespa/document/test/make_bucket_space.h>
 #include <vespa/storage/config/config-stor-distributormanager.h>
 #include <vespa/storage/distributor/distributor.h>
+#include <vespa/storage/distributor/distributormetricsset.h>
 #include <vespa/vespalib/text/stringtokenizer.h>
 #include <vespa/vespalib/util/time.h>
 #include <thread>
@@ -463,6 +464,51 @@ TEST_F(DistributorTest, metric_update_hook_updates_pending_maintenance_metrics)
     }
 }
 
+namespace {
+
+uint64_t db_sample_interval_sec(const Distributor& d) noexcept {
+    return std::chrono::duration_cast<std::chrono::seconds>(d.db_memory_sample_interval()).count();
+}
+
+}
+
+TEST_F(DistributorTest, bucket_db_memory_usage_metrics_only_updated_at_fixed_time_intervals) {
+    getClock().setAbsoluteTimeInSeconds(1000);
+
+    setupDistributor(Redundancy(2), NodeCount(2), "storage:2 distributor:1");
+    addNodesToBucketDB(document::BucketId(16, 1), "0=1/1/1/t/a,1=2/2/2");
+    tickDistributorNTimes(10);
+
+    vespalib::Monitor l;
+    distributor_metric_update_hook().updateMetrics(vespalib::MonitorGuard(l));
+    auto* m = getDistributor().getMetrics().mutable_dbs.memory_usage.getMetric("used_bytes");
+    ASSERT_TRUE(m != nullptr);
+    auto last_used = m->getLongValue("last");
+    EXPECT_GT(last_used, 0);
+
+    // Add another bucket to the DB. This should increase the underlying used number of
+    // bytes, but this should not be aggregated into the metrics until the sampling time
+    // interval has passed. Instead, old metric gauge values should be preserved.
+    addNodesToBucketDB(document::BucketId(16, 2), "0=1/1/1/t/a,1=2/2/2");
+
+    const auto sample_interval_sec = db_sample_interval_sec(getDistributor());
+    getClock().setAbsoluteTimeInSeconds(1000 + sample_interval_sec - 1); // Not there yet.
+    tickDistributorNTimes(50);
+    distributor_metric_update_hook().updateMetrics(vespalib::MonitorGuard(l));
+
+    m = getDistributor().getMetrics().mutable_dbs.memory_usage.getMetric("used_bytes");
+    auto now_used = m->getLongValue("last");
+    EXPECT_EQ(now_used, last_used);
+
+    getClock().setAbsoluteTimeInSeconds(1000 + sample_interval_sec + 1);
+    tickDistributorNTimes(10);
+    distributor_metric_update_hook().updateMetrics(vespalib::MonitorGuard(l));
+
+    m = getDistributor().getMetrics().mutable_dbs.memory_usage.getMetric("used_bytes");
+    now_used = m->getLongValue("last");
+    EXPECT_GT(now_used, last_used);
+}
+
 TEST_F(DistributorTest, priority_config_is_propagated_to_distributor_configuration) {
     using namespace vespa::config::content::core;
 
diff --git a/storage/src/vespa/storage/distributor/distributor.cpp b/storage/src/vespa/storage/distributor/distributor.cpp
index 3cca2847671..cce3d2d1acb 100644
--- a/storage/src/vespa/storage/distributor/distributor.cpp
+++ b/storage/src/vespa/storage/distributor/distributor.cpp
@@ -18,6 +18,8 @@
 #include <vespa/log/log.h>
 LOG_SETUP(".distributor-main");
 
+using namespace std::chrono_literals;
+
 namespace storage::distributor {
 
 class Distributor::Status {
@@ -100,9 +102,9 @@ Distributor::Distributor(DistributorComponentRegister& compReg,
       _bucketSpacesStats(),
       _bucketDbStats(),
       _hostInfoReporter(*this, *this),
-      _ownershipSafeTimeCalc(
-            std::make_unique<OwnershipTransferSafeTimePointCalculator>(
-                std::chrono::seconds(0))), // Set by config later
+      _ownershipSafeTimeCalc(std::make_unique<OwnershipTransferSafeTimePointCalculator>(0s)), // Set by config later
+      _db_memory_sample_interval(30s),
+      _last_db_memory_sample_time_point(),
       _must_send_updated_host_info(false),
       _use_btree_database(use_btree_database)
 {
@@ -769,15 +771,23 @@ Distributor::updateInternalMetricsForCompletedScan()
         _must_send_updated_host_info = true;
     }
     _bucketSpacesStats = std::move(new_space_stats);
-    update_bucket_db_memory_usage_stats();
+    maybe_update_bucket_db_memory_usage_stats();
 }
 
-void Distributor::update_bucket_db_memory_usage_stats() {
-    for (auto& space : *_bucketSpaceRepo) {
-        _bucketDBMetricUpdater.update_db_memory_usage(space.second->getBucketDatabase().memory_usage(), true);
-    }
-    for (auto& space : *_readOnlyBucketSpaceRepo) {
-        _bucketDBMetricUpdater.update_db_memory_usage(space.second->getBucketDatabase().memory_usage(), false);
+void Distributor::maybe_update_bucket_db_memory_usage_stats() {
+    auto now = _component.getClock().getMonotonicTime();
+    if ((now - _last_db_memory_sample_time_point) > _db_memory_sample_interval) {
+        for (auto& space : *_bucketSpaceRepo) {
+            _bucketDBMetricUpdater.update_db_memory_usage(space.second->getBucketDatabase().memory_usage(), true);
+        }
+        for (auto& space : *_readOnlyBucketSpaceRepo) {
+            _bucketDBMetricUpdater.update_db_memory_usage(space.second->getBucketDatabase().memory_usage(), false);
+        }
+        _last_db_memory_sample_time_point = now;
+    } else {
+        // Reuse previous memory statistics instead of sampling new.
+        _bucketDBMetricUpdater.update_db_memory_usage(_bucketDbStats._mutable_db_mem_usage, true);
+        _bucketDBMetricUpdater.update_db_memory_usage(_bucketDbStats._read_only_db_mem_usage, false);
     }
 }
 
diff --git a/storage/src/vespa/storage/distributor/distributor.h b/storage/src/vespa/storage/distributor/distributor.h
index 36e34592cba..bf780434edf 100644
--- a/storage/src/vespa/storage/distributor/distributor.h
+++ b/storage/src/vespa/storage/distributor/distributor.h
@@ -191,6 +191,10 @@ public:
         Distributor& _self;
     };
 
+    std::chrono::steady_clock::duration db_memory_sample_interval() const noexcept {
+        return _db_memory_sample_interval;
+    }
+
 private:
     friend struct DistributorTest;
     friend class BucketDBUpdaterTest;
@@ -226,7 +230,7 @@ private:
      * Takes metric lock.
      */
     void updateInternalMetricsForCompletedScan();
-    void update_bucket_db_memory_usage_stats();
+    void maybe_update_bucket_db_memory_usage_stats();
     void scanAllBuckets();
     MaintenanceScanner::ScanResult scanNextBucket();
     void enableNextConfig();
@@ -330,6 +334,8 @@ private:
     BucketDBMetricUpdater::Stats _bucketDbStats;
     DistributorHostInfoReporter _hostInfoReporter;
     std::unique_ptr<OwnershipTransferSafeTimePointCalculator> _ownershipSafeTimeCalc;
+    std::chrono::steady_clock::duration _db_memory_sample_interval;
+    std::chrono::steady_clock::time_point _last_db_memory_sample_time_point;
     bool _must_send_updated_host_info;
     const bool _use_btree_database;
 };