Skip to content

Commit 1309585

Browse files
jepett0jepett0
committed
Minor refactoring + log level increase in some unit tests
Log level needs to be TRACE to see the raw table stats received by the SchemeShard
1 parent 4b411f4 commit 1309585

10 files changed

+50
-37
lines changed

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

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -276,19 +276,20 @@ class TTenantsManager::TTxCreateTenant : public TTransactionBase<TTenantsManager
276276
return Error(Ydb::StatusIds::BAD_REQUEST,
277277
TStringBuilder() << "Overall data size soft quota (" << softQuota << ")"
278278
<< " of the database " << path
279-
<< " must be smaller than the hard quota (" << hardQuota << ")",
279+
<< " must be less than or equal to the hard quota (" << hardQuota << ")",
280280
ctx
281281
);
282282
}
283-
for (const auto& storageQuota : quotas.storage_quotas()) {
284-
const auto unitHardQuota = storageQuota.data_size_hard_quota();
285-
const auto unitSoftQuota = storageQuota.data_size_soft_quota();
283+
for (const auto& storageUnitQuota : quotas.storage_quotas()) {
284+
const auto unitHardQuota = storageUnitQuota.data_size_hard_quota();
285+
const auto unitSoftQuota = storageUnitQuota.data_size_soft_quota();
286286
if (unitHardQuota && unitSoftQuota && unitHardQuota < unitSoftQuota) {
287287
return Error(Ydb::StatusIds::BAD_REQUEST,
288288
TStringBuilder() << "Data size soft quota (" << unitSoftQuota << ")"
289-
<< " for a " << storageQuota.unit_kind() << " storage unit "
289+
<< " for a " << storageUnitQuota.unit_kind() << " storage unit "
290290
<< " of the database " << path
291-
<< " must be smaller than the corresponding hard quota (" << unitHardQuota << ")",
291+
<< " must be less than or equal to"
292+
<< " the corresponding hard quota (" << unitHardQuota << ")",
292293
ctx
293294
);
294295
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -279,7 +279,7 @@ VerifyParams(TParamsDelta* delta, const TPathId pathId, const TSubDomainInfo::TP
279279
if (const auto& effectivePools = requestedPools.empty()
280280
? actualPools
281281
: requestedPools;
282-
!CheckStorageQuotasKinds(input.GetDatabaseQuotas(), effectivePools, pathId.ToString(), error)
282+
!CheckStoragePoolsInQuotas(input.GetDatabaseQuotas(), effectivePools, input.GetName(), error)
283283
) {
284284
return paramError(error);
285285
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -290,7 +290,7 @@ class TAlterSubDomain: public TSubOperation {
290290
if (const auto& effectivePools = requestedPools.empty()
291291
? actualPools
292292
: requestedPools;
293-
!CheckStorageQuotasKinds(settings.GetDatabaseQuotas(), effectivePools, path.PathString(), errStr)
293+
!CheckStoragePoolsInQuotas(settings.GetDatabaseQuotas(), effectivePools, path.PathString(), errStr)
294294
) {
295295
result->SetError(NKikimrScheme::StatusInvalidParameter, errStr);
296296
return result;

‎ydb/core/tx/schemeshard/schemeshard__operation_common_subdomain.h‎

Lines changed: 27 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -6,33 +6,43 @@
66
namespace NKikimr {
77
namespace NSchemeShard {
88

9-
inline bool CheckStorageQuotasKinds(const Ydb::Cms::DatabaseQuotas& quotas,
10-
const TVector<TStoragePool>& pools,
11-
const TString& path,
12-
TString& error
9+
inline bool CheckStoragePoolsInQuotas(const Ydb::Cms::DatabaseQuotas& quotas,
10+
const TVector<TStoragePool>& pools,
11+
const TString& path,
12+
TString& error
1313
) {
1414
TVector<TString> quotedKinds;
15+
quotedKinds.reserve(quotas.storage_quotas_size());
1516
for (const auto& storageQuota : quotas.storage_quotas()) {
1617
quotedKinds.emplace_back(storageQuota.unit_kind());
1718
}
1819
Sort(quotedKinds);
19-
const auto uniqueEnd = Unique(quotedKinds.begin(), quotedKinds.end());
20-
if (uniqueEnd != quotedKinds.end()) {
20+
if (const auto equalKinds = AdjacentFind(quotedKinds);
21+
equalKinds != quotedKinds.end()
22+
) {
2123
error = TStringBuilder()
22-
<< "Malformed subdomain request: storage quotas' unit kinds must be unique, but "
23-
<< *uniqueEnd << " appears twice in the storage quotas definition of the " << path << " subdomain.";
24+
<< "Malformed subdomain request: storage kinds in DatabaseQuotas must be unique, but "
25+
<< *equalKinds << " appears twice in the quotas definition of the " << path << " subdomain.";
2426
return false;
2527
}
2628

27-
for (const auto& quotedKind : quotedKinds) {
28-
if (!AnyOf(pools, [&quotedKind](const TStoragePool& pool) {
29-
return pool.GetKind() == quotedKind;
30-
})) {
31-
error = TStringBuilder()
32-
<< "Malformed subdomain request: cannot set a " << quotedKind << " storage quota, "
33-
<< "because no storage pool in the subdomain " << path << " has the specified kind.";
34-
return false;
35-
}
29+
TVector<TString> existingKinds;
30+
existingKinds.reserve(pools.size());
31+
for (const auto& pool : pools) {
32+
existingKinds.emplace_back(pool.GetKind());
33+
}
34+
Sort(existingKinds);
35+
TVector<TString> unknownKinds;
36+
SetDifference(quotedKinds.begin(), quotedKinds.end(),
37+
existingKinds.begin(), existingKinds.end(),
38+
std::back_inserter(unknownKinds)
39+
);
40+
if (!unknownKinds.empty()) {
41+
error = TStringBuilder()
42+
<< "Malformed subdomain request: cannot set storage quotas of the following kinds: " << JoinSeq(", ", unknownKinds)
43+
<< ", because no storage pool in the subdomain " << path << " has the specified kinds. "
44+
<< "Existing storage kinds are: " << JoinSeq(", ", existingKinds);
45+
return false;
3646
}
3747
return true;
3848
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -295,7 +295,7 @@ class TCreateSubDomain: public TSubOperation {
295295

296296
if (settings.HasDatabaseQuotas()) {
297297
if (!requestedStoragePools.empty()
298-
&& !CheckStorageQuotasKinds(settings.GetDatabaseQuotas(), requestedStoragePools, dstPath.PathString(), errStr)
298+
&& !CheckStoragePoolsInQuotas(settings.GetDatabaseQuotas(), requestedStoragePools, dstPath.PathString(), errStr)
299299
) {
300300
result->SetError(NKikimrScheme::StatusInvalidParameter, errStr);
301301
return result;

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -173,8 +173,8 @@ TPartitionStats TTxStoreTableStats::PrepareStats(const TActorContext& ctx,
173173
indexSize += channelStats.GetIndexSize();
174174
} else {
175175
LOG_DEBUG_S(ctx, NKikimrServices::FLAT_TX_SCHEMESHARD,
176-
"PrepareStats: the subdomain has no info about the datashard's channel "
177-
<< channelStats.GetChannel()
176+
"PrepareStats: SchemeShard has no info on DataShard "
177+
<< rec.GetDatashardId() << " channel " << channelStats.GetChannel() << " binding"
178178
);
179179
}
180180
}

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

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1577,19 +1577,21 @@ void TAggregatedStats::UpdateShardStats(TShardIdx datashardIdx, const TPartition
15771577
Aggregated.IndexSize += (newStats.IndexSize - oldStats.IndexSize);
15781578
for (const auto& [poolKind, newStoragePoolStats] : newStats.StoragePoolsStats) {
15791579
auto* aggregatedStoragePoolStats = Aggregated.StoragePoolsStats.FindPtr(poolKind);
1580+
const auto* oldStoragePoolStats = oldStats.StoragePoolsStats.FindPtr(poolKind);
15801581
if (aggregatedStoragePoolStats) {
1581-
const auto* oldStoragePoolStats = oldStats.StoragePoolsStats.FindPtr(poolKind);
1582-
15831582
// Missing old stats for a particular storage pool are interpreted as if this data
15841583
// has just been written to the datashard and we need to increment the aggregate by the entire new stats' sizes.
15851584
aggregatedStoragePoolStats->DataSize += newStoragePoolStats.DataSize - (oldStoragePoolStats ? oldStoragePoolStats->DataSize : 0u);
15861585
aggregatedStoragePoolStats->IndexSize += newStoragePoolStats.IndexSize - (oldStoragePoolStats ? oldStoragePoolStats->IndexSize : 0u);
15871586
} else {
1587+
Y_ABORT_UNLESS(!oldStoragePoolStats, "Old stats are present, but they haven't been aggregated.");
15881588
Aggregated.StoragePoolsStats.emplace(poolKind, newStoragePoolStats);
15891589
}
15901590
}
15911591
for (const auto& [poolKind, oldStoragePoolStats] : oldStats.StoragePoolsStats) {
1592-
if (const auto* newStoragePoolStats = newStats.StoragePoolsStats.FindPtr(poolKind); !newStoragePoolStats) {
1592+
if (const auto* newStoragePoolStats = newStats.StoragePoolsStats.FindPtr(poolKind);
1593+
!newStoragePoolStats
1594+
) {
15931595
auto* aggregatedStoragePoolStats = Aggregated.StoragePoolsStats.FindPtr(poolKind);
15941596
Y_ABORT_UNLESS(aggregatedStoragePoolStats, "Old stats are present, but they haven't been aggregated.");
15951597

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -601,7 +601,7 @@ Y_UNIT_TEST_SUITE(TStoragePoolsStatsPersistence) {
601601
TTestBasicRuntime runtime;
602602

603603
runtime.SetLogPriority(NKikimrServices::TX_DATASHARD, NLog::PRI_DEBUG);
604-
runtime.SetLogPriority(NKikimrServices::FLAT_TX_SCHEMESHARD, NActors::NLog::PRI_DEBUG);
604+
runtime.SetLogPriority(NKikimrServices::FLAT_TX_SCHEMESHARD, NActors::NLog::PRI_TRACE);
605605

606606
TTestEnvOptions opts;
607607
opts.DisableStatsBatching(true);
@@ -662,7 +662,7 @@ Y_UNIT_TEST_SUITE(TStoragePoolsStatsPersistence) {
662662
UNIT_ASSERT_VALUES_EQUAL(NKikimr::CompactTable(runtime, datashard, ResolveTableId(runtime, "/MyRoot/SomeTable")).GetStatus(),
663663
NKikimrTxDataShard::TEvCompactTableResult::OK
664664
);
665-
// we wait for at least 1 part count, because it means that the table has been compacted
665+
// we wait for at least 1 part count, because it signals that the stats have been recalculated after compaction
666666
WaitTableStats(runtime, datashard, 1, rowsCount).GetTableStats();
667667

668668
auto checkUsage = [&poolsKinds](ui64 totalUsage, const auto& poolUsage) {

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3364,7 +3364,7 @@ Y_UNIT_TEST_SUITE(TStoragePoolsQuotasTest) {
33643364

33653365
Y_UNIT_TEST_FLAG(DisableWritesToDatabase, IsExternalSubdomain) {
33663366
TTestBasicRuntime runtime;
3367-
runtime.SetLogPriority(NKikimrServices::FLAT_TX_SCHEMESHARD, NLog::PRI_DEBUG);
3367+
runtime.SetLogPriority(NKikimrServices::FLAT_TX_SCHEMESHARD, NLog::PRI_TRACE);
33683368

33693369
TTestEnvOptions opts;
33703370
opts.DisableStatsBatching(true);
@@ -3486,7 +3486,7 @@ Y_UNIT_TEST_SUITE(TStoragePoolsQuotasTest) {
34863486

34873487
Y_UNIT_TEST_FLAG(QuoteNonexistentPool, IsExternalSubdomain) {
34883488
TTestBasicRuntime runtime;
3489-
runtime.SetLogPriority(NKikimrServices::FLAT_TX_SCHEMESHARD, NLog::PRI_DEBUG);
3489+
runtime.SetLogPriority(NKikimrServices::FLAT_TX_SCHEMESHARD, NLog::PRI_TRACE);
34903490

34913491
TTestEnvOptions opts;
34923492
TTestEnv env(runtime, opts);
@@ -3535,7 +3535,7 @@ Y_UNIT_TEST_SUITE(TStoragePoolsQuotasTest) {
35353535
// To fix the test you need to update canonical quotas and / or batch sizes.
35363536
Y_UNIT_TEST_FLAG(DifferentQuotasInteraction, IsExternalSubdomain) {
35373537
TTestBasicRuntime runtime;
3538-
runtime.SetLogPriority(NKikimrServices::FLAT_TX_SCHEMESHARD, NLog::PRI_DEBUG);
3538+
runtime.SetLogPriority(NKikimrServices::FLAT_TX_SCHEMESHARD, NLog::PRI_TRACE);
35393539

35403540
TTestEnvOptions opts;
35413541
opts.DisableStatsBatching(true);

‎ydb/services/ydb/ydb_ut.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5574,7 +5574,7 @@ Y_UNIT_TEST(DisableWritesToDatabase) {
55745574
auto sender = runtime.AllocateEdgeActor();
55755575
InitRoot(server, sender);
55765576

5577-
runtime.SetLogPriority(NKikimrServices::FLAT_TX_SCHEMESHARD, NLog::PRI_DEBUG);
5577+
runtime.SetLogPriority(NKikimrServices::FLAT_TX_SCHEMESHARD, NLog::PRI_TRACE);
55785578
NDataShard::gDbStatsReportInterval = TDuration::Seconds(0);
55795579
NDataShard::gDbStatsDataSizeResolution = 1;
55805580
NDataShard::gDbStatsRowCountResolution = 1;

0 commit comments

Comments
 (0)