issue-95: Compaction should rebase blocks in order not to write garba…

…ge blocks to dst blobs (#729)
ydb-platform · Mar 14, 2024 · 877ce2c · 877ce2c
1 parent f958de8
commit 877ce2c
Show file tree

Hide file tree

Showing 5 changed files with 43 additions and 44 deletions.
diff --git a/cloud/filestore/libs/storage/tablet/rebase_logic.cpp b/cloud/filestore/libs/storage/tablet/rebase_logic.cpp
@@ -4,11 +4,11 @@ namespace NCloud::NFileStore::NStorage {
 
 ////////////////////////////////////////////////////////////////////////////////
 
-TRebaseResult RebaseMixedBlocks(
+TRebaseResult RebaseBlocks(
     TVector<TBlock>& blocks,
     ui64 lastCommitId,
-    TFindCheckpoint findCheckpoint,
-    TFindBlock findBlock)
+    const TFindCheckpoint& findCheckpoint,
+    const TFindBlock& findBlock)
 {
     TRebaseResult result;
 

diff --git a/cloud/filestore/libs/storage/tablet/rebase_logic.h b/cloud/filestore/libs/storage/tablet/rebase_logic.h
@@ -29,10 +29,10 @@ using TFindBlock = std::function<bool(ui64 nodeId, ui32 blockIndex)>;
 
 ////////////////////////////////////////////////////////////////////////////////
 
-TRebaseResult RebaseMixedBlocks(
+TRebaseResult RebaseBlocks(
     TVector<TBlock>& blocks,
     ui64 lastCommitId,
-    TFindCheckpoint findCheckpoint,
-    TFindBlock findBlock);
+    const TFindCheckpoint& findCheckpoint,
+    const TFindBlock& findBlock);
 
 }   // namespace NCloud::NFileStore::NStorage
diff --git a/cloud/filestore/libs/storage/tablet/tablet_state.h b/cloud/filestore/libs/storage/tablet/tablet_state.h
@@ -7,6 +7,7 @@
 
 #include "checkpoint.h"
 #include "helpers.h"
+#include "rebase_logic.h"
 #include "session.h"
 
 #include <cloud/filestore/libs/storage/model/channel_data_kind.h>
@@ -741,6 +742,8 @@ FILESTORE_DUPCACHE_REQUESTS(FILESTORE_DECLARE_DUPCACHE)
         const TPartialBlobId& blobId,
         const TVector<TBlock>& blocks);
 
+    TRebaseResult RebaseMixedBlocks(TVector<TBlock>& blocks) const;
+
     //
     // Garbage
     //

diff --git a/cloud/filestore/libs/storage/tablet/tablet_state_data.cpp b/cloud/filestore/libs/storage/tablet/tablet_state_data.cpp
@@ -1,7 +1,6 @@
 #include "tablet_state_impl.h"
 
 #include "profile_log_events.h"
-#include "rebase_logic.h"
 
 #include <cloud/filestore/libs/storage/model/utils.h>
 #include <cloud/filestore/libs/storage/tablet/model/block.h>
@@ -580,21 +579,7 @@ bool TIndexTabletState::WriteMixedBlocks(
         Impl->MixedBlocks.ApplyDeletionMarkers(GetRangeIdHasher(), blocks);
     }
 
-    auto rebaseResult = RebaseMixedBlocks(
-        blocks,
-        GetCurrentCommitId(),
-        [=] (ui64 nodeId, ui64 commitId) {
-            return Impl->Checkpoints.FindCheckpoint(nodeId, commitId);
-        },
-        [=] (ui64 nodeId, ui32 blockIndex) {
-            return IntersectsWithFresh(
-                Impl->FreshBytes,
-                Impl->FreshBlocks,
-                GetBlockSize(),
-                nodeId,
-                blockIndex
-            );
-        });
+    auto rebaseResult = RebaseMixedBlocks(blocks);
 
     if (!rebaseResult.LiveBlocks) {
         AddGarbageBlob(db, blobId);
@@ -676,9 +661,32 @@ void TIndexTabletState::DeleteMixedBlocks(
     DecrementMixedBlocksCount(db, blocks.size());
 }
 
+TRebaseResult TIndexTabletState::RebaseMixedBlocks(TVector<TBlock>& blocks) const
+{
+    return RebaseBlocks(
+        blocks,
+        GetCurrentCommitId(),
+        [=] (ui64 nodeId, ui64 commitId) {
+            return Impl->Checkpoints.FindCheckpoint(nodeId, commitId);
+        },
+        [=] (ui64 nodeId, ui32 blockIndex) {
+            return IntersectsWithFresh(
+                Impl->FreshBytes,
+                Impl->FreshBlocks,
+                GetBlockSize(),
+                nodeId,
+                blockIndex
+            );
+        });
+}
+
 TVector<TMixedBlobMeta> TIndexTabletState::GetBlobsForCompaction(ui32 rangeId) const
 {
-    return Impl->MixedBlocks.GetBlobsForCompaction(rangeId);
+    auto blobs = Impl->MixedBlocks.GetBlobsForCompaction(rangeId);
+    for (auto& blob: blobs) {
+        RebaseMixedBlocks(blob.Blocks);
+    }
+    return blobs;
 }
 
 TMixedBlobMeta TIndexTabletState::FindBlob(ui32 rangeId, TPartialBlobId blobId) const
@@ -808,21 +816,7 @@ void TIndexTabletState::RewriteMixedBlocks(
 
     Impl->MixedBlocks.ApplyDeletionMarkers(GetRangeIdHasher(), blob.Blocks);
 
-    auto rebaseResult = RebaseMixedBlocks(
-        blob.Blocks,
-        GetCurrentCommitId(),
-        [=] (ui64 nodeId, ui64 commitId) {
-            return Impl->Checkpoints.FindCheckpoint(nodeId, commitId);
-        },
-        [=] (ui64 nodeId, ui32 blockIndex) {
-            return IntersectsWithFresh(
-                Impl->FreshBytes,
-                Impl->FreshBlocks,
-                GetBlockSize(),
-                nodeId,
-                blockIndex
-            );
-        });
+    auto rebaseResult = RebaseMixedBlocks(blob.Blocks);
 
     if (!rebaseResult.LiveBlocks) {
         DeleteMixedBlocks(db, blob.BlobId, blob.Blocks);

diff --git a/cloud/filestore/libs/storage/tablet/tablet_ut_data.cpp b/cloud/filestore/libs/storage/tablet/tablet_ut_data.cpp
@@ -989,7 +989,9 @@ Y_UNIT_TEST_SUITE(TIndexTabletTest_Data)
             const auto& stats = response->Record.GetStats();
             UNIT_ASSERT_VALUES_EQUAL(stats.GetMixedBlocksCount(), 0);
             UNIT_ASSERT_VALUES_EQUAL(stats.GetMixedBlobsCount(), 0);
-            UNIT_ASSERT_VALUES_EQUAL(stats.GetGarbageQueueSize(), 2 * block);
+            // still 1 block - Compaction doesn't move unneeded blocks to its
+            // dst blobs
+            UNIT_ASSERT_VALUES_EQUAL(stats.GetGarbageQueueSize(), block);
         }
 
         id = CreateNode(tablet, TCreateNodeArgs::File(RootNodeId, "test"));
@@ -1010,7 +1012,7 @@ Y_UNIT_TEST_SUITE(TIndexTabletTest_Data)
             // GarbageQueueSize remains the same since FlushBytes doesn't
             // generate any new blobs after UnlinkNode (leads to Truncate to
             // zero size)
-            UNIT_ASSERT_VALUES_EQUAL(stats.GetGarbageQueueSize(), 2 * block);
+            UNIT_ASSERT_VALUES_EQUAL(stats.GetGarbageQueueSize(), block);
         }
 
         tablet.CollectGarbage();
@@ -1072,20 +1074,20 @@ Y_UNIT_TEST_SUITE(TIndexTabletTest_Data)
         {
             auto response = tablet.GetStorageStats();
             const auto& stats = response->Record.GetStats();
-            UNIT_ASSERT_VALUES_EQUAL(stats.GetMixedBlocksCount(), 2);
+            UNIT_ASSERT_VALUES_EQUAL(stats.GetMixedBlocksCount(), 1);
             UNIT_ASSERT_VALUES_EQUAL(stats.GetMixedBlobsCount(), 1);
             UNIT_ASSERT_VALUES_EQUAL(
                 stats.GetGarbageQueueSize(),
-                2 * block + 2 * block
-            );  // new: 1 (x 2 blocks), garbage: 2 (x block)
+                1 * block + 2 * block
+            );  // new: 1 (x block), garbage: 2 (x block)
         }
 
         tablet.CollectGarbage();
 
         {
             auto response = tablet.GetStorageStats();
             const auto& stats = response->Record.GetStats();
-            UNIT_ASSERT_VALUES_EQUAL(stats.GetMixedBlocksCount(), 2);
+            UNIT_ASSERT_VALUES_EQUAL(stats.GetMixedBlocksCount(), 1);
             UNIT_ASSERT_VALUES_EQUAL(stats.GetMixedBlobsCount(), 1);
             UNIT_ASSERT_VALUES_EQUAL(stats.GetGarbageQueueSize(), 0);   // new: 0, garbage: 0
         }