Merge pull request #18145 from vespa-engine/balder/add-merge-throttler-metrics

Balder/add merge throttler metrics

7 files changed, 15 insertions, 19 deletions

diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/VespaMetricSet.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/VespaMetricSet.java
index c5aba8388b4..114a3e380ef 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/VespaMetricSet.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/VespaMetricSet.java
@@ -765,6 +765,15 @@ public class VespaMetricSet {
 
         metrics.add(new Metric("vds.bouncer.clock_skew_aborts.count"));
 
+        metrics.add(new Metric("vds.mergethrottler.averagequeuewaitingtime.max"));
+        metrics.add(new Metric("vds.mergethrottler.averagequeuewaitingtime.sum"));
+        metrics.add(new Metric("vds.mergethrottler.averagequeuewaitingtime.count"));
+        metrics.add(new Metric("vds.mergethrottler.queuesize.max"));
+        metrics.add(new Metric("vds.mergethrottler.queuesize.sum"));
+        metrics.add(new Metric("vds.mergethrottler.queuesize.count"));
+        metrics.add(new Metric("vds.mergethrottler.bounced_due_to_back_pressure.rate"));
+        metrics.add(new Metric("vds.mergethrottler.locallyexecutedmerges.ok.rate"));
+        metrics.add(new Metric("vds.mergethrottler.mergechains.ok.rate"));
         return metrics;
     }
 
diff --git a/metrics/src/vespa/metrics/countmetric.h b/metrics/src/vespa/metrics/countmetric.h
index 02a6827d1ce..1701071104e 100644
--- a/metrics/src/vespa/metrics/countmetric.h
+++ b/metrics/src/vespa/metrics/countmetric.h
@@ -105,7 +105,7 @@ public:
     void addToSnapshot(Metric&, std::vector<Metric::UP> &) const override;
 };
 
-typedef CountMetric<uint64_t, true> LongCountMetric;
+using LongCountMetric = CountMetric<uint64_t, true>;
 
 } // metrics
 
diff --git a/metrics/src/vespa/metrics/metricvalueset.h b/metrics/src/vespa/metrics/metricvalueset.h
index 2463990378e..c522876f5b1 100644
--- a/metrics/src/vespa/metrics/metricvalueset.h
+++ b/metrics/src/vespa/metrics/metricvalueset.h
@@ -76,12 +76,6 @@ public:
      */
     bool setValues(const ValueClass& values);
 
-    /**
-     * Retrieve and reset in a single operation, to minimize chance of
-     * alteration in the process.
-     */
-    ValueClass getValuesAndReset();
-
     void reset() {
         setFlag(RESET);
     }
@@ -105,9 +99,6 @@ public:
         _flags.store(_flags.load(std::memory_order_relaxed) & ~flags,
                      std::memory_order_relaxed);
     }
-    uint32_t getFlags() const {
-        return _flags.load(std::memory_order_relaxed);
-    }
 };
 
 } // metrics
diff --git a/metrics/src/vespa/metrics/metricvalueset.hpp b/metrics/src/vespa/metrics/metricvalueset.hpp
index 8c5b32afcf8..57b3e7f9901 100644
--- a/metrics/src/vespa/metrics/metricvalueset.hpp
+++ b/metrics/src/vespa/metrics/metricvalueset.hpp
@@ -70,14 +70,6 @@ MetricValueSet<ValueClass>::setValues(const ValueClass& values) {
 }
 
 template<typename ValueClass>
-ValueClass
-MetricValueSet<ValueClass>::getValuesAndReset() {
-    ValueClass result(getValues());
-    setFlag(RESET);
-    return result;
-}
-
-template<typename ValueClass>
 std::string
 MetricValueSet<ValueClass>::toString() {
     std::ostringstream ost;
diff --git a/storage/src/tests/storageserver/mergethrottlertest.cpp b/storage/src/tests/storageserver/mergethrottlertest.cpp
index 12ed9ead1b6..3a153fef9c3 100644
--- a/storage/src/tests/storageserver/mergethrottlertest.cpp
+++ b/storage/src/tests/storageserver/mergethrottlertest.cpp
@@ -1233,6 +1233,7 @@ TEST_F(MergeThrottlerTest, busy_returned_on_full_queue) {
 
     // Wait till we have maxPending replies and maxQueue queued
     _topLinks[0]->waitForMessages(maxPending, _messageWaitTime);
+    EXPECT_EQ(19, _throttlers[0]->getMetrics().queueSize.getMaximum());
     waitUntilMergeQueueIs(*_throttlers[0], maxQueue, _messageWaitTime);
 
     // Clear all forwarded merges
diff --git a/storage/src/vespa/storage/storageserver/mergethrottler.cpp b/storage/src/vespa/storage/storageserver/mergethrottler.cpp
index dc8457769a2..a9bd7ade270 100644
--- a/storage/src/vespa/storage/storageserver/mergethrottler.cpp
+++ b/storage/src/vespa/storage/storageserver/mergethrottler.cpp
@@ -71,6 +71,7 @@ MergeThrottler::ChainedMergeState::~ChainedMergeState() = default;
 MergeThrottler::Metrics::Metrics(metrics::MetricSet* owner)
     : metrics::MetricSet("mergethrottler", {}, "", owner),
       averageQueueWaitingTime("averagequeuewaitingtime", {}, "Average time a merge spends in the throttler queue", this),
+      queueSize("queuesize", {}, "Length of merge queue", this),
       bounced_due_to_back_pressure("bounced_due_to_back_pressure", {}, "Number of merges bounced due to resource exhaustion back-pressure", this),
       chaining("mergechains", this),
       local("locallyexecutedmerges", this)
@@ -415,6 +416,7 @@ MergeThrottler::enqueueMerge(
     if (!validateNewMerge(mergeCmd, nodeSeq, msgGuard)) {
         return;
     }
+    _metrics->queueSize.set(_queue.size());
     _queue.insert(MergePriorityQueue::value_type(msg, _queueSequence++));
 }
 
diff --git a/storage/src/vespa/storage/storageserver/mergethrottler.h b/storage/src/vespa/storage/storageserver/mergethrottler.h
index e8815eee680..0c608f29196 100644
--- a/storage/src/vespa/storage/storageserver/mergethrottler.h
+++ b/storage/src/vespa/storage/storageserver/mergethrottler.h
@@ -57,12 +57,13 @@ public:
         MergeFailureMetrics failures;
 
         MergeOperationMetrics(const std::string& name, metrics::MetricSet* owner);
-        ~MergeOperationMetrics();
+        ~MergeOperationMetrics() override;
     };
 
     class Metrics : public metrics::MetricSet {
     public:
         metrics::DoubleAverageMetric averageQueueWaitingTime;
+        metrics::LongValueMetric queueSize;
         metrics::LongCountMetric bounced_due_to_back_pressure;
         MergeOperationMetrics chaining;
         MergeOperationMetrics local;