Skip to content

Commit 96c9a09

Browse files
snaurysnaury
committed
Use fixed database labels order and avoid quadratic lookups (#29667)
1 parent c7aec57 commit 96c9a09

File tree

4 files changed

+119
-106
lines changed

4 files changed

+119
-106
lines changed

‎ydb/core/base/counters.cpp‎

Lines changed: 34 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ static const THashSet<TString> DATABASE_SERVICES
5151
static const THashSet<TString> DATABASE_ATTRIBUTE_SERVICES
5252
= {{ TString("ydb"), TString("datastreams") }};
5353

54-
static const THashSet<TString> DATABASE_ATTRIBUTE_LABELS
54+
static const TVector<TString> DATABASE_ATTRIBUTE_LABELS
5555
= {{ TString("cloud_id"),
5656
TString("folder_id"),
5757
TString("database_id")
@@ -77,26 +77,20 @@ const THashSet<TString> &GetDatabaseAttributeSensorServices()
7777
return DATABASE_ATTRIBUTE_SERVICES;
7878
}
7979

80-
const THashSet<TString> &GetDatabaseAttributeLabels()
80+
const TVector<TString> &GetDatabaseAttributeLabels()
8181
{
8282
return DATABASE_ATTRIBUTE_LABELS;
8383
}
8484

8585
static TIntrusivePtr<TDynamicCounters> SkipLabels(TIntrusivePtr<TDynamicCounters> counters,
86-
const THashSet<TString> &labels)
86+
const TVector<TString> &labels)
8787
{
88-
TString name, value;
89-
do {
90-
name = "";
91-
counters->EnumerateSubgroups([&name, &value, &labels](const TString &n, const TString &v) {
92-
if (labels.contains(n)) {
93-
name = n;
94-
value = v;
95-
}
96-
});
97-
if (name)
98-
counters = counters->GetSubgroup(name, value);
99-
} while (name);
88+
for (const TString& label : labels) {
89+
auto next = counters->FindSubgroup(label);
90+
if (next) {
91+
counters = next;
92+
}
93+
}
10094

10195
return counters;
10296
}
@@ -122,17 +116,32 @@ TIntrusivePtr<TDynamicCounters> GetServiceCountersRoot(TIntrusivePtr<TDynamicCou
122116
return root->GetSubgroup("counters", pair.first);
123117
}
124118

125-
static THashSet<TString> MakeServiceCountersExtraLabels() {
126-
THashSet<TString> extraLabels;
127-
extraLabels.insert(DATABASE_LABEL);
128-
extraLabels.insert(SLOT_LABEL);
129-
extraLabels.insert(HOST_LABEL);
130-
extraLabels.insert(DATABASE_ATTRIBUTE_LABELS.begin(),
131-
DATABASE_ATTRIBUTE_LABELS.end());
119+
static TVector<TString> MakeServiceCountersExtraLabels() {
120+
// NOTE: order of labels should match labels maintainer order for efficiency
121+
TVector<TString> extraLabels;
122+
extraLabels.push_back(DATABASE_LABEL);
123+
extraLabels.push_back(SLOT_LABEL);
124+
extraLabels.push_back(HOST_LABEL);
125+
extraLabels.insert(extraLabels.end(),
126+
DATABASE_ATTRIBUTE_LABELS.begin(),
127+
DATABASE_ATTRIBUTE_LABELS.end());
132128
return extraLabels;
133129
}
134130

135-
static const THashSet<TString> SERVICE_COUNTERS_EXTRA_LABELS = MakeServiceCountersExtraLabels();
131+
static const TVector<TString> SERVICE_COUNTERS_EXTRA_LABELS = MakeServiceCountersExtraLabels();
132+
133+
static TIntrusivePtr<TDynamicCounters> SkipExtraLabels(TIntrusivePtr<TDynamicCounters> counters) {
134+
for (;;) {
135+
// Keep trying as long as there is something to skip
136+
auto next = SkipLabels(counters, SERVICE_COUNTERS_EXTRA_LABELS);
137+
if (next == counters) {
138+
break;
139+
}
140+
counters = next;
141+
}
142+
143+
return counters;
144+
}
136145

137146
TIntrusivePtr<TDynamicCounters> GetServiceCounters(TIntrusivePtr<TDynamicCounters> root,
138147
const TString &service, bool skipAddedLabels)
@@ -145,10 +154,10 @@ TIntrusivePtr<TDynamicCounters> GetServiceCounters(TIntrusivePtr<TDynamicCounter
145154
if (!skipAddedLabels)
146155
return res;
147156

148-
res = SkipLabels(res, SERVICE_COUNTERS_EXTRA_LABELS);
157+
res = SkipExtraLabels(res);
149158

150159
auto utils = root->GetSubgroup("counters", "utils");
151-
utils = SkipLabels(utils, SERVICE_COUNTERS_EXTRA_LABELS);
160+
utils = SkipExtraLabels(utils);
152161
auto lookupCounter = utils->GetSubgroup("component", service)->GetCounter("CounterLookups", true);
153162
res->SetLookupCounter(lookupCounter);
154163
res->SetOnLookup(OnCounterLookup);

‎ydb/core/base/counters.h‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#include <library/cpp/monlib/dynamic_counters/counters.h>
55

66
#include <util/generic/hash_set.h>
7+
#include <util/generic/vector.h>
78

89
namespace NKikimr {
910
constexpr char DATABASE_LABEL[] = "database";
@@ -22,7 +23,8 @@ namespace NKikimr {
2223
const THashSet<TString> &GetDatabaseSensorServices();
2324
// Get list of services which use top-level database attribute labels for own sensors.
2425
const THashSet<TString> &GetDatabaseAttributeSensorServices();
25-
const THashSet<TString> &GetDatabaseAttributeLabels();
26+
// Get a list of attribute labels (order is important)
27+
const TVector<TString> &GetDatabaseAttributeLabels();
2628
// Drop all extra labels.
2729
void ReplaceSubgroup(TIntrusivePtr<::NMonitoring::TDynamicCounters> root, const TString &service);
2830
} // namespace NKikimr

‎ydb/core/mind/labels_maintainer.cpp‎

Lines changed: 80 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -167,15 +167,9 @@ class TLabelsMaintainer : public TActorBootstrapped<TLabelsMaintainer> {
167167
}
168168

169169
void RemoveLabels(const TActorContext &ctx)
170-
{
171-
RemoveDatabaseLabels(ctx);
172-
RemoveAttributeLabels(ctx);
173-
}
174-
175-
void RemoveDatabaseLabels(const TActorContext &ctx)
176170
{
177171
auto root = AppData(ctx)->Counters;
178-
for (auto &service : DatabaseSensorServices) {
172+
for (auto &service : AllSensorServices) {
179173
LOG_DEBUG_S(ctx, NKikimrServices::LABELS_MAINTAINER,
180174
"Removing database labels from " << service << " counters");
181175

@@ -192,97 +186,99 @@ class TLabelsMaintainer : public TActorBootstrapped<TLabelsMaintainer> {
192186
}
193187
}
194188

195-
void RemoveAttributeLabels(const TActorContext &ctx)
189+
void AddLabels(const TActorContext &ctx)
196190
{
197-
auto root = AppData(ctx)->Counters;
198-
for (auto &service : DatabaseAttributeSensorServices) {
199-
LOG_DEBUG_S(ctx, NKikimrServices::LABELS_MAINTAINER,
200-
"Removing database attribute labels from " << service << " counters");
191+
// NOTE: order of labels should match skip order in GetServiceCounters
192+
TSmallVec<std::pair<TString, TString>> dbLabels;
193+
TSmallVec<std::pair<TString, TString>> attrLabels;
201194

202-
ReplaceSubgroup(root, service);
195+
if (DatabaseLabelsEnabled && CurrentDatabaseLabel) {
196+
if (GroupAllMetrics) {
197+
dbLabels.push_back({DATABASE_LABEL, ""});
198+
} else {
199+
dbLabels.push_back({DATABASE_LABEL, CurrentDatabaseLabel});
200+
}
201+
202+
dbLabels.push_back({SLOT_LABEL, "static"});
203+
if (!CurrentHostLabel.empty()) {
204+
dbLabels.push_back({HOST_LABEL, CurrentHostLabel});
205+
}
203206
}
204-
}
205207

206-
void AddLabels(const TActorContext &ctx)
207-
{
208-
AddDatabaseLabels(ctx);
209-
AddAttributeLabels(ctx);
208+
if (DatabaseAttributeLabelsEnabled) {
209+
for (auto &attr : GetDatabaseAttributeLabels()) {
210+
if (CurrentAttributes.contains(attr)) {
211+
attrLabels.push_back(*CurrentAttributes.find(attr));
212+
}
213+
}
214+
}
215+
216+
if (!dbLabels.empty() || !attrLabels.empty()) {
217+
AddLabelsToServices(ctx, AllSensorServices, dbLabels, attrLabels);
218+
}
210219
}
211220

212-
void AddDatabaseLabels(const TActorContext &ctx)
221+
void AddLabelsToServices(const TActorContext& ctx,
222+
const THashSet<TString>& services,
223+
const TSmallVec<std::pair<TString, TString>>& dbLabels,
224+
const TSmallVec<std::pair<TString, TString>>& attrLabels)
213225
{
214-
if (!DatabaseLabelsEnabled)
215-
return;
216-
217-
if (!CurrentDatabaseLabel)
218-
return;
219-
220226
auto root = AppData(ctx)->Counters;
221-
TSmallVec<std::pair<TString, TString>> labels;
222-
if (GroupAllMetrics) {
223-
labels.push_back({DATABASE_LABEL, ""});
224-
} else {
225-
labels.push_back({DATABASE_LABEL, CurrentDatabaseLabel});
226-
}
227+
for (const auto& service : services) {
228+
bool needDbLabels = DatabaseSensorServices.contains(service) && !dbLabels.empty();
229+
bool needAttrLabels = DatabaseAttributeSensorServices.contains(service) && !attrLabels.empty();
230+
if (!needDbLabels && !needAttrLabels) {
231+
continue;
232+
}
227233

228-
labels.push_back({SLOT_LABEL, "static"});
229-
if (!CurrentHostLabel.empty()) {
230-
labels.push_back({"host", CurrentHostLabel});
231-
}
234+
const auto [svc, subSvc] = ExtractSubServiceName(service);
232235

233-
AddLabelsToServices(ctx, labels, DatabaseSensorServices);
234-
}
236+
// Find current subgroup and corresponding root and label
237+
auto serviceRoot = root;
238+
std::pair<TString, TString> serviceLabel = { "counters", svc };
239+
auto oldGroup = serviceRoot->GetSubgroup(serviceLabel.first, serviceLabel.second);
240+
if (!subSvc.empty()) {
241+
serviceRoot = oldGroup;
242+
serviceLabel = { "subsystem", subSvc };
243+
oldGroup = serviceRoot->GetSubgroup(serviceLabel.first, serviceLabel.second);
244+
}
235245

236-
void AddLabelsToServices(const TActorContext& ctx, const TSmallVec<std::pair<TString, TString>> &labels, const THashSet<TString> &services) {
237-
if (!labels.empty()) {
238-
auto root = AppData(ctx)->Counters;
239-
for(auto &service: services) {
240-
LOG_DEBUG_S(ctx, NKikimrServices::LABELS_MAINTAINER,
241-
"Add labels to service " << service << " counters"
242-
<< " labels=" << PrintLabels(labels));
243-
const auto &[svc, subSvc] = ExtractSubServiceName(service);
244-
auto oldGroup = root->GetSubgroup("counters", svc);
245-
if (!subSvc.empty())
246-
oldGroup = oldGroup->GetSubgroup("subsystem", subSvc);
247-
TIntrusivePtr<::NMonitoring::TDynamicCounters> serviceGroup = new ::NMonitoring::TDynamicCounters;
248-
TIntrusivePtr<::NMonitoring::TDynamicCounters> curGroup = serviceGroup;
249-
250-
const auto* actualLabels = &labels;
251-
252-
TSmallVec<std::pair<TString, TString>> ydbLabels;
253-
if (DatabaseAttributeSensorServices.contains(service)) {
254-
// explicitly remove "slot" label for external services ("ydb")
255-
ydbLabels = labels;
256-
if (auto it = std::find_if(ydbLabels.begin(), ydbLabels.end(), [](auto& el){ return el.first == SLOT_LABEL; });
257-
it != ydbLabels.end())
258-
{
259-
ydbLabels.erase(it);
260-
actualLabels = &ydbLabels;
261-
}
246+
TIntrusivePtr<::NMonitoring::TDynamicCounters> newGroup = new ::NMonitoring::TDynamicCounters;
247+
TIntrusivePtr<::NMonitoring::TDynamicCounters> curGroup = newGroup;
248+
249+
std::pair<TString, TString> lastLabel;
250+
auto processLabel = [&](const auto& label) {
251+
// Explicitly remove "slot" label for external services ("ydb")
252+
if (DatabaseAttributeSensorServices.contains(service) && label.first == SLOT_LABEL) {
253+
return;
262254
}
263255

264-
for (size_t i = 0; i < actualLabels->size() - 1; ++i) {
265-
curGroup = curGroup->GetSubgroup((*actualLabels)[i].first, (*actualLabels)[i].second);
256+
if (!lastLabel.first.empty()) {
257+
curGroup = curGroup->GetSubgroup(lastLabel.first, lastLabel.second);
266258
}
267-
curGroup->RegisterSubgroup(actualLabels->back().first, actualLabels->back().second, oldGroup);
259+
lastLabel = label;
260+
};
268261

269-
auto rt = GetServiceCountersRoot(root, service);
270-
rt->ReplaceSubgroup(subSvc.empty() ? "counters" : "subsystem", subSvc.empty() ? svc : subSvc, serviceGroup);
262+
if (needDbLabels) {
263+
for (const auto& label : dbLabels) {
264+
processLabel(label);
265+
}
271266
}
272-
}
273-
}
274267

275-
void AddAttributeLabels(const TActorContext &ctx)
276-
{
277-
if (!DatabaseAttributeLabelsEnabled)
278-
return;
268+
if (needAttrLabels) {
269+
for (const auto& label : attrLabels) {
270+
processLabel(label);
271+
}
272+
}
279273

280-
TSmallVec<std::pair<TString, TString>> labels;
281-
for (auto &attr : GetDatabaseAttributeLabels())
282-
if (CurrentAttributes.contains(attr))
283-
labels.push_back(*CurrentAttributes.find(attr));
274+
if (lastLabel.first.empty()) {
275+
// No labels to add
276+
continue;
277+
}
284278

285-
AddLabelsToServices(ctx, labels, DatabaseAttributeSensorServices);
279+
curGroup->RegisterSubgroup(lastLabel.first, lastLabel.second, oldGroup);
280+
serviceRoot->ReplaceSubgroup(serviceLabel.first, serviceLabel.second, newGroup);
281+
}
286282
}
287283

288284
void ApplyConfig(const NKikimrConfig::TMonitoringConfig &config,
@@ -322,6 +318,10 @@ class TLabelsMaintainer : public TActorBootstrapped<TLabelsMaintainer> {
322318
if (DatabaseAttributeSensorServices.empty())
323319
DatabaseAttributeSensorServices = GetDatabaseAttributeSensorServices();
324320

321+
AllSensorServices.clear();
322+
AllSensorServices.insert(DatabaseSensorServices.begin(), DatabaseSensorServices.end());
323+
AllSensorServices.insert(DatabaseAttributeSensorServices.begin(), DatabaseAttributeSensorServices.end());
324+
325325
if (!InitializedLocalOptions) {
326326
InitializedLocalOptions = true;
327327
DataCenter = config.GetDataCenter();
@@ -376,6 +376,7 @@ class TLabelsMaintainer : public TActorBootstrapped<TLabelsMaintainer> {
376376

377377
THashSet<TString> DatabaseSensorServices;
378378
THashSet<TString> DatabaseAttributeSensorServices;
379+
THashSet<TString> AllSensorServices;
379380

380381
TString NoneDatabasetLabelValue;
381382
TString MultipleDatabaseLabelValue;

‎ydb/core/mind/tenant_ut_pool.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,8 @@ void CheckLabels(TIntrusivePtr<::NMonitoring::TDynamicCounters> counters,
5757
allServices.insert(attrServices.begin(), attrServices.end());
5858

5959
for (auto &service : allServices) {
60-
THashSet<TString> allLabels = GetDatabaseAttributeLabels();
60+
const TVector<TString>& attrLabelNames = GetDatabaseAttributeLabels();
61+
THashSet<TString> allLabels(attrLabelNames.begin(), attrLabelNames.end());
6162
allLabels.insert(DATABASE_LABEL);
6263
allLabels.insert(SLOT_LABEL);
6364

0 commit comments

Comments
 (0)