issue-2388: counting all MixedBlobs generated by a single Compaction …

…run as a single logical blob in CompactionMap range stats (#2395)
ydb-platform · Oct 31, 2024 · f8490dd · f8490dd
1 parent c2b8ef3
commit f8490dd
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 31 deletions.
diff --git a/cloud/filestore/libs/storage/tablet/tablet_actor_addblob.cpp b/cloud/filestore/libs/storage/tablet/tablet_actor_addblob.cpp
@@ -4,6 +4,8 @@
 #include <cloud/filestore/libs/storage/tablet/model/group_by.h>
 #include <cloud/filestore/libs/storage/tablet/model/profile_log_events.h>
 
+#include <util/generic/hash.h>
+#include <util/generic/hash_set.h>
 #include <util/generic/set.h>
 
 namespace NCloud::NFileStore::NStorage {
@@ -22,7 +24,6 @@ class TAddBlobsExecutor
 private:
     const TString LogTag;
     TIndexTabletActor& Tablet;
-    const ui32 CompactionThreshold;
 
     struct TCompactionRangeInfo
     {
@@ -32,15 +33,10 @@ class TAddBlobsExecutor
     THashMap<ui32, TCompactionRangeInfo> RangeId2CompactionStats;
 
 public:
-    TAddBlobsExecutor(
-            TString logTag,
-            TIndexTabletActor& tablet,
-            ui32 compactionThreshold)
+    TAddBlobsExecutor(TString logTag, TIndexTabletActor& tablet)
         : LogTag(std::move(logTag))
         , Tablet(tablet)
-        , CompactionThreshold(compactionThreshold)
-    {
-    }
+    {}
 
 public:
     void Execute(
@@ -342,29 +338,24 @@ class TAddBlobsExecutor
             rangeIds.insert(rangeId);
         }
 
-        THashMap<ui32, ui32> rangeId2AddedBlobsCount;
+        THashSet<ui32> writtenRangeIds;
         for (auto& blob: args.MixedBlobs) {
             const auto rangeId = Tablet.GetMixedRangeIndex(blob.Blocks);
-            // Incrementing blobs count as there could be multiple blobs
-            // per compacted range see NBS-4424
             if (Tablet.WriteMixedBlocks(db, blob.BlobId, blob.Blocks)) {
-                ++rangeId2AddedBlobsCount[rangeId];
+                writtenRangeIds.insert(rangeId);
             }
 
             rangeIds.insert(rangeId);
         }
 
-        for (const auto& [rangeId, addedBlobsCount]: rangeId2AddedBlobsCount) {
-            auto& stats = AccessCompactionRangeInfo(rangeId).Stats;
-
-            // If addedBlobsCount >= compactionThreshold, then Compaction will
-            // enter an infinite loop
-            // A simple solution is to limit addedBlobsCount by threshold - 1
-            auto increment = addedBlobsCount;
-            if (increment >= CompactionThreshold && CompactionThreshold > 1) {
-                increment = CompactionThreshold - 1;
-            }
-            stats.BlobsCount += increment;
+        for (const auto& rangeId: writtenRangeIds) {
+            // Deliberately incrementing BlobsCount only once per range. The
+            // data belonging to a range might need to be stored in more than
+            // one blob due to hash collisions in the calculation of
+            // <nodeId, blockIndex> -> rangeId mapping. We don't want such
+            // situations to cause extra compactions and we thus treat such
+            // a group of blobs as a single "logical" blob in CompactionMap.
+            ++AccessCompactionRangeInfo(rangeId).Stats.BlobsCount;
         }
 
         // recalculating GarbageBlocksCount for each of the affected ranges
@@ -513,9 +504,7 @@ void TIndexTabletActor::ExecuteTx_AddBlob(
     TTransactionContext& tx,
     TTxIndexTablet::TAddBlob& args)
 {
-    const auto compactionThreshold =
-        ScaleCompactionThreshold(Config->GetCompactionThreshold());
-    TAddBlobsExecutor executor(LogTag, *this, compactionThreshold);
+    TAddBlobsExecutor executor(LogTag, *this);
     executor.Execute(ctx, tx, args);
 }
 

diff --git a/cloud/filestore/libs/storage/tablet/tablet_ut_data_stress.cpp b/cloud/filestore/libs/storage/tablet/tablet_ut_data_stress.cpp
@@ -408,14 +408,14 @@ Y_UNIT_TEST_SUITE(TIndexTabletTest_Data_Stress)
             collidingBlocks.push_back(i * (BlockGroupSize << 16));
         }
 
+        const auto expectedWriteCount = nodeCount * collisions;
         // should be 140 x 64 x 4KiB == 35MiB for builds without slow sanitizers
-        const auto expectedBlockCount = nodeCount * collisions * BlockGroupSize;
+        const auto expectedBlockCount = expectedWriteCount * BlockGroupSize;
         const auto expectedBlobCount = static_cast<ui32>(ceil(
             static_cast<double>(expectedBlockCount) / (4_MB / block)));
         UNIT_ASSERT_C(
             compactionThreshold < expectedBlobCount,
             TStringBuilder() << "expectedBlobCount: " << expectedBlobCount);
-
         for (ui32 i = 0; i < nodeCount; ++i) {
             const auto id = CreateNode(
                 tablet,
@@ -455,8 +455,9 @@ Y_UNIT_TEST_SUITE(TIndexTabletTest_Data_Stress)
             UNIT_ASSERT_VALUES_EQUAL(
                 expectedBlockCount,
                 stats.GetMixedBlocksCount());
+            const auto expectedExcessBlobCount = compactionThreshold - 2;
             UNIT_ASSERT_VALUES_EQUAL(
-                expectedBlobCount,
+                expectedBlobCount + expectedExcessBlobCount,
                 stats.GetMixedBlobsCount());
             UNIT_ASSERT_VALUES_EQUAL(1, stats.GetUsedCompactionRanges());
             UNIT_ASSERT_VALUES_EQUAL(
@@ -467,9 +468,10 @@ Y_UNIT_TEST_SUITE(TIndexTabletTest_Data_Stress)
             UNIT_ASSERT_VALUES_EQUAL(1, stats.CompactionRangeStatsSize());
             UNIT_ASSERT_VALUES_EQUAL(
                 Sprintf(
-                    "r=1177944064 b=%u d=%u g=0",
+                    "r=1177944064 b=%u d=%u g=%u",
                     (compactionThreshold - 1),
-                    deletionMarkers),
+                    deletionMarkers,
+                    expectedExcessBlobCount * BlockGroupSize),
                 CompactionRangeToString(stats.GetCompactionRangeStats(0)));
         }
     }