From b601d6b64d9b77f92a75c95ae994e7052c8bc1bf Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=D0=9A=D0=B8=D1=80=D0=B8=D0=BB=D0=BB=20=D0=93=D0=B0=D1=80?=
 =?UTF-8?q?=D0=B1=D0=B0=D1=80?= <st087492@student.spbu.ru>
Date: Thu, 1 Aug 2024 23:57:55 +0300
Subject: [PATCH] Move all logic to operation

---
 .../ObjectStorages/DiskObjectStorage.cpp      | 11 +--
 .../DiskObjectStorageTransaction.cpp          | 75 ++++++++++---------
 2 files changed, 43 insertions(+), 43 deletions(-)

diff --git a/src/Disks/ObjectStorages/DiskObjectStorage.cpp b/src/Disks/ObjectStorages/DiskObjectStorage.cpp
index af2f9bf4258..a48bbbaf69c 100644
--- a/src/Disks/ObjectStorages/DiskObjectStorage.cpp
+++ b/src/Disks/ObjectStorages/DiskObjectStorage.cpp
@@ -376,14 +376,9 @@ void DiskObjectStorage::removeSharedFileIfExists(const String & path, bool delet
 void DiskObjectStorage::removeSharedRecursive(
     const String & path, bool keep_all_batch_data, const NameSet & file_names_remove_metadata_only)
 {
-    /// At most remove_shared_recursive_file_limit files are removed in one transaction
-    /// Retry until all of them are removed
-    while (exists(path))
-    {
-        auto transaction = createObjectStorageTransaction();
-        transaction->removeSharedRecursive(path, keep_all_batch_data, file_names_remove_metadata_only);
-        transaction->commit();
-    }
+    auto transaction = createObjectStorageTransaction();
+    transaction->removeSharedRecursive(path, keep_all_batch_data, file_names_remove_metadata_only);
+    transaction->commit();
 }
 
 bool DiskObjectStorage::tryReserve(UInt64 bytes)
diff --git a/src/Disks/ObjectStorages/DiskObjectStorageTransaction.cpp b/src/Disks/ObjectStorages/DiskObjectStorageTransaction.cpp
index 17a2a4ebdd1..875fb380b93 100644
--- a/src/Disks/ObjectStorages/DiskObjectStorageTransaction.cpp
+++ b/src/Disks/ObjectStorages/DiskObjectStorageTransaction.cpp
@@ -326,11 +326,46 @@ struct RemoveRecursiveObjectStorageOperation final : public IDiskObjectStorageOp
         return limit > 0 && objects_to_remove_by_path.size() == limit;
     }
 
+    void removeObjects()
+    {
+        if (!keep_all_batch_data)
+        {
+            std::vector<String> total_removed_paths;
+            total_removed_paths.reserve(objects_to_remove_by_path.size());
+
+            StoredObjects remove_from_remote;
+            for (auto && [local_path, objects_to_remove] : objects_to_remove_by_path)
+            {
+                chassert(!file_names_remove_metadata_only.contains(local_path));
+                if (objects_to_remove.unlink_outcome->num_hardlinks == 0)
+                {
+                    std::move(objects_to_remove.objects.begin(), objects_to_remove.objects.end(), std::back_inserter(remove_from_remote));
+                    total_removed_paths.push_back(local_path);
+                }
+            }
+
+            /// Read comment inside RemoveObjectStorageOperation class
+            /// TL;DR Don't pay any attention to 404 status code
+            object_storage.removeObjectsIfExist(remove_from_remote);
+
+            objects_to_remove_by_path.clear();
+
+            LOG_DEBUG(
+                getLogger("RemoveRecursiveObjectStorageOperation"),
+                "Recursively remove path {}: "
+                "metadata and objects were removed for [{}], "
+                "only metadata were removed for [{}].",
+                path,
+                boost::algorithm::join(total_removed_paths, ", "),
+                boost::algorithm::join(file_names_remove_metadata_only, ", "));
+        }
+    }
+
     void removeMetadataRecursive(MetadataTransactionPtr tx, const std::string & path_to_remove)
     {
         checkStackSize(); /// This is needed to prevent stack overflow in case of cyclic symlinks.
         if (checkLimitReached())
-            return;
+            removeObjects();
 
         if (metadata_storage.isFile(path_to_remove))
         {
@@ -373,13 +408,11 @@ struct RemoveRecursiveObjectStorageOperation final : public IDiskObjectStorageOp
         {
             for (auto it = metadata_storage.iterateDirectory(path_to_remove); it->isValid(); it->next())
             {
-                if (checkLimitReached())
-                    return;
                 removeMetadataRecursive(tx, it->path());
+                if (checkLimitReached())
+                    removeObjects();
             }
-            /// Do not delete in case directory contains >= limit files
-            if (objects_to_remove_by_path.size() < limit)
-                tx->removeDirectory(path_to_remove);
+            tx->removeDirectory(path_to_remove);
         }
     }
 
@@ -396,35 +429,7 @@ struct RemoveRecursiveObjectStorageOperation final : public IDiskObjectStorageOp
 
     void finalize() override
     {
-        if (!keep_all_batch_data)
-        {
-            std::vector<String> total_removed_paths;
-            total_removed_paths.reserve(objects_to_remove_by_path.size());
-
-            StoredObjects remove_from_remote;
-            for (auto && [local_path, objects_to_remove] : objects_to_remove_by_path)
-            {
-                chassert(!file_names_remove_metadata_only.contains(local_path));
-                if (objects_to_remove.unlink_outcome->num_hardlinks == 0)
-                {
-                    std::move(objects_to_remove.objects.begin(), objects_to_remove.objects.end(), std::back_inserter(remove_from_remote));
-                    total_removed_paths.push_back(local_path);
-                }
-            }
-
-            /// Read comment inside RemoveObjectStorageOperation class
-            /// TL;DR Don't pay any attention to 404 status code
-            object_storage.removeObjectsIfExist(remove_from_remote);
-
-            LOG_DEBUG(
-                getLogger("RemoveRecursiveObjectStorageOperation"),
-                "Recursively remove path {}: "
-                "metadata and objects were removed for [{}], "
-                "only metadata were removed for [{}].",
-                path,
-                boost::algorithm::join(total_removed_paths, ", "),
-                boost::algorithm::join(file_names_remove_metadata_only, ", "));
-        }
+        removeObjects();
     }
 };