prevent negative table stats

4 files changed, 61 insertions, 51 deletions

diff --git a/ydb/core/tx/columnshard/columnshard.cpp b/ydb/core/tx/columnshard/columnshard.cpp
index 9fabb7329eb..178384bb202 100644
--- a/ydb/core/tx/columnshard/columnshard.cpp
+++ b/ydb/core/tx/columnshard/columnshard.cpp
@@ -328,6 +328,14 @@ void TColumnShard::SendPeriodicStats() {
             NOlap::TSnapshot lastIndexUpdate = TablesManager.GetPrimaryIndexSafe().LastUpdate();
             auto activeIndexStats = indexStats.Active(); // data stats excluding inactive and evicted
 
+            if (activeIndexStats.Rows < 0 || activeIndexStats.Bytes < 0) {
+                LOG_S_WARN("Negative stats counter. Rows: " << activeIndexStats.Rows
+                    << " Bytes: " << activeIndexStats.Bytes << TabletID());
+
+                activeIndexStats.Rows = (activeIndexStats.Rows < 0) ? 0 : activeIndexStats.Rows;
+                activeIndexStats.Bytes = (activeIndexStats.Bytes < 0) ? 0 : activeIndexStats.Bytes;
+            }
+
             tabletStats->SetRowCount(activeIndexStats.Rows);
             tabletStats->SetDataSize(activeIndexStats.Bytes + TabletCounters->Simple()[COUNTER_COMMITTED_BYTES].Get());
             // TODO: we need row/dataSize counters for evicted data (managed by tablet but stored outside)
diff --git a/ydb/core/tx/columnshard/columnshard__stats_scan.cpp b/ydb/core/tx/columnshard/columnshard__stats_scan.cpp
index 4460e2ed6e5..ceeb90d7fce 100644
--- a/ydb/core/tx/columnshard/columnshard__stats_scan.cpp
+++ b/ydb/core/tx/columnshard/columnshard__stats_scan.cpp
@@ -86,39 +86,39 @@ void TStatsIterator::AppendStats(const std::vector<std::unique_ptr<arrow::ArrayB
     ui64 tabletId = ReadMetadata->TabletId;
     TUInt64 tabletIds[NUM_KINDS] = { tabletId, tabletId, tabletId, tabletId, tabletId };
     TUInt64 rows[NUM_KINDS] = {
-        stats.Inserted.Rows,
-        stats.Compacted.Rows,
-        stats.SplitCompacted.Rows,
-        stats.Inactive.Rows,
-        stats.Evicted.Rows
+        (ui64)stats.Inserted.Rows,
+        (ui64)stats.Compacted.Rows,
+        (ui64)stats.SplitCompacted.Rows,
+        (ui64)stats.Inactive.Rows,
+        (ui64)stats.Evicted.Rows
     };
     TUInt64 bytes[NUM_KINDS] = {
-        stats.Inserted.Bytes,
-        stats.Compacted.Bytes,
-        stats.SplitCompacted.Bytes,
-        stats.Inactive.Bytes,
-        stats.Evicted.Bytes
+        (ui64)stats.Inserted.Bytes,
+        (ui64)stats.Compacted.Bytes,
+        (ui64)stats.SplitCompacted.Bytes,
+        (ui64)stats.Inactive.Bytes,
+        (ui64)stats.Evicted.Bytes
     };
     TUInt64 rawBytes[NUM_KINDS] = {
-        stats.Inserted.RawBytes,
-        stats.Compacted.RawBytes,
-        stats.SplitCompacted.RawBytes,
-        stats.Inactive.RawBytes,
-        stats.Evicted.RawBytes
+        (ui64)stats.Inserted.RawBytes,
+        (ui64)stats.Compacted.RawBytes,
+        (ui64)stats.SplitCompacted.RawBytes,
+        (ui64)stats.Inactive.RawBytes,
+        (ui64)stats.Evicted.RawBytes
     };
     TUInt64 portions[NUM_KINDS] = {
-        stats.Inserted.Portions,
-        stats.Compacted.Portions,
-        stats.SplitCompacted.Portions,
-        stats.Inactive.Portions,
-        stats.Evicted.Portions
+        (ui64)stats.Inserted.Portions,
+        (ui64)stats.Compacted.Portions,
+        (ui64)stats.SplitCompacted.Portions,
+        (ui64)stats.Inactive.Portions,
+        (ui64)stats.Evicted.Portions
     };
     TUInt64 blobs[NUM_KINDS] = {
-        stats.Inserted.Blobs,
-        stats.Compacted.Blobs,
-        stats.SplitCompacted.Blobs,
-        stats.Inactive.Blobs,
-        stats.Evicted.Blobs
+        (ui64)stats.Inserted.Blobs,
+        (ui64)stats.Compacted.Blobs,
+        (ui64)stats.SplitCompacted.Blobs,
+        (ui64)stats.Inactive.Blobs,
+        (ui64)stats.Evicted.Blobs
     };
 
     if (Reverse) {
diff --git a/ydb/core/tx/columnshard/engines/column_engine.h b/ydb/core/tx/columnshard/engines/column_engine.h
index aaa4a26e838..96e90b00e98 100644
--- a/ydb/core/tx/columnshard/engines/column_engine.h
+++ b/ydb/core/tx/columnshard/engines/column_engine.h
@@ -294,19 +294,19 @@ struct TSelectInfo {
 
 struct TColumnEngineStats {
     struct TPortionsStats {
-        ui64 Portions{};
-        ui64 Blobs{};
-        ui64 Rows{};
-        ui64 Bytes{};
-        ui64 RawBytes{};
+        i64 Portions{};
+        i64 Blobs{};
+        i64 Rows{};
+        i64 Bytes{};
+        i64 RawBytes{};
     };
 
-    ui64 Tables{};
-    ui64 Granules{};
-    ui64 EmptyGranules{};
-    ui64 OverloadedGranules{};
-    ui64 ColumnRecords{};
-    ui64 ColumnMetadataBytes{};
+    i64 Tables{};
+    i64 Granules{};
+    i64 EmptyGranules{};
+    i64 OverloadedGranules{};
+    i64 ColumnRecords{};
+    i64 ColumnMetadataBytes{};
     TPortionsStats Inserted{};
     TPortionsStats Compacted{};
     TPortionsStats SplitCompacted{};
@@ -323,11 +323,11 @@ struct TColumnEngineStats {
         };
     }
 
-    ui64 ActivePortions() const { return Inserted.Portions + Compacted.Portions + SplitCompacted.Portions; }
-    ui64 ActiveBlobs() const { return Inserted.Blobs + Compacted.Blobs + SplitCompacted.Blobs; }
-    ui64 ActiveRows() const { return Inserted.Rows + Compacted.Rows + SplitCompacted.Rows; }
-    ui64 ActiveBytes() const { return Inserted.Bytes + Compacted.Bytes + SplitCompacted.Bytes; }
-    ui64 ActiveRawBytes() const { return Inserted.RawBytes + Compacted.RawBytes + SplitCompacted.RawBytes; }
+    i64 ActivePortions() const { return Inserted.Portions + Compacted.Portions + SplitCompacted.Portions; }
+    i64 ActiveBlobs() const { return Inserted.Blobs + Compacted.Blobs + SplitCompacted.Blobs; }
+    i64 ActiveRows() const { return Inserted.Rows + Compacted.Rows + SplitCompacted.Rows; }
+    i64 ActiveBytes() const { return Inserted.Bytes + Compacted.Bytes + SplitCompacted.Bytes; }
+    i64 ActiveRawBytes() const { return Inserted.RawBytes + Compacted.RawBytes + SplitCompacted.RawBytes; }
 
     void Clear() {
         *this = {};
diff --git a/ydb/core/tx/columnshard/engines/column_engine_logs.cpp b/ydb/core/tx/columnshard/engines/column_engine_logs.cpp
index 9eeea895567..2702c410ce2 100644
--- a/ydb/core/tx/columnshard/engines/column_engine_logs.cpp
+++ b/ydb/core/tx/columnshard/engines/column_engine_logs.cpp
@@ -292,6 +292,7 @@ void TColumnEngineForLogs::UpdatePortionStats(TColumnEngineStats& engineStats, c
             srcStats = &engineStats.SplitCompacted;
             break;
         case NOlap::TPortionMeta::INACTIVE:
+            Y_VERIFY_DEBUG(false); // Stale portions are not set INACTIVE. They have IsActive() property instead.
             srcStats = &engineStats.Inactive;
             break;
         case NOlap::TPortionMeta::EVICTED:
@@ -931,8 +932,9 @@ bool TColumnEngineForLogs::ApplyChanges(IDbWrapper& db, const TChanges& changes,
             Y_VERIFY(!portionInfo.IsActive());
 
             ui64 granule = portionInfo.Granule();
-            if (!Granules.contains(granule)) {
-                LOG_S_ERROR("Cannot update portion " << portionInfo << " with unknown granule at tablet " << TabletId);
+            ui64 portion = portionInfo.Portion();
+            if (!Granules.contains(granule) || !Granules[granule]->Portions.contains(portion)) {
+                LOG_S_ERROR("Cannot update unknown portion " << portionInfo << " at tablet " << TabletId);
                 return false;
             }
 
@@ -1170,7 +1172,8 @@ bool TColumnEngineForLogs::ErasePortion(const TPortionInfo& portionInfo, bool ap
     ui64 portion = portionInfo.Portion();
 
     if (!apply) {
-        if (!Granules.contains(granule)) {
+        if (!Granules.contains(granule) || !Granules[granule]->Portions.contains(portion)) {
+            LOG_S_ERROR("Cannot erase unknown portion " << portionInfo << " at tablet " << TabletId);
             return false;
         }
         return true;
@@ -1178,14 +1181,13 @@ bool TColumnEngineForLogs::ErasePortion(const TPortionInfo& portionInfo, bool ap
 
     auto& spg = Granules[granule];
     Y_VERIFY(spg);
-    if (spg->Portions.contains(portion)) {
-        if (updateStats) {
-            UpdatePortionStats(spg->Portions[portion], EStatsUpdateType::ERASE);
-        }
-        spg->Portions.erase(portion);
-    } else {
-        LOG_S_ERROR("Erase for unknown portion " << portionInfo << " at tablet " << TabletId);
+    Y_VERIFY(spg->Portions.contains(portion));
+
+    if (updateStats) {
+        UpdatePortionStats(spg->Portions[portion], EStatsUpdateType::ERASE);
     }
+    spg->Portions.erase(portion);
+
     return true; // It must return true if (apply == true)
 }