issue-1825: deduplicate blobs in collect garbage with a linear comple…

…xity (#1826)
ydb-platform · Aug 21, 2024 · 6053800 · 6053800
1 parent 5bae10c
commit 6053800
Show file tree

Hide file tree

Showing 3 changed files with 54 additions and 6 deletions.
diff --git a/cloud/storage/core/libs/tablet/gc_logic.cpp b/cloud/storage/core/libs/tablet/gc_logic.cpp
@@ -166,13 +166,23 @@ void RemoveDuplicates(
             // we should leave it in garbageBlobs to be sure it is
             // finally collected
             if (nit->Generation() == genstep.first) {
-                git = garbageBlobs.erase(git);
-                gend = garbageBlobs.end();
+                *git = InvalidPartialBlobId;
+                ++git;
             }
-            nit = newBlobs.erase(nit);
-            nend = newBlobs.end();
+            *nit = InvalidPartialBlobId;
+            ++nit;
         }
     }
+
+    auto it =
+        std::remove(newBlobs.begin(), newBlobs.end(), InvalidPartialBlobId);
+    newBlobs.erase(it, newBlobs.end());
+
+    it = std::remove(
+        garbageBlobs.begin(),
+        garbageBlobs.end(),
+        InvalidPartialBlobId);
+    garbageBlobs.erase(it, garbageBlobs.end());
 }
 
 void FindGarbageVersions(

diff --git a/cloud/storage/core/libs/tablet/gc_logic_ut.cpp b/cloud/storage/core/libs/tablet/gc_logic_ut.cpp
@@ -356,6 +356,40 @@ Y_UNIT_TEST_SUITE(TGCLogicTest)
         UNIT_ASSERT_VALUES_EQUAL(NumberOfNewBlobs, 0);
         UNIT_ASSERT_VALUES_EQUAL(NumberOfGarbageBlobs, 0);
     }
+
+    Y_UNIT_TEST(ShouldRemoveDuplicates)
+    {
+        auto blob1 = TPartialBlobId(2, 10, 3, 4*1024*1024, 0, 0);
+        auto blob2 = TPartialBlobId(3, 10, 2, 4*1024*1024, 0, 0);
+        auto blob3 = TPartialBlobId(3, 10, 4, 4*1024*1024, 0, 0);
+        auto blob4 = TPartialBlobId(3, 11, 6, 4*1024*1024, 0, 0);
+
+        TVector<TPartialBlobId> newBlobs = {
+            // previous generation, should only be present in garbageBlobs
+            blob1,
+            // present in both newBlobs and garbageBlobs, should be excluded from both
+            blob2,
+            // present only in newBlobs, should be kept
+            blob3,
+        };
+
+        TVector<TPartialBlobId> garbageBlobs = {
+            // previous generation, should only be present in garbageBlobs
+            blob1,
+            // present in both newBlobs and garbageBlobs, should be excluded from both
+            blob2,
+            // present only in garbageBlobs, should be kept
+            blob4,
+        };
+
+        RemoveDuplicates(newBlobs, garbageBlobs, MakeCommitId(3, 0));
+        UNIT_ASSERT_VALUES_EQUAL(1, newBlobs.size());
+        UNIT_ASSERT_VALUES_EQUAL(ToString(blob3), ToString(newBlobs[0]));
+
+        UNIT_ASSERT_VALUES_EQUAL(2, garbageBlobs.size());
+        UNIT_ASSERT_VALUES_EQUAL(ToString(blob1), ToString(garbageBlobs[0]));
+        UNIT_ASSERT_VALUES_EQUAL(ToString(blob4), ToString(garbageBlobs[1]));
+    }
 }
 
 }   // namespace NCloud::NStorage
diff --git a/cloud/storage/core/libs/tablet/model/partial_blob_id.h b/cloud/storage/core/libs/tablet/model/partial_blob_id.h
@@ -1,14 +1,14 @@
 #pragma once
 
+#include "commit.h"
+
 #include <cloud/storage/core/libs/common/alloc.h>
 
 #include <util/digest/multi.h>
 #include <util/generic/hash_set.h>
 #include <util/stream/output.h>
 #include <util/system/defaults.h>
 
-#include <utility>
-
 namespace NCloud {
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -188,6 +188,10 @@ inline TPartialBlobId NextBlobId(TPartialBlobId blobId, ui64 maxUniqueId)
     return MakePartialBlobId(blobId.CommitId(), blobId.UniqueId() + 1);
 }
 
+////////////////////////////////////////////////////////////////////////////////
+
+const TPartialBlobId InvalidPartialBlobId = TPartialBlobId(InvalidCommitId, 0);
+
 }   // namespace NCloud
 
 ////////////////////////////////////////////////////////////////////////////////