From b56dc4bb1ca04ea6855474838c66db453055fdd9 Mon Sep 17 00:00:00 2001 From: Remi Gacogne Date: Mon, 5 May 2025 17:03:06 +0200 Subject: [PATCH] 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. --- pdns/dnsdistdist/dnsdist-lua-ffi.cc | 8 +-- pdns/dnsdistdist/dnsdist-metrics.cc | 104 ++++++++++++++++++---------- pdns/dnsdistdist/dnsdist-metrics.hh | 10 +-- 3 files changed, 77 insertions(+), 45 deletions(-) diff --git a/pdns/dnsdistdist/dnsdist-lua-ffi.cc b/pdns/dnsdistdist/dnsdist-lua-ffi.cc index 08b3743df2..d8b20f8c77 100644 --- 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(&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(&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(&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(&result) != nullptr) { return; } diff --git a/pdns/dnsdistdist/dnsdist-metrics.cc b/pdns/dnsdistdist/dnsdist-metrics.cc index 80538b5666..4ec0fed496 100644 --- 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::less<>>> s_customCounters; -static SharedLockGuarded, std::less<>>> s_customGauges; +/* map of metric name -> map of labels -> metric value */ +template +using LabelsToMetricMap = std::map; +static SharedLockGuarded, std::less<>>> s_customCounters; +static SharedLockGuarded, std::less<>>> s_customGauges; Stats::Stats() : entries{std::vector{ @@ -227,64 +230,91 @@ static string prometheusLabelValueEscape(const string& value) return ret; } -static std::string generateCombinationOfLabels(const std::unordered_map& 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& label) { return acc + (acc.empty() ? std::string() : ",") + label.first + "=" + "\"" + prometheusLabelValueEscape(label.second) + "\""; }); } -template -static T& initializeOrGetMetric(const std::string_view& name, std::map& metricEntries, const std::unordered_map& labels) +template +static std::variant updateMetric(const std::string_view& name, SharedLockGuarded, std::less<>>>& metricMap, const Labels& labels, const std::function& 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) { + 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) { + 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 incrementCustomCounter(const std::string_view& name, uint64_t step, const std::unordered_map& labels) +std::variant 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(name, s_customCounters, labels, [step](const MutableCounter& counter) -> void { + counter.d_value += step; + }); } -std::variant decrementCustomCounter(const std::string_view& name, uint64_t step, const std::unordered_map& labels) +std::variant 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(name, s_customCounters, labels, [step](const MutableCounter& counter) { + counter.d_value -= step; + }); } -std::variant setCustomGauge(const std::string_view& name, const double value, const std::unordered_map& labels) +std::variant 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(name, s_customGauges, labels, [value](const MutableGauge& gauge) { + gauge.d_value = value; + }); } -std::variant getCustomMetric(const std::string_view& name, const std::unordered_map& labels) +std::variant 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 6cab905272..7842bb2205 100644 --- a/pdns/dnsdistdist/dnsdist-metrics.hh +++ b/pdns/dnsdistdist/dnsdist-metrics.hh @@ -22,6 +22,7 @@ #pragma once #include +#include #include #include #include @@ -34,12 +35,13 @@ namespace dnsdist::metrics { using Error = std::string; +using Labels = std::optional>>; [[nodiscard]] std::optional declareCustomMetric(const std::string& name, const std::string& type, const std::string& description, std::optional customName, bool withLabels); -[[nodiscard]] std::variant incrementCustomCounter(const std::string_view& name, uint64_t step, const std::unordered_map& labels); -[[nodiscard]] std::variant decrementCustomCounter(const std::string_view& name, uint64_t step, const std::unordered_map& labels); -[[nodiscard]] std::variant setCustomGauge(const std::string_view& name, const double value, const std::unordered_map& labels); -[[nodiscard]] std::variant getCustomMetric(const std::string_view& name, const std::unordered_map& labels); +[[nodiscard]] std::variant incrementCustomCounter(const std::string_view& name, uint64_t step, const Labels& labels); +[[nodiscard]] std::variant decrementCustomCounter(const std::string_view& name, uint64_t step, const Labels& labels); +[[nodiscard]] std::variant setCustomGauge(const std::string_view& name, const double value, const Labels& labels); +[[nodiscard]] std::variant getCustomMetric(const std::string_view& name, const Labels& labels); using pdns::stat_t; -- 2.47.2