issue-1157: subtract blocks waiting for cleanup from disk garbage blo…

…cks counters (#1170) * issue-1157: subtract blocks waiting for cleanup from disk garbage blocks counters * fix ut * update * update * update
ydb-platform · May 14, 2024 · ae7b9b0 · ae7b9b0
1 parent 04c98a4
commit ae7b9b0
Show file tree

Hide file tree

Showing 7 changed files with 180 additions and 5 deletions.
diff --git a/cloud/blockstore/libs/storage/partition/model/cleanup_queue.cpp b/cloud/blockstore/libs/storage/partition/model/cleanup_queue.cpp
@@ -69,8 +69,9 @@ struct TCleanupQueue::TImpl
 
 ////////////////////////////////////////////////////////////////////////////////
 
-TCleanupQueue::TCleanupQueue()
+TCleanupQueue::TCleanupQueue(ui64 blockSize)
     : Impl(new TImpl())
+    , BlockSize(blockSize)
 {}
 
 TCleanupQueue::~TCleanupQueue()
@@ -81,6 +82,7 @@ bool TCleanupQueue::Add(const TCleanupQueueItem& item)
     bool result = Impl->Add(item);
     if (result) {
         QueueBytes += item.BlobId.BlobSize();
+        QueueBlocks += item.BlobId.BlobSize() / BlockSize;
     }
     return result;
 }
@@ -93,6 +95,7 @@ bool TCleanupQueue::Add(const TVector<TCleanupQueueItem>& items)
             return false;
         }
         QueueBytes += item.BlobId.BlobSize();
+        QueueBlocks += item.BlobId.BlobSize() / BlockSize;
     }
     return true;
 }
@@ -102,6 +105,7 @@ bool TCleanupQueue::Remove(const TCleanupQueueItem& item)
     bool result = Impl->Remove(item);
     if (result) {
         QueueBytes -= item.BlobId.BlobSize();
+        QueueBlocks -= item.BlobId.BlobSize() / BlockSize;
     }
     return result;
 }
@@ -121,4 +125,9 @@ ui64 TCleanupQueue::GetQueueBytes() const
     return QueueBytes;
 }
 
+ui64 TCleanupQueue::GetQueueBlocks() const
+{
+    return QueueBlocks;
+}
+
 }   // namespace NCloud::NBlockStore::NStorage::NPartition
diff --git a/cloud/blockstore/libs/storage/partition/model/cleanup_queue.h b/cloud/blockstore/libs/storage/partition/model/cleanup_queue.h
@@ -35,10 +35,13 @@ class TCleanupQueue
     struct TImpl;
     std::unique_ptr<TImpl> Impl;
 
+    const ui64 BlockSize;
+
     ui64 QueueBytes = 0;
+    ui64 QueueBlocks = 0;
 
 public:
-    TCleanupQueue();
+    explicit TCleanupQueue(ui64 blockSize);
     ~TCleanupQueue();
 
     //
@@ -57,6 +60,7 @@ class TCleanupQueue
         size_t limit = 100) const;
 
     ui64 GetQueueBytes() const;
+    ui64 GetQueueBlocks() const;
 };
 
 }   // namespace NCloud::NBlockStore::NStorage::NPartition
diff --git a/cloud/blockstore/libs/storage/partition/model/cleanup_queue_ut.cpp b/cloud/blockstore/libs/storage/partition/model/cleanup_queue_ut.cpp
@@ -28,7 +28,7 @@ Y_UNIT_TEST_SUITE(TCleanupQueueTest)
 
     Y_UNIT_TEST(ShouldKeepItemsSorted)
     {
-        TCleanupQueue queue;
+        TCleanupQueue queue(1024);
 
         ui32 deletionStep = 10;
         for (ui32 step: Steps) {
@@ -49,7 +49,7 @@ Y_UNIT_TEST_SUITE(TCleanupQueueTest)
 
     Y_UNIT_TEST(ShouldTrimQueue)
     {
-        TCleanupQueue queue;
+        TCleanupQueue queue(1024);
 
         ui32 deletionStep = 10;
         for (ui32 step: Steps) {

diff --git a/cloud/blockstore/libs/storage/partition/part_actor_compaction.cpp b/cloud/blockstore/libs/storage/partition/part_actor_compaction.cpp
@@ -1048,7 +1048,7 @@ void TPartitionActor::EnqueueCompactionIfNeeded(const TActorContext& ctx)
     }
 
     const auto blockCount = State->GetMixedBlocksCount()
-        + State->GetMergedBlocksCount();
+        + State->GetMergedBlocksCount() - State->GetCleanupQueue().GetQueueBlocks();
     const auto diskGarbage =
         GetPercentage(State->GetUsedBlocksCount(), blockCount);
 

diff --git a/cloud/blockstore/libs/storage/partition/part_state.cpp b/cloud/blockstore/libs/storage/partition/part_state.cpp
@@ -143,6 +143,7 @@ TPartitionState::TPartitionState(
         / allocationUnit, 1ul) * maxBlobsPerUnit)
     , MaxBlobsPerRange(maxBlobsPerRange)
     , CompactionRangeCountPerRun(compactionRangeCountPerRun)
+    , CleanupQueue(GetBlockSize())
     , CleanupScoreHistory(cleanupScoreHistorySize)
     , Stats(*Meta.MutableStats())
 {

diff --git a/cloud/blockstore/libs/storage/partition/part_state_ut.cpp b/cloud/blockstore/libs/storage/partition/part_state_ut.cpp
@@ -1254,6 +1254,48 @@ Y_UNIT_TEST_SUITE(TPartitionStateTest)
         state.RegisterSuccess(now, group2);
         UNIT_ASSERT_VALUES_EQUAL(0, dts.size());
     }
+
+    Y_UNIT_TEST(ShouldTrackCleanupQueueBlockCount)
+    {
+        TPartitionState state(
+            DefaultConfig(1, 1000),
+            0,  // generation
+            BuildDefaultCompactionPolicy(5),
+            0,  // compactionScoreHistorySize
+            0,  // cleanupScoreHistorySize
+            DefaultBPConfig(),
+            DefaultFreeSpaceConfig(),
+            Max(),  // maxIORequestsInFlight
+            0,  // reassignChannelsPercentageThreshold
+            0,  // lastCommitId
+            5,  // channelCount
+            0,  // mixedIndexCacheSize
+            10000,  // allocationUnit
+            100,  // maxBlobsPerUnit
+            10,  // maxBlobsPerRange,
+            1  // compactionRangeCountPerRun
+        );
+
+        TCleanupQueueItem b1 {{1, 1, 4, 4_MB, 0, 0}, 111};
+        TCleanupQueueItem b2 {{1, 2, 4, 4096, 0, 0}, 112};
+
+        state.GetCleanupQueue().Add(b2);
+        state.GetCleanupQueue().Add(b1);
+
+        UNIT_ASSERT_VALUES_EQUAL(
+            1025,
+            state.GetCleanupQueue().GetQueueBlocks());
+
+        state.GetCleanupQueue().Remove(b1);
+        UNIT_ASSERT_VALUES_EQUAL(
+            1,
+            state.GetCleanupQueue().GetQueueBlocks());
+
+        state.GetCleanupQueue().Remove(b2);
+        UNIT_ASSERT_VALUES_EQUAL(
+            0,
+            state.GetCleanupQueue().GetQueueBlocks());
+    }
 }
 
 }   // namespace NCloud::NBlockStore::NStorage::NPartition

diff --git a/cloud/blockstore/libs/storage/partition/part_ut.cpp b/cloud/blockstore/libs/storage/partition/part_ut.cpp
@@ -11073,6 +11073,125 @@ Y_UNIT_TEST_SUITE(TPartitionTest)
         }
 
         UNIT_ASSERT_VALUES_EQUAL(1, compactionByBlobCount);
+
+        // wait for background operations completion
+        runtime->DispatchEvents(TDispatchOptions(), TDuration::Seconds(1));
+
+        // no more compactions generated because blobs in cleanup queue are
+        // considered to be already removed
+        UNIT_ASSERT_VALUES_EQUAL(1, compactionRequestObserved);
+
+        partition.SendToPipe(
+            std::make_unique<TEvPartitionPrivate::TEvUpdateCounters>());
+        {
+            TDispatchOptions options;
+            options.FinalEvents.emplace_back(
+                TEvStatsService::EvVolumePartCounters);
+            runtime->DispatchEvents(options);
+        }
+
+        UNIT_ASSERT_VALUES_EQUAL(0, compactionByBlobCount);
+    }
+
+    Y_UNIT_TEST(ShouldRunCompactionIfBlocksCountIsGreaterThanThreshold)
+    {
+        auto config = DefaultConfig();
+        config.SetHDDCompactionType(NProto::CT_LOAD);
+        config.SetV1GarbageCompactionEnabled(true);
+        config.SetCompactionGarbageThreshold(20);
+        config.SetCompactionRangeGarbageThreshold(999999999);
+        config.SetSSDMaxBlobsPerUnit(999999999);
+        config.SetHDDMaxBlobsPerUnit(999999999);
+
+        ui32 blocksCount =  10 * 1024;
+
+        auto runtime = PrepareTestActorRuntime(config, blocksCount);
+
+        TPartitionClient partition(*runtime);
+        partition.WaitReady();
+
+        ui64 compactionByBlockCount = 0;
+        bool compactionRequestObserved = false;
+        runtime->SetEventFilter([&] (auto& runtime, auto& event) {
+                Y_UNUSED(runtime);
+                switch (event->GetTypeRewrite()) {
+                    case TEvPartitionPrivate::EvCompactionRequest: {
+                        compactionRequestObserved = true;
+                        break;
+                    }
+                    case TEvPartitionPrivate::EvCleanupRequest: {
+                        return true;
+                    }
+                    case TEvStatsService::EvVolumePartCounters: {
+                        auto* msg =
+                            event->template Get<TEvStatsService::TEvVolumePartCounters>();
+                        const auto& cc = msg->DiskCounters->Cumulative;
+                        compactionByBlockCount =
+                            cc.CompactionByGarbageBlocksPerDisk.Value;
+                    }
+                }
+                return false;
+            }
+        );
+
+        for (size_t i = 0; i < 10; ++i) {
+            partition.WriteBlocks(TBlockRange32::WithLength(i * 1024, 1024), i);
+        }
+
+        partition.SendToPipe(
+            std::make_unique<TEvPartitionPrivate::TEvUpdateCounters>());
+        {
+            TDispatchOptions options;
+            options.FinalEvents.emplace_back(
+                TEvStatsService::EvVolumePartCounters);
+            runtime->DispatchEvents(options);
+        }
+        UNIT_ASSERT_EQUAL(0, compactionByBlockCount);
+
+        // wait for background operations completion
+        runtime->DispatchEvents(TDispatchOptions(), TDuration::Seconds(1));
+
+        // garbage block count is less than 20%  => no compaction
+        UNIT_ASSERT(!compactionRequestObserved);
+
+        for (size_t i = 0; i < 2; ++i) {
+            partition.WriteBlocks(TBlockRange32::WithLength(i * 1024, 1024), i);
+        }
+
+        // wait for background operations completion
+        runtime->DispatchEvents(TDispatchOptions(), TDuration::Seconds(1));
+
+        // garbage block count is greater than threshold => compaction
+        UNIT_ASSERT(compactionRequestObserved);
+
+        partition.SendToPipe(
+            std::make_unique<TEvPartitionPrivate::TEvUpdateCounters>());
+        {
+            TDispatchOptions options;
+            options.FinalEvents.emplace_back(
+                TEvStatsService::EvVolumePartCounters);
+            runtime->DispatchEvents(options);
+        }
+
+        UNIT_ASSERT_VALUES_EQUAL(1, compactionByBlockCount);
+
+        // wait for background operations completion
+        runtime->DispatchEvents(TDispatchOptions(), TDuration::Seconds(1));
+
+        // no more compactions generated because blocks in cleanup queue are
+        // considered to be already removed
+        UNIT_ASSERT_VALUES_EQUAL(1, compactionRequestObserved);
+
+        partition.SendToPipe(
+            std::make_unique<TEvPartitionPrivate::TEvUpdateCounters>());
+        {
+            TDispatchOptions options;
+            options.FinalEvents.emplace_back(
+                TEvStatsService::EvVolumePartCounters);
+            runtime->DispatchEvents(options);
+        }
+
+        UNIT_ASSERT_VALUES_EQUAL(0, compactionByBlockCount);
     }
 }