dnsdist: Improve scalability of custom metrics

This commit improves the scalability of custom metrics by:
- being optimistic about the existence of a given metric (including labels):
  since most of the time a given metric, even with labels, will be increased
  more than once, we can take read-only lock and only fallback to taking a
  write lock if we actually have to add a new entry. This is especially
  useful when using custom metrics with per-thread Lua, since there is no
  global lock involved in this case.
- optimizing the "no label" case, since the Lua FFI interface does not
  use anyway: skip the creation (and destruction) of an empty labels
  map whenever possible, return an empty string early when combining
  empty labels.

It already yields a noticeable improvement when a single thread is used,
but really shines when several threads are processing queries simultaneously.

diff --git a/pdns/dnsdistdist/dnsdist-lua-ffi.cc b/pdns/dnsdistdist/dnsdist-lua-ffi.cc
index 08b3743df2a4c3e225d53926dbe47a4e56df7960..d8b20f8c77da4a50fcf656d0ff014951821bf27c 100644 (file)
--- a/pdns/dnsdistdist/dnsdist-lua-ffi.cc
+++ b/pdns/dnsdistdist/dnsdist-lua-ffi.cc
@@ -1864,7 +1864,7 @@ bool dnsdist_ffi_metric_declare(const char* name, size_t nameLen, const char* ty
 
 void dnsdist_ffi_metric_inc(const char* metricName, size_t metricNameLen)
 {
-  auto result = dnsdist::metrics::incrementCustomCounter(std::string_view(metricName, metricNameLen), 1U, {});
+  auto result = dnsdist::metrics::incrementCustomCounter(std::string_view(metricName, metricNameLen), 1U, std::nullopt);
   if (std::get_if<dnsdist::metrics::Error>(&result) != nullptr) {
     return;
   }
@@ -1872,7 +1872,7 @@ void dnsdist_ffi_metric_inc(const char* metricName, size_t metricNameLen)
 
 void dnsdist_ffi_metric_inc_by(const char* metricName, size_t metricNameLen, uint64_t value)
 {
-  auto result = dnsdist::metrics::incrementCustomCounter(std::string_view(metricName, metricNameLen), value, {});
+  auto result = dnsdist::metrics::incrementCustomCounter(std::string_view(metricName, metricNameLen), value, std::nullopt);
   if (std::get_if<dnsdist::metrics::Error>(&result) != nullptr) {
     return;
   }
@@ -1880,7 +1880,7 @@ void dnsdist_ffi_metric_inc_by(const char* metricName, size_t metricNameLen, uin
 
 void dnsdist_ffi_metric_dec(const char* metricName, size_t metricNameLen)
 {
-  auto result = dnsdist::metrics::decrementCustomCounter(std::string_view(metricName, metricNameLen), 1U, {});
+  auto result = dnsdist::metrics::decrementCustomCounter(std::string_view(metricName, metricNameLen), 1U, std::nullopt);
   if (std::get_if<dnsdist::metrics::Error>(&result) != nullptr) {
     return;
   }
@@ -1888,7 +1888,7 @@ void dnsdist_ffi_metric_dec(const char* metricName, size_t metricNameLen)
 
 void dnsdist_ffi_metric_set(const char* metricName, size_t metricNameLen, double value)
 {
-  auto result = dnsdist::metrics::setCustomGauge(std::string_view(metricName, metricNameLen), value, {});
+  auto result = dnsdist::metrics::setCustomGauge(std::string_view(metricName, metricNameLen), value, std::nullopt);
   if (std::get_if<dnsdist::metrics::Error>(&result) != nullptr) {
     return;
   }

diff --git a/pdns/dnsdistdist/dnsdist-metrics.cc b/pdns/dnsdistdist/dnsdist-metrics.cc
index 80538b56669a96ed8c62b4e861fca4659ecda200..4ec0fed4968d4cd939951cc6d5d5ef11e5627510 100644 (file)
--- a/pdns/dnsdistdist/dnsdist-metrics.cc
+++ b/pdns/dnsdistdist/dnsdist-metrics.cc
@@ -70,8 +70,11 @@ struct MutableGauge
   mutable pdns::stat_double_t d_value{0};
 };
 
-static SharedLockGuarded<std::map<std::string, std::map<std::string, MutableCounter>, std::less<>>> s_customCounters;
-static SharedLockGuarded<std::map<std::string, std::map<std::string, MutableGauge>, std::less<>>> s_customGauges;
+/* map of metric name -> map of labels -> metric value */
+template <class MetricType>
+using LabelsToMetricMap = std::map<std::string, MetricType>;
+static SharedLockGuarded<std::map<std::string, LabelsToMetricMap<MutableCounter>, std::less<>>> s_customCounters;
+static SharedLockGuarded<std::map<std::string, LabelsToMetricMap<MutableGauge>, std::less<>>> s_customGauges;
 
 Stats::Stats() :
   entries{std::vector<EntryTriple>{
@@ -227,64 +230,91 @@ static string prometheusLabelValueEscape(const string& value)
   return ret;
 }
 
-static std::string generateCombinationOfLabels(const std::unordered_map<std::string, std::string>& labels)
+static std::string generateCombinationOfLabels(const Labels& optLabels)
 {
+  if (!optLabels || optLabels->get().empty()) {
+    return {};
+  }
+  const auto& labels = optLabels->get();
   auto ordered = std::map(labels.begin(), labels.end());
   return std::accumulate(ordered.begin(), ordered.end(), std::string(), [](const std::string& acc, const std::pair<std::string, std::string>& label) {
     return acc + (acc.empty() ? std::string() : ",") + label.first + "=" + "\"" + prometheusLabelValueEscape(label.second) + "\"";
   });
 }
 
-template <typename T>
-static T& initializeOrGetMetric(const std::string_view& name, std::map<std::string, T>& metricEntries, const std::unordered_map<std::string, std::string>& labels)
+template <typename MetricType, typename MetricValueType>
+static std::variant<MetricValueType, Error> updateMetric(const std::string_view& name, SharedLockGuarded<std::map<std::string, LabelsToMetricMap<MetricType>, std::less<>>>& metricMap, const Labels& labels, const std::function<void(const MetricType&)>& callback)
 {
   auto combinationOfLabels = generateCombinationOfLabels(labels);
-  auto metricEntry = metricEntries.find(combinationOfLabels);
-  if (metricEntry == metricEntries.end()) {
+  /* be optimistic first, and see if the metric and labels exist */
+  {
+    auto readLockedMap = metricMap.read_lock();
+    auto labelsMapIt = readLockedMap->find(name);
+    if (labelsMapIt == readLockedMap->end()) {
+      if constexpr (std::is_same_v<MetricType, MutableCounter>) {
+        return std::string("Unable to update custom metric '") + std::string(name) + "': no such counter";
+      }
+      else {
+        return std::string("Unable to update custom metric '") + std::string(name) + "': no such gauge";
+      }
+    }
+
+    auto& metricEntries = labelsMapIt->second;
+    auto metricEntry = metricEntries.find(combinationOfLabels);
+    if (metricEntry != metricEntries.end()) {
+      callback(metricEntry->second);
+      return metricEntry->second.d_value.load();
+    }
+  }
+
+  /* OK, so we the metric exists (otherwise we would have returned an Error) but the label doesn't yet */
+  {
+    auto writeLockedMap = metricMap.write_lock();
+    auto labelsMapIt = writeLockedMap->find(name);
+    if (labelsMapIt == writeLockedMap->end()) {
+      if constexpr (std::is_same_v<MetricType, MutableCounter>) {
+        return std::string("Unable to update custom metric '") + std::string(name) + "': no such counter";
+      }
+      else {
+        return std::string("Unable to update custom metric '") + std::string(name) + "': no such gauge";
+      }
+    }
+    /* we need to check again, it might have been inserted in the meantime */
+    auto& metricEntries = labelsMapIt->second;
+    auto metricEntry = metricEntries.find(combinationOfLabels);
+    if (metricEntry != metricEntries.end()) {
+      callback(metricEntry->second);
+      return metricEntry->second.d_value.load();
+    }
     metricEntry = metricEntries.emplace(std::piecewise_construct, std::forward_as_tuple(combinationOfLabels), std::forward_as_tuple()).first;
     g_stats.entries.write_lock()->emplace_back(Stats::EntryTriple{std::string(name), std::move(combinationOfLabels), &metricEntry->second.d_value});
+    callback(metricEntry->second);
+    return metricEntry->second.d_value.load();
   }
-  return metricEntry->second;
 }
 
-std::variant<uint64_t, Error> incrementCustomCounter(const std::string_view& name, uint64_t step, const std::unordered_map<std::string, std::string>& labels)
+std::variant<uint64_t, Error> incrementCustomCounter(const std::string_view& name, uint64_t step, const Labels& labels)
 {
-  auto customCounters = s_customCounters.write_lock();
-  auto metric = customCounters->find(name);
-  if (metric != customCounters->end()) {
-    auto& metricEntry = initializeOrGetMetric(name, metric->second, labels);
-    metricEntry.d_value += step;
-    return metricEntry.d_value.load();
-  }
-  return std::string("Unable to increment custom metric '") + std::string(name) + "': no such counter";
+  return updateMetric<MutableCounter, uint64_t>(name, s_customCounters, labels, [step](const MutableCounter& counter) -> void {
+    counter.d_value += step;
+  });
 }
 
-std::variant<uint64_t, Error> decrementCustomCounter(const std::string_view& name, uint64_t step, const std::unordered_map<std::string, std::string>& labels)
+std::variant<uint64_t, Error> decrementCustomCounter(const std::string_view& name, uint64_t step, const Labels& labels)
 {
-  auto customCounters = s_customCounters.write_lock();
-  auto metric = customCounters->find(name);
-  if (metric != customCounters->end()) {
-    auto& metricEntry = initializeOrGetMetric(name, metric->second, labels);
-    metricEntry.d_value -= step;
-    return metricEntry.d_value.load();
-  }
-  return std::string("Unable to decrement custom metric '") + std::string(name) + "': no such counter";
+  return updateMetric<MutableCounter, uint64_t>(name, s_customCounters, labels, [step](const MutableCounter& counter) {
+    counter.d_value -= step;
+  });
 }
 
-std::variant<double, Error> setCustomGauge(const std::string_view& name, const double value, const std::unordered_map<std::string, std::string>& labels)
+std::variant<double, Error> setCustomGauge(const std::string_view& name, const double value, const Labels& labels)
 {
-  auto customGauges = s_customGauges.write_lock();
-  auto metric = customGauges->find(name);
-  if (metric != customGauges->end()) {
-    auto& metricEntry = initializeOrGetMetric(name, metric->second, labels);
-    metricEntry.d_value = value;
-    return value;
-  }
-
-  return std::string("Unable to set metric '") + std::string(name) + "': no such gauge";
+  return updateMetric<MutableGauge, double>(name, s_customGauges, labels, [value](const MutableGauge& gauge) {
+    gauge.d_value = value;
+  });
 }
 
-std::variant<double, Error> getCustomMetric(const std::string_view& name, const std::unordered_map<std::string, std::string>& labels)
+std::variant<double, Error> getCustomMetric(const std::string_view& name, const Labels& labels)
 {
   {
     auto customCounters = s_customCounters.read_lock();

diff --git a/pdns/dnsdistdist/dnsdist-metrics.hh b/pdns/dnsdistdist/dnsdist-metrics.hh
index 6cab90527284c2080ccf08887a54a05efb4c3a0a..7842bb2205d232e02498f261d3251aee4d807f89 100644 (file)
--- a/pdns/dnsdistdist/dnsdist-metrics.hh
+++ b/pdns/dnsdistdist/dnsdist-metrics.hh
@@ -22,6 +22,7 @@
 #pragma once
 
 #include <cinttypes>
+#include <functional>
 #include <optional>
 #include <string>
 #include <string_view>
@@ -34,12 +35,13 @@
 namespace dnsdist::metrics
 {
 using Error = std::string;
+using Labels = std::optional<std::reference_wrapper<const std::unordered_map<std::string, std::string>>>;
 
 [[nodiscard]] std::optional<Error> declareCustomMetric(const std::string& name, const std::string& type, const std::string& description, std::optional<std::string> customName, bool withLabels);
-[[nodiscard]] std::variant<uint64_t, Error> incrementCustomCounter(const std::string_view& name, uint64_t step, const std::unordered_map<std::string, std::string>& labels);
-[[nodiscard]] std::variant<uint64_t, Error> decrementCustomCounter(const std::string_view& name, uint64_t step, const std::unordered_map<std::string, std::string>& labels);
-[[nodiscard]] std::variant<double, Error> setCustomGauge(const std::string_view& name, const double value, const std::unordered_map<std::string, std::string>& labels);
-[[nodiscard]] std::variant<double, Error> getCustomMetric(const std::string_view& name, const std::unordered_map<std::string, std::string>& labels);
+[[nodiscard]] std::variant<uint64_t, Error> incrementCustomCounter(const std::string_view& name, uint64_t step, const Labels& labels);
+[[nodiscard]] std::variant<uint64_t, Error> decrementCustomCounter(const std::string_view& name, uint64_t step, const Labels& labels);
+[[nodiscard]] std::variant<double, Error> setCustomGauge(const std::string_view& name, const double value, const Labels& labels);
+[[nodiscard]] std::variant<double, Error> getCustomMetric(const std::string_view& name, const Labels& labels);
 
 using pdns::stat_t;