schemeshard: fix stats processing op1, part 3 (#29813)

Fix disk space aggregation for column tables.
Make subdomain and storage pool kind aggregation levels separation more robust.
Add tests on subdomain level aggregation.

Author: ijon

ydb/core/tx/schemeshard/olap/table/table.h

@@ -109,9 +109,9 @@ struct TColumnTableInfo {
         Stats.UpdateShardStats(diskSpaceUsageDelta, shardIdx, newStats, now);
     }
 
-    void UpdateTableStats(TDiskSpaceUsageDelta* diskSpaceUsageDelta, const TShardIdx shardIdx, const TPathId& pathId, const TPartitionStats& newStats, TInstant now) {
+    void UpdateTableStats(const TShardIdx shardIdx, const TPathId& pathId, const TPartitionStats& newStats, TInstant now) {
         Stats.TableStats[pathId].Aggregated.PartCount = GetColumnShards().size();
-        Stats.UpdateTableStats(diskSpaceUsageDelta, shardIdx, pathId, newStats, now);
+        Stats.UpdateTableStats(shardIdx, pathId, newStats, now);
     }
 
     TConclusion<std::shared_ptr<NOlap::NAlter::ISSEntity>> BuildEntity(const TPathId& pathId, const NOlap::NAlter::TEntityInitializationContext& iContext) const;

ydb/core/tx/schemeshard/schemeshard__table_stats.cpp

@@ -459,7 +459,7 @@ bool TTxStoreTableStats::PersistSingleStats(const TPathId& pathId,
                 LOG_TRACE_S(ctx, NKikimrServices::FLAT_TX_SCHEMESHARD,
                             "add stats for exists table with pathId=" << tablePathId);
 
-                Self->ColumnTables.GetVerifiedPtr(tablePathId)->UpdateTableStats(&diskSpaceUsageDelta, shardIdx, tablePathId, newTableStats, now);
+                Self->ColumnTables.GetVerifiedPtr(tablePathId)->UpdateTableStats(shardIdx, tablePathId, newTableStats, now);
             } else {
                 LOG_WARN_S(ctx, NKikimrServices::FLAT_TX_SCHEMESHARD,
                            "failed add stats for table with pathId=" << tablePathId);

ydb/core/tx/schemeshard/schemeshard_info_types.cpp

@@ -279,27 +279,33 @@ void TSubDomainInfo::AggrDiskSpaceUsage(IQuotaCounters* counters, const TPartiti
 }
 
 void TSubDomainInfo::AggrDiskSpaceUsage(IQuotaCounters* counters, const TDiskSpaceUsageDelta& diskSpaceUsageDelta) {
+    // just (in)sanity check, diskSpaceUsageDelta should have at least 1 element
+    if (diskSpaceUsageDelta.empty()) {
+        return;
+    }
     // see filling of diskSpaceUsageDelta in UpdateShardStats()
-    for (const auto& [poolKind, delta] : diskSpaceUsageDelta) {
-        if (poolKind.empty()) {
-            // total space
-            DiskSpaceUsage.Tables.DataSize += delta.DataSize;
-            counters->ChangeDiskSpaceTablesDataBytes(delta.DataSize);
-
-            DiskSpaceUsage.Tables.IndexSize += delta.IndexSize;
-            counters->ChangeDiskSpaceTablesIndexBytes(delta.IndexSize);
-
-            i64 oldTotalBytes = DiskSpaceUsage.Tables.TotalSize;
-            DiskSpaceUsage.Tables.TotalSize = DiskSpaceUsage.Tables.DataSize + DiskSpaceUsage.Tables.IndexSize;
-            i64 newTotalBytes = DiskSpaceUsage.Tables.TotalSize;
-            counters->ChangeDiskSpaceTablesTotalBytes(newTotalBytes - oldTotalBytes);
-        } else {
-            // space separated by storage pool kinds
-            auto& r = DiskSpaceUsage.StoragePoolsUsage[poolKind];
-            r.DataSize += delta.DataSize;
-            r.IndexSize += delta.IndexSize;
-            counters->AddDiskSpaceTables(GetUserFacingStorageType(poolKind), delta.DataSize, delta.IndexSize);
-        }
+    // total space usage, index 0
+    {
+        const auto& [poolKind, delta] = diskSpaceUsageDelta[0];
+
+        DiskSpaceUsage.Tables.DataSize += delta.DataSize;
+        counters->ChangeDiskSpaceTablesDataBytes(delta.DataSize);
+
+        DiskSpaceUsage.Tables.IndexSize += delta.IndexSize;
+        counters->ChangeDiskSpaceTablesIndexBytes(delta.IndexSize);
+
+        i64 oldTotalBytes = DiskSpaceUsage.Tables.TotalSize;
+        DiskSpaceUsage.Tables.TotalSize = DiskSpaceUsage.Tables.DataSize + DiskSpaceUsage.Tables.IndexSize;
+        i64 newTotalBytes = DiskSpaceUsage.Tables.TotalSize;
+        counters->ChangeDiskSpaceTablesTotalBytes(newTotalBytes - oldTotalBytes);
+    }
+    // space usage by storage pool kinds, from index 1 onwards
+    for (size_t i = 1; i < diskSpaceUsageDelta.size(); ++i) {
+        const auto& [poolKind, delta] = diskSpaceUsageDelta[i];
+        auto& r = DiskSpaceUsage.StoragePoolsUsage[poolKind];
+        r.DataSize += delta.DataSize;
+        r.IndexSize += delta.IndexSize;
+        counters->AddDiskSpaceTables(GetUserFacingStorageType(poolKind), delta.DataSize, delta.IndexSize);
     }
 }
 
@@ -1853,10 +1859,9 @@ void TTableAggregatedStats::UpdateShardStats(TDiskSpaceUsageDelta* diskSpaceUsag
             diskSpaceUsageDelta->emplace_back(poolKind, delta);
         }
     }
-    for (const auto& [poolKind, delta] : *diskSpaceUsageDelta) {
-        if (poolKind.empty()) {
-            continue;
-        }
+    // from index 1 onwards (as index 0 holds total space delta)
+    for (size_t i = 1; i < diskSpaceUsageDelta->size(); ++i) {
+        const auto& [poolKind, delta] = (*diskSpaceUsageDelta)[i];
         auto& r = Aggregated.StoragePoolsStats[poolKind];
         r.DataSize += delta.DataSize;
         r.IndexSize += delta.IndexSize;
@@ -1961,10 +1966,11 @@ void TTableAggregatedStats::UpdateShardStatsForFollower(
     oldStats.TopCpuUsage.Update(newStats.TopCpuUsage); // The left is new stats now!
 }
 
-void TAggregatedStats::UpdateTableStats(TDiskSpaceUsageDelta* diskSpaceUsageDelta, TShardIdx shardIdx, const TPathId& pathId, const TPartitionStats& newStats, TInstant now) {
+void TAggregatedStats::UpdateTableStats(TShardIdx shardIdx, const TPathId& pathId, const TPartitionStats& newStats, TInstant now) {
     auto& tableStats = TableStats[pathId];
     tableStats.PartitionStats[shardIdx]; // insert if none
-    tableStats.UpdateShardStats(diskSpaceUsageDelta, shardIdx, newStats, now);
+    TDiskSpaceUsageDelta unused;
+    tableStats.UpdateShardStats(&unused, shardIdx, newStats, now);
 }
 
 void TTableInfo::RegisterSplitMergeOp(TOperationId opId, const TTxState& txState) {

ydb/core/tx/schemeshard/schemeshard_info_types.h

@@ -483,7 +483,7 @@ struct TTableAggregatedStats {
 struct TAggregatedStats : public TTableAggregatedStats {
     THashMap<TPathId, TTableAggregatedStats> TableStats;
 
-    void UpdateTableStats(TDiskSpaceUsageDelta* diskSpaceUsageDelta, TShardIdx datashardIdx, const TPathId& pathId, const TPartitionStats& newStats, TInstant now);
+    void UpdateTableStats(TShardIdx datashardIdx, const TPathId& pathId, const TPartitionStats& newStats, TInstant now);
 };
 
 struct TSubDomainInfo;