Skip to content

Commit 7009c75

Browse files
jepett0jepett0
committed
Review fixes
- add some explanations - don't crash, but omit unknown info - debug logs for omitted info - aborts instead of errors for consistency across the code
1 parent 11d3b02 commit 7009c75

File tree

6 files changed

+73
-42
lines changed

6 files changed

+73
-42
lines changed

‎ydb/core/cms/console/console__create_tenant.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -270,9 +270,9 @@ class TTenantsManager::TTxCreateTenant : public TTransactionBase<TTenantsManager
270270
auto hardQuota = quotas.data_size_hard_quota();
271271
auto softQuota = quotas.data_size_soft_quota();
272272
if (hardQuota && softQuota && hardQuota < softQuota
273-
|| AnyOf(quotas.storage_pools_quotas(), [](const auto& storagePoolQuota) {
274-
const auto hardQuota = storagePoolQuota.data_size_hard_quota();
275-
const auto softQuota = storagePoolQuota.data_size_soft_quota();
273+
|| AnyOf(quotas.storage_quotas(), [](const auto& storageQuota) {
274+
const auto hardQuota = storageQuota.data_size_hard_quota();
275+
const auto softQuota = storageQuota.data_size_soft_quota();
276276
return hardQuota && softQuota && hardQuota < softQuota;
277277
})
278278
) {

‎ydb/core/protos/table_stats.proto‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,5 +61,6 @@ message TTableStats {
6161
optional uint64 DataSize = 2;
6262
optional uint64 IndexSize = 3;
6363
}
64+
// Storage pools usage field is added solely for the more informative Describe result for a table in the viewer.
6465
repeated TStoragePoolStats StoragePools = 31;
6566
}

‎ydb/core/tx/schemeshard/schemeshard__table_stats.cpp‎

Lines changed: 31 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,24 @@
77

88
namespace {
99

10-
TVector<TString> MapChannelsToStoragePoolKinds(const NKikimr::TStoragePools& pools, const NKikimr::TChannelsBindings& bindings) {
10+
THashMap<ui32, TString> MapChannelsToStoragePoolKinds(const NActors::TActorContext& ctx,
11+
const NKikimr::TStoragePools& pools,
12+
const NKikimr::TChannelsBindings& bindings
13+
) {
1114
THashMap<TString, TString> nameToKindMap(pools.size());
1215
for (const auto& pool : pools) {
1316
nameToKindMap.emplace(pool.GetName(), pool.GetKind());
1417
}
15-
TVector<TString> channelsMapping;
16-
channelsMapping.reserve(bindings.size());
17-
for (const auto& binding : bindings) {
18-
channelsMapping.emplace_back(nameToKindMap.at(binding.GetStoragePoolName()));
18+
THashMap<ui32, TString> channelsMapping(bindings.size());
19+
for (ui32 channel = 0u; channel < bindings.size(); ++channel) {
20+
if (const auto* poolKind = nameToKindMap.FindPtr(bindings[channel].GetStoragePoolName())) {
21+
channelsMapping.emplace(channel, *poolKind);
22+
} else {
23+
LOG_DEBUG_S(ctx, NKikimrServices::FLAT_TX_SCHEMESHARD,
24+
"MapChannelsToStoragePoolKinds: the subdomain has no info about the storage pool named "
25+
<< bindings[channel].GetStoragePoolName()
26+
);
27+
}
1928
}
2029
return channelsMapping;
2130
}
@@ -110,7 +119,7 @@ class TTxStoreTableStats: public TTxStoreStats<TEvDataShard::TEvPeriodicTableSta
110119
void ScheduleNextBatch(const TActorContext& ctx) override;
111120

112121
template <typename T>
113-
TPartitionStats PrepareStats(const TActorContext& ctx, const T& rec, const TVector<TString>& channelsMapping = {}) const;
122+
TPartitionStats PrepareStats(const TActorContext& ctx, const T& rec, const THashMap<ui32, TString>& channelsMapping = {}) const;
114123
};
115124

116125

@@ -144,7 +153,7 @@ THolder<TProposeRequest> MergeRequest(
144153
template <typename T>
145154
TPartitionStats TTxStoreTableStats::PrepareStats(const TActorContext& ctx,
146155
const T& rec,
147-
const TVector<TString>& channelsMapping
156+
const THashMap<ui32, TString>& channelsMapping
148157
) const {
149158
const auto& tableStats = rec.GetTableStats();
150159
const auto& tabletMetrics = rec.GetTabletMetrics();
@@ -158,10 +167,16 @@ TPartitionStats TTxStoreTableStats::PrepareStats(const TActorContext& ctx,
158167
newStats.LastAccessTime = TInstant::MilliSeconds(tableStats.GetLastAccessTime());
159168
newStats.LastUpdateTime = TInstant::MilliSeconds(tableStats.GetLastUpdateTime());
160169
for (const auto& channelStats : tableStats.GetChannels()) {
161-
const auto& poolKind = channelsMapping.at(channelStats.GetChannel());
162-
auto& [dataSize, indexSize] = newStats.StoragePoolsStats[poolKind];
163-
dataSize += channelStats.GetDataSize();
164-
indexSize += channelStats.GetIndexSize();
170+
if (const auto* poolKind = channelsMapping.FindPtr(channelStats.GetChannel())) {
171+
auto& [dataSize, indexSize] = newStats.StoragePoolsStats[*poolKind];
172+
dataSize += channelStats.GetDataSize();
173+
indexSize += channelStats.GetIndexSize();
174+
} else {
175+
LOG_DEBUG_S(ctx, NKikimrServices::FLAT_TX_SCHEMESHARD,
176+
"PrepareStats: the subdomain has no info about the datashard's channel "
177+
<< channelStats.GetChannel()
178+
);
179+
}
165180
}
166181

167182
newStats.ImmediateTxCompleted = tableStats.GetImmediateTxCompleted();
@@ -231,9 +246,12 @@ bool TTxStoreTableStats::PersistSingleStats(const TPathId& pathId,
231246
}
232247

233248
TShardIdx shardIdx = Self->TabletIdToShardIdx[datashardId];
234-
const auto& shardInfo = Self->ShardInfos[shardIdx];
249+
const auto* shardInfo = Self->ShardInfos.FindPtr(shardIdx);
250+
Y_ABORT_UNLESS(shardInfo, "ShardInfos does not know about the shardIdx returned by TabletIdToShardIdx.");
235251
auto subDomainInfo = Self->ResolveDomainInfo(pathId);
236-
const auto channelsMapping = MapChannelsToStoragePoolKinds(subDomainInfo->EffectiveStoragePools(), shardInfo.BindedChannels);
252+
const auto channelsMapping = MapChannelsToStoragePoolKinds(ctx,
253+
subDomainInfo->EffectiveStoragePools(),
254+
shardInfo->BindedChannels);
237255

238256
LOG_DEBUG_S(ctx, NKikimrServices::FLAT_TX_SCHEMESHARD,
239257
"TTxStoreTableStats.PersistSingleStats: main stats from"

‎ydb/core/tx/schemeshard/schemeshard_info_types.cpp‎

Lines changed: 28 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,10 @@ EDiskUsageStatus CheckStoragePoolsQuotas(const THashMap<TString, TStoragePoolUsa
3838
for (const auto& [poolKind, usage] : storagePoolsUsage) {
3939
if (const auto* quota = storagePoolsQuotas.FindPtr(poolKind)) {
4040
const auto totalSize = usage.DataSize + usage.IndexSize;
41-
if (totalSize > quota->HardQuota) {
41+
if (quota->HardQuota && totalSize > quota->HardQuota) {
4242
return EDiskUsageStatus::AboveHardQuota;
4343
}
44-
if (totalSize > quota->SoftQuota) {
44+
if (quota->SoftQuota && totalSize >= quota->SoftQuota) {
4545
softQuotaExceeded = true;
4646
}
4747
}
@@ -95,19 +95,19 @@ TDiskSpaceQuotas TSubDomainInfo::GetDiskSpaceQuotas() const {
9595
}
9696

9797
THashMap<TString, TQuotasPair> storagePoolsQuotas;
98-
for (const auto& storagePoolQuota : DatabaseQuotas->storage_pools_quotas()) {
99-
ui64 storagePoolHardQuota = storagePoolQuota.data_size_hard_quota();
100-
ui64 storagePoolSoftQuota = storagePoolQuota.data_size_soft_quota();
98+
for (const auto& storageQuota : DatabaseQuotas->storage_quotas()) {
99+
ui64 unitHardQuota = storageQuota.data_size_hard_quota();
100+
ui64 unitSoftQuota = storageQuota.data_size_soft_quota();
101101

102-
if (!storagePoolSoftQuota) {
103-
storagePoolSoftQuota = storagePoolHardQuota;
104-
} else if (!storagePoolHardQuota) {
105-
storagePoolHardQuota = storagePoolSoftQuota;
102+
if (!unitSoftQuota) {
103+
unitSoftQuota = unitHardQuota;
104+
} else if (!unitHardQuota) {
105+
unitHardQuota = unitSoftQuota;
106106
}
107107

108-
storagePoolsQuotas.emplace(storagePoolQuota.storage_pool_kind(), TQuotasPair{
109-
.HardQuota = storagePoolHardQuota,
110-
.SoftQuota = storagePoolSoftQuota
108+
storagePoolsQuotas.emplace(storageQuota.unit_kind(), TQuotasPair{
109+
.HardQuota = unitHardQuota,
110+
.SoftQuota = unitSoftQuota
111111
}
112112
);
113113
}
@@ -139,13 +139,19 @@ bool TSubDomainInfo::CheckDiskSpaceQuotas(IQuotaCounters* counters) {
139139

140140
ui64 totalUsage = TotalDiskSpaceUsage();
141141
const auto storagePoolsUsageStatus = CheckStoragePoolsQuotas(DiskSpaceUsage.Tables.StoragePoolsUsage, quotas.StoragePoolsQuotas);
142-
if (quotas.HardQuota && totalUsage > quotas.HardQuota
143-
|| !quotas.StoragePoolsQuotas.empty() && storagePoolsUsageStatus == EDiskUsageStatus::AboveHardQuota) {
142+
143+
// Quota being equal to zero is treated as if there is no limit on disk space usage.
144+
const bool overallHardQuotaIsExceeded = quotas.HardQuota && totalUsage > quotas.HardQuota;
145+
const bool someStoragePoolHardQuotaIsExceeded = !quotas.StoragePoolsQuotas.empty()
146+
&& storagePoolsUsageStatus == EDiskUsageStatus::AboveHardQuota;
147+
if (overallHardQuotaIsExceeded || someStoragePoolHardQuotaIsExceeded) {
144148
return changeSubdomainState(EDiskUsageStatus::AboveHardQuota);
145149
}
146150

147-
if ((!quotas.SoftQuota || totalUsage < quotas.SoftQuota)
148-
&& (quotas.StoragePoolsQuotas.empty() || storagePoolsUsageStatus == EDiskUsageStatus::BelowSoftQuota)) {
151+
const bool totalUsageIsBelowOverallSoftQuota = !quotas.SoftQuota || totalUsage < quotas.SoftQuota;
152+
const bool allStoragePoolsUsageIsBelowSoftQuota = quotas.StoragePoolsQuotas.empty()
153+
|| storagePoolsUsageStatus == EDiskUsageStatus::BelowSoftQuota;
154+
if (totalUsageIsBelowOverallSoftQuota && allStoragePoolsUsageIsBelowSoftQuota) {
149155
return changeSubdomainState(EDiskUsageStatus::BelowSoftQuota);
150156
}
151157

@@ -1495,6 +1501,11 @@ void TTableInfo::SetPartitioning(TVector<TTableShardInfo>&& newPartitioning) {
14951501
newAggregatedStats.RowCount += newStats.RowCount;
14961502
newAggregatedStats.DataSize += newStats.DataSize;
14971503
newAggregatedStats.IndexSize += newStats.IndexSize;
1504+
for (const auto& [poolKind, newStoragePoolStats] : newStats.StoragePoolsStats) {
1505+
auto& aggregatedStoragePoolStats = newAggregatedStats.StoragePoolsStats[poolKind];
1506+
aggregatedStoragePoolStats.DataSize += newStoragePoolStats.DataSize;
1507+
aggregatedStoragePoolStats.IndexSize += newStoragePoolStats.IndexSize;
1508+
}
14981509
newAggregatedStats.InFlightTxCount += newStats.InFlightTxCount;
14991510
cpuTotal += newStats.GetCurrentRawCpuUsage();
15001511
newAggregatedStats.Memory += newStats.Memory;
@@ -1591,7 +1602,7 @@ void TAggregatedStats::UpdateShardStats(TShardIdx datashardIdx, const TPartition
15911602
for (const auto& [poolKind, oldStoragePoolStats] : oldStats.StoragePoolsStats) {
15921603
if (const auto* newStoragePoolStats = newStats.StoragePoolsStats.FindPtr(poolKind); !newStoragePoolStats) {
15931604
auto* aggregatedStoragePoolStats = Aggregated.StoragePoolsStats.FindPtr(poolKind);
1594-
Y_ENSURE(aggregatedStoragePoolStats); // How else could it be present in the oldStats?
1605+
Y_ABORT_UNLESS(aggregatedStoragePoolStats, "Old stats are present, but they haven't been aggregated.");
15951606

15961607
// Missing new stats for a particular storage pool are interpreted as if this data
15971608
// has been removed from the datashard and we need to subtract the old stats' sizes from the aggregate.

‎ydb/core/tx/schemeshard/ut_subdomain/ut_subdomain.cpp‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3076,8 +3076,8 @@ Y_UNIT_TEST_SUITE(TSchemeShardSubDomainTest) {
30763076
Kind: "quoted_storage_pool_kind"
30773077
}
30783078
DatabaseQuotas {
3079-
storage_pools_quotas {
3080-
storage_pool_kind: "quoted_storage_pool_kind"
3079+
storage_quotas {
3080+
unit_kind: "quoted_storage_pool_kind"
30813081
data_size_hard_quota: 1
30823082
}
30833083
}
@@ -3124,7 +3124,7 @@ Y_UNIT_TEST_SUITE(TSchemeShardSubDomainTest) {
31243124
}
31253125
CheckQuotaExceedance(runtime, "/MyRoot/SomeTenant", false, "after the data insertion");
31263126

3127-
// step 4: compact the table
3127+
// step 4: compact the table (statistics by channels does not appear in the messages from datashards otherwise)
31283128
const auto tableId = ResolveTableId(runtime, "/MyRoot/SomeTenant/SomeTable");
31293129
const auto compactionResult = CompactTable(runtime, shards[0], tableId);
31303130
UNIT_ASSERT_VALUES_EQUAL(compactionResult.GetStatus(), NKikimrTxDataShard::TEvCompactTableResult::OK);
@@ -3178,8 +3178,8 @@ Y_UNIT_TEST_SUITE(TSchemeShardSubDomainTest) {
31783178
Kind: "quoted_storage_pool_kind"
31793179
}
31803180
DatabaseQuotas {
3181-
storage_pools_quotas {
3182-
storage_pool_kind: "quoted_storage_pool_kind"
3181+
storage_quotas {
3182+
unit_kind: "quoted_storage_pool_kind"
31833183
data_size_hard_quota: 1
31843184
}
31853185
}
@@ -3234,7 +3234,7 @@ Y_UNIT_TEST_SUITE(TSchemeShardSubDomainTest) {
32343234
}
32353235
CheckQuotaExceedance(runtime, tenantSchemeShard, "/MyRoot/SomeDatabase", false, "after the data insertion");
32363236

3237-
// step 4: compact the table
3237+
// step 4: compact the table (statistics by channels does not appear in the messages from datashards otherwise)
32383238
const auto tableId = ResolveTableId(runtime, "/MyRoot/SomeDatabase/SomeTable");
32393239
const auto compactionResult = CompactTable(runtime, shards[0], tableId);
32403240
UNIT_ASSERT_VALUES_EQUAL(compactionResult.GetStatus(), NKikimrTxDataShard::TEvCompactTableResult::OK);

‎ydb/public/api/protos/ydb_cms.proto‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -95,12 +95,13 @@ message DatabaseQuotas {
9595
// Default is 1800 (15 minutes).
9696
uint32 ttl_min_run_internal_seconds = 4;
9797

98-
message StoragePoolQuotas {
99-
string storage_pool_kind = 1;
98+
message StorageQuotas {
99+
// in theory an arbitrary string, but in practice HDD or SSD
100+
string unit_kind = 1;
100101
uint64 data_size_hard_quota = 2;
101102
uint64 data_size_soft_quota = 3;
102103
}
103-
repeated StoragePoolQuotas storage_pools_quotas = 6;
104+
repeated StorageQuotas storage_quotas = 6;
104105
}
105106

106107
// Request to create a new database. For successfull creation

0 commit comments

Comments
 (0)