From 4a82fb53896d0a9fb4d7526042d302abf160f7ed Mon Sep 17 00:00:00 2001 From: snaury Date: Wed, 26 Nov 2025 10:27:28 +0300 Subject: [PATCH 1/3] Delay monlib index SortPages until the next render MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit YDB динамически регистрирует страницы запускающихся акторов в некоторых сервисах, при этом страницы в индексе хочется иметь сортированными. При одновременной регистрации большого кол-ва акторов множественные вызовы SortPages приводят к тому, что их регистрация растёт квадратично. Мне кажется лучше откладывать сортировку страниц до следующего рендера. commit_hash:22e547b6c8d2ce0c1fabebe985793520ec932f30 --- library/cpp/monlib/service/pages/index_mon_page.cpp | 10 +++++++--- library/cpp/monlib/service/pages/index_mon_page.h | 1 + 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/library/cpp/monlib/service/pages/index_mon_page.cpp b/library/cpp/monlib/service/pages/index_mon_page.cpp index cb8423fe8692..1387e350ca45 100644 --- a/library/cpp/monlib/service/pages/index_mon_page.cpp +++ b/library/cpp/monlib/service/pages/index_mon_page.cpp @@ -52,6 +52,12 @@ void TIndexMonPage::Output(IMonHttpRequest& request) { void TIndexMonPage::OutputIndex(IOutputStream& out, bool pathEndsWithSlash) { TGuard g(Mtx); + if (SortPagesPending) { + Pages.sort([](const TMonPagePtr& a, const TMonPagePtr& b) { + return AsciiCompareIgnoreCase(a->GetTitle(), b->GetTitle()) < 0; + }); + SortPagesPending = false; + } for (auto& Page : Pages) { IMonPage* page = Page.Get(); if (page->IsInIndex()) { @@ -164,9 +170,7 @@ void TIndexMonPage::OutputBody(IMonHttpRequest& req) { void TIndexMonPage::SortPages() { TGuard g(Mtx); - Pages.sort([](const TMonPagePtr& a, const TMonPagePtr& b) { - return AsciiCompareIgnoreCase(a->GetTitle(), b->GetTitle()) < 0; - }); + SortPagesPending = true; } void TIndexMonPage::ClearPages() { diff --git a/library/cpp/monlib/service/pages/index_mon_page.h b/library/cpp/monlib/service/pages/index_mon_page.h index 0aaf826d4694..383e676bef30 100644 --- a/library/cpp/monlib/service/pages/index_mon_page.h +++ b/library/cpp/monlib/service/pages/index_mon_page.h @@ -11,6 +11,7 @@ namespace NMonitoring { TPages Pages; // a list of pages to maintain specific order using TPagesByPath = THashMap; TPagesByPath PagesByPath; + bool SortPagesPending = false; TIndexMonPage(const TString& path, const TString& title) : IMonPage(path, title) From c7aec57a075e3975218a6281ff20c03c73d5e45c Mon Sep 17 00:00:00 2001 From: snaury Date: Wed, 26 Nov 2025 10:25:09 +0300 Subject: [PATCH 2/3] Allow FindSubgroup in dynamic counters to find unknown values MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit YDB использует динамическое дерево счётчиков с множеством опциональных лейблов после идентификатора сервиса, которые могут появляться и меняться. Сервисы при регистрации пытаются найти в каком узле дерева регистрировать счётчики пропуская эти опциональные лейблы, если они уже существуют. Исторически такой пропуск работает через `EnumerateSubgroups`, однако обнаружилось, что если сразу после опциональных лейблов дерево очень сильно ветвится (например по номерам групп в dsproxy), то при регистрации большого кол-ва счётчиков по группам эта регистрация становится квадратичной от количества групп. Хочется добавить новый метод `FindSubgroup(TString)` чтобы искать пропускаемые лейблы за `O(log N)` вместо `O(N)`. commit_hash:7ce074f58a3a90caf535d8fad24eef7ac6782d06 --- .../cpp/monlib/dynamic_counters/counters.cpp | 12 +++++++++ .../cpp/monlib/dynamic_counters/counters.h | 1 + .../monlib/dynamic_counters/counters_ut.cpp | 26 +++++++++++++++++++ 3 files changed, 39 insertions(+) diff --git a/library/cpp/monlib/dynamic_counters/counters.cpp b/library/cpp/monlib/dynamic_counters/counters.cpp index 09455be2d787..3a1ceac70de3 100644 --- a/library/cpp/monlib/dynamic_counters/counters.cpp +++ b/library/cpp/monlib/dynamic_counters/counters.cpp @@ -141,6 +141,18 @@ TIntrusivePtr TDynamicCounters::GetSubgroup(const TString& nam return res; } +TIntrusivePtr TDynamicCounters::FindSubgroup(const TString& name) const { + TReadGuard g(Lock); + const auto it = Counters.lower_bound({name, TString()}); + if (it != Counters.end() && it->first.LabelName == name) { + const auto it2 = std::next(it); + if (it2 == Counters.end() || it2->first.LabelName != name) { + return AsDynamicCounters(it->second); + } + } + return nullptr; +} + TIntrusivePtr TDynamicCounters::FindSubgroup(const TString& name, const TString& value) const { TReadGuard g(Lock); const auto it = Counters.find({name, value}); diff --git a/library/cpp/monlib/dynamic_counters/counters.h b/library/cpp/monlib/dynamic_counters/counters.h index 49b54737a0f7..d0e409f4b502 100644 --- a/library/cpp/monlib/dynamic_counters/counters.h +++ b/library/cpp/monlib/dynamic_counters/counters.h @@ -331,6 +331,7 @@ namespace NMonitoring { void RemoveSubgroupChain(const std::vector>& chain); TIntrusivePtr GetSubgroup(const TString& name, const TString& value); + TIntrusivePtr FindSubgroup(const TString& name) const; TIntrusivePtr FindSubgroup(const TString& name, const TString& value) const; bool RemoveSubgroup(const TString& name, const TString& value); void ReplaceSubgroup(const TString& name, const TString& value, TIntrusivePtr subgroup); diff --git a/library/cpp/monlib/dynamic_counters/counters_ut.cpp b/library/cpp/monlib/dynamic_counters/counters_ut.cpp index 3591037e0a7d..f60410958513 100644 --- a/library/cpp/monlib/dynamic_counters/counters_ut.cpp +++ b/library/cpp/monlib/dynamic_counters/counters_ut.cpp @@ -339,4 +339,30 @@ Y_UNIT_TEST_SUITE(TDynamicCountersTest) { histogram = rootGroup->FindNamedHistogram("name", "histogram2"); UNIT_ASSERT(histogram); } + + Y_UNIT_TEST(FindSubgroup) { + TDynamicCounterPtr rootGroup(new TDynamicCounters()); + + auto a = rootGroup->GetSubgroup("a", "1"); + auto b1 = rootGroup->GetSubgroup("b", "1"); + auto b2 = rootGroup->GetSubgroup("b", "2"); + auto c = rootGroup->GetSubgroup("c", "1"); + auto e = rootGroup->GetSubgroup("e", "1"); + + UNIT_ASSERT(a == rootGroup->FindSubgroup("a")); + UNIT_ASSERT(a == rootGroup->FindSubgroup("a", "1")); + UNIT_ASSERT(nullptr == rootGroup->FindSubgroup("a", "2")); + + UNIT_ASSERT(nullptr == rootGroup->FindSubgroup("b")); + UNIT_ASSERT(b1 == rootGroup->FindSubgroup("b", "1")); + UNIT_ASSERT(b2 == rootGroup->FindSubgroup("b", "2")); + UNIT_ASSERT(nullptr == rootGroup->FindSubgroup("b", "3")); + + UNIT_ASSERT(c == rootGroup->FindSubgroup("c")); + UNIT_ASSERT(c == rootGroup->FindSubgroup("c", "1")); + UNIT_ASSERT(nullptr == rootGroup->FindSubgroup("c", "2")); + + UNIT_ASSERT(nullptr == rootGroup->FindSubgroup("d")); + UNIT_ASSERT(nullptr == rootGroup->FindSubgroup("f")); + } } From 96c9a09fa5f56c24dfc8a0f49b0390d0bbb1ae53 Mon Sep 17 00:00:00 2001 From: Aleksei Borzenkov Date: Fri, 28 Nov 2025 16:52:29 +0300 Subject: [PATCH 3/3] Use fixed database labels order and avoid quadratic lookups (#29667) --- ydb/core/base/counters.cpp | 59 ++++++----- ydb/core/base/counters.h | 4 +- ydb/core/mind/labels_maintainer.cpp | 159 ++++++++++++++-------------- ydb/core/mind/tenant_ut_pool.cpp | 3 +- 4 files changed, 119 insertions(+), 106 deletions(-) diff --git a/ydb/core/base/counters.cpp b/ydb/core/base/counters.cpp index 7ca43e194354..1990446fe66c 100644 --- a/ydb/core/base/counters.cpp +++ b/ydb/core/base/counters.cpp @@ -51,7 +51,7 @@ static const THashSet DATABASE_SERVICES static const THashSet DATABASE_ATTRIBUTE_SERVICES = {{ TString("ydb"), TString("datastreams") }}; -static const THashSet DATABASE_ATTRIBUTE_LABELS +static const TVector DATABASE_ATTRIBUTE_LABELS = {{ TString("cloud_id"), TString("folder_id"), TString("database_id") @@ -77,26 +77,20 @@ const THashSet &GetDatabaseAttributeSensorServices() return DATABASE_ATTRIBUTE_SERVICES; } -const THashSet &GetDatabaseAttributeLabels() +const TVector &GetDatabaseAttributeLabels() { return DATABASE_ATTRIBUTE_LABELS; } static TIntrusivePtr SkipLabels(TIntrusivePtr counters, - const THashSet &labels) + const TVector &labels) { - TString name, value; - do { - name = ""; - counters->EnumerateSubgroups([&name, &value, &labels](const TString &n, const TString &v) { - if (labels.contains(n)) { - name = n; - value = v; - } - }); - if (name) - counters = counters->GetSubgroup(name, value); - } while (name); + for (const TString& label : labels) { + auto next = counters->FindSubgroup(label); + if (next) { + counters = next; + } + } return counters; } @@ -122,17 +116,32 @@ TIntrusivePtr GetServiceCountersRoot(TIntrusivePtrGetSubgroup("counters", pair.first); } -static THashSet MakeServiceCountersExtraLabels() { - THashSet extraLabels; - extraLabels.insert(DATABASE_LABEL); - extraLabels.insert(SLOT_LABEL); - extraLabels.insert(HOST_LABEL); - extraLabels.insert(DATABASE_ATTRIBUTE_LABELS.begin(), - DATABASE_ATTRIBUTE_LABELS.end()); +static TVector MakeServiceCountersExtraLabels() { + // NOTE: order of labels should match labels maintainer order for efficiency + TVector extraLabels; + extraLabels.push_back(DATABASE_LABEL); + extraLabels.push_back(SLOT_LABEL); + extraLabels.push_back(HOST_LABEL); + extraLabels.insert(extraLabels.end(), + DATABASE_ATTRIBUTE_LABELS.begin(), + DATABASE_ATTRIBUTE_LABELS.end()); return extraLabels; } -static const THashSet SERVICE_COUNTERS_EXTRA_LABELS = MakeServiceCountersExtraLabels(); +static const TVector SERVICE_COUNTERS_EXTRA_LABELS = MakeServiceCountersExtraLabels(); + +static TIntrusivePtr SkipExtraLabels(TIntrusivePtr counters) { + for (;;) { + // Keep trying as long as there is something to skip + auto next = SkipLabels(counters, SERVICE_COUNTERS_EXTRA_LABELS); + if (next == counters) { + break; + } + counters = next; + } + + return counters; +} TIntrusivePtr GetServiceCounters(TIntrusivePtr root, const TString &service, bool skipAddedLabels) @@ -145,10 +154,10 @@ TIntrusivePtr GetServiceCounters(TIntrusivePtrGetSubgroup("counters", "utils"); - utils = SkipLabels(utils, SERVICE_COUNTERS_EXTRA_LABELS); + utils = SkipExtraLabels(utils); auto lookupCounter = utils->GetSubgroup("component", service)->GetCounter("CounterLookups", true); res->SetLookupCounter(lookupCounter); res->SetOnLookup(OnCounterLookup); diff --git a/ydb/core/base/counters.h b/ydb/core/base/counters.h index 6abfe3cae944..aadd9ff32cd5 100644 --- a/ydb/core/base/counters.h +++ b/ydb/core/base/counters.h @@ -4,6 +4,7 @@ #include #include +#include namespace NKikimr { constexpr char DATABASE_LABEL[] = "database"; @@ -22,7 +23,8 @@ namespace NKikimr { const THashSet &GetDatabaseSensorServices(); // Get list of services which use top-level database attribute labels for own sensors. const THashSet &GetDatabaseAttributeSensorServices(); - const THashSet &GetDatabaseAttributeLabels(); + // Get a list of attribute labels (order is important) + const TVector &GetDatabaseAttributeLabels(); // Drop all extra labels. void ReplaceSubgroup(TIntrusivePtr<::NMonitoring::TDynamicCounters> root, const TString &service); } // namespace NKikimr diff --git a/ydb/core/mind/labels_maintainer.cpp b/ydb/core/mind/labels_maintainer.cpp index 3f07bc5a02e4..76d39389345a 100644 --- a/ydb/core/mind/labels_maintainer.cpp +++ b/ydb/core/mind/labels_maintainer.cpp @@ -167,15 +167,9 @@ class TLabelsMaintainer : public TActorBootstrapped { } void RemoveLabels(const TActorContext &ctx) - { - RemoveDatabaseLabels(ctx); - RemoveAttributeLabels(ctx); - } - - void RemoveDatabaseLabels(const TActorContext &ctx) { auto root = AppData(ctx)->Counters; - for (auto &service : DatabaseSensorServices) { + for (auto &service : AllSensorServices) { LOG_DEBUG_S(ctx, NKikimrServices::LABELS_MAINTAINER, "Removing database labels from " << service << " counters"); @@ -192,97 +186,99 @@ class TLabelsMaintainer : public TActorBootstrapped { } } - void RemoveAttributeLabels(const TActorContext &ctx) + void AddLabels(const TActorContext &ctx) { - auto root = AppData(ctx)->Counters; - for (auto &service : DatabaseAttributeSensorServices) { - LOG_DEBUG_S(ctx, NKikimrServices::LABELS_MAINTAINER, - "Removing database attribute labels from " << service << " counters"); + // NOTE: order of labels should match skip order in GetServiceCounters + TSmallVec> dbLabels; + TSmallVec> attrLabels; - ReplaceSubgroup(root, service); + if (DatabaseLabelsEnabled && CurrentDatabaseLabel) { + if (GroupAllMetrics) { + dbLabels.push_back({DATABASE_LABEL, ""}); + } else { + dbLabels.push_back({DATABASE_LABEL, CurrentDatabaseLabel}); + } + + dbLabels.push_back({SLOT_LABEL, "static"}); + if (!CurrentHostLabel.empty()) { + dbLabels.push_back({HOST_LABEL, CurrentHostLabel}); + } } - } - void AddLabels(const TActorContext &ctx) - { - AddDatabaseLabels(ctx); - AddAttributeLabels(ctx); + if (DatabaseAttributeLabelsEnabled) { + for (auto &attr : GetDatabaseAttributeLabels()) { + if (CurrentAttributes.contains(attr)) { + attrLabels.push_back(*CurrentAttributes.find(attr)); + } + } + } + + if (!dbLabels.empty() || !attrLabels.empty()) { + AddLabelsToServices(ctx, AllSensorServices, dbLabels, attrLabels); + } } - void AddDatabaseLabels(const TActorContext &ctx) + void AddLabelsToServices(const TActorContext& ctx, + const THashSet& services, + const TSmallVec>& dbLabels, + const TSmallVec>& attrLabels) { - if (!DatabaseLabelsEnabled) - return; - - if (!CurrentDatabaseLabel) - return; - auto root = AppData(ctx)->Counters; - TSmallVec> labels; - if (GroupAllMetrics) { - labels.push_back({DATABASE_LABEL, ""}); - } else { - labels.push_back({DATABASE_LABEL, CurrentDatabaseLabel}); - } + for (const auto& service : services) { + bool needDbLabels = DatabaseSensorServices.contains(service) && !dbLabels.empty(); + bool needAttrLabels = DatabaseAttributeSensorServices.contains(service) && !attrLabels.empty(); + if (!needDbLabels && !needAttrLabels) { + continue; + } - labels.push_back({SLOT_LABEL, "static"}); - if (!CurrentHostLabel.empty()) { - labels.push_back({"host", CurrentHostLabel}); - } + const auto [svc, subSvc] = ExtractSubServiceName(service); - AddLabelsToServices(ctx, labels, DatabaseSensorServices); - } + // Find current subgroup and corresponding root and label + auto serviceRoot = root; + std::pair serviceLabel = { "counters", svc }; + auto oldGroup = serviceRoot->GetSubgroup(serviceLabel.first, serviceLabel.second); + if (!subSvc.empty()) { + serviceRoot = oldGroup; + serviceLabel = { "subsystem", subSvc }; + oldGroup = serviceRoot->GetSubgroup(serviceLabel.first, serviceLabel.second); + } - void AddLabelsToServices(const TActorContext& ctx, const TSmallVec> &labels, const THashSet &services) { - if (!labels.empty()) { - auto root = AppData(ctx)->Counters; - for(auto &service: services) { - LOG_DEBUG_S(ctx, NKikimrServices::LABELS_MAINTAINER, - "Add labels to service " << service << " counters" - << " labels=" << PrintLabels(labels)); - const auto &[svc, subSvc] = ExtractSubServiceName(service); - auto oldGroup = root->GetSubgroup("counters", svc); - if (!subSvc.empty()) - oldGroup = oldGroup->GetSubgroup("subsystem", subSvc); - TIntrusivePtr<::NMonitoring::TDynamicCounters> serviceGroup = new ::NMonitoring::TDynamicCounters; - TIntrusivePtr<::NMonitoring::TDynamicCounters> curGroup = serviceGroup; - - const auto* actualLabels = &labels; - - TSmallVec> ydbLabels; - if (DatabaseAttributeSensorServices.contains(service)) { - // explicitly remove "slot" label for external services ("ydb") - ydbLabels = labels; - if (auto it = std::find_if(ydbLabels.begin(), ydbLabels.end(), [](auto& el){ return el.first == SLOT_LABEL; }); - it != ydbLabels.end()) - { - ydbLabels.erase(it); - actualLabels = &ydbLabels; - } + TIntrusivePtr<::NMonitoring::TDynamicCounters> newGroup = new ::NMonitoring::TDynamicCounters; + TIntrusivePtr<::NMonitoring::TDynamicCounters> curGroup = newGroup; + + std::pair lastLabel; + auto processLabel = [&](const auto& label) { + // Explicitly remove "slot" label for external services ("ydb") + if (DatabaseAttributeSensorServices.contains(service) && label.first == SLOT_LABEL) { + return; } - for (size_t i = 0; i < actualLabels->size() - 1; ++i) { - curGroup = curGroup->GetSubgroup((*actualLabels)[i].first, (*actualLabels)[i].second); + if (!lastLabel.first.empty()) { + curGroup = curGroup->GetSubgroup(lastLabel.first, lastLabel.second); } - curGroup->RegisterSubgroup(actualLabels->back().first, actualLabels->back().second, oldGroup); + lastLabel = label; + }; - auto rt = GetServiceCountersRoot(root, service); - rt->ReplaceSubgroup(subSvc.empty() ? "counters" : "subsystem", subSvc.empty() ? svc : subSvc, serviceGroup); + if (needDbLabels) { + for (const auto& label : dbLabels) { + processLabel(label); + } } - } - } - void AddAttributeLabels(const TActorContext &ctx) - { - if (!DatabaseAttributeLabelsEnabled) - return; + if (needAttrLabels) { + for (const auto& label : attrLabels) { + processLabel(label); + } + } - TSmallVec> labels; - for (auto &attr : GetDatabaseAttributeLabels()) - if (CurrentAttributes.contains(attr)) - labels.push_back(*CurrentAttributes.find(attr)); + if (lastLabel.first.empty()) { + // No labels to add + continue; + } - AddLabelsToServices(ctx, labels, DatabaseAttributeSensorServices); + curGroup->RegisterSubgroup(lastLabel.first, lastLabel.second, oldGroup); + serviceRoot->ReplaceSubgroup(serviceLabel.first, serviceLabel.second, newGroup); + } } void ApplyConfig(const NKikimrConfig::TMonitoringConfig &config, @@ -322,6 +318,10 @@ class TLabelsMaintainer : public TActorBootstrapped { if (DatabaseAttributeSensorServices.empty()) DatabaseAttributeSensorServices = GetDatabaseAttributeSensorServices(); + AllSensorServices.clear(); + AllSensorServices.insert(DatabaseSensorServices.begin(), DatabaseSensorServices.end()); + AllSensorServices.insert(DatabaseAttributeSensorServices.begin(), DatabaseAttributeSensorServices.end()); + if (!InitializedLocalOptions) { InitializedLocalOptions = true; DataCenter = config.GetDataCenter(); @@ -376,6 +376,7 @@ class TLabelsMaintainer : public TActorBootstrapped { THashSet DatabaseSensorServices; THashSet DatabaseAttributeSensorServices; + THashSet AllSensorServices; TString NoneDatabasetLabelValue; TString MultipleDatabaseLabelValue; diff --git a/ydb/core/mind/tenant_ut_pool.cpp b/ydb/core/mind/tenant_ut_pool.cpp index 2cd7be89bf63..45321fa1be32 100644 --- a/ydb/core/mind/tenant_ut_pool.cpp +++ b/ydb/core/mind/tenant_ut_pool.cpp @@ -57,7 +57,8 @@ void CheckLabels(TIntrusivePtr<::NMonitoring::TDynamicCounters> counters, allServices.insert(attrServices.begin(), attrServices.end()); for (auto &service : allServices) { - THashSet allLabels = GetDatabaseAttributeLabels(); + const TVector& attrLabelNames = GetDatabaseAttributeLabels(); + THashSet allLabels(attrLabelNames.begin(), attrLabelNames.end()); allLabels.insert(DATABASE_LABEL); allLabels.insert(SLOT_LABEL);