Merge pull request #5 from RunningXie/feature_handle_remove_error

handle remove error
young-scott · Jul 25, 2023 · ac8df32 · ac8df32
2 parents 8dd4984 + fb86e55
commit ac8df32
Show file tree

Hide file tree

Showing 3 changed files with 69 additions and 14 deletions.
diff --git a/src/Core/Settings.h b/src/Core/Settings.h
@@ -34,6 +34,7 @@ class IColumn;
   */
 
 #define COMMON_SETTINGS(M) \
+    M(String, handle_remove_error_path, "", "When remove_all throws the directory not empty exception, the undeleted data will be moved to the the setting directory. Empty means that the exception will not be processed.", 0) \
     M(UInt64, min_compress_block_size, 65536, "The actual size of the block to compress, if the uncompressed data less than max_compress_block_size is no less than this value and no less than the volume of data for one mark.", 0) \
     M(UInt64, max_compress_block_size, 1048576, "The maximum size of blocks of uncompressed data before compressing for writing to a table.", 0) \
     M(UInt64, max_block_size, DEFAULT_BLOCK_SIZE, "Maximum block size for reading", 0) \

diff --git a/src/Disks/DiskLocal.cpp b/src/Disks/DiskLocal.cpp
@@ -15,11 +15,11 @@
 #include <Disks/DiskFactory.h>
 #include <Disks/DiskMemory.h>
 #include <Disks/DiskRestartProxy.h>
-#include <Common/randomSeed.h>
 #include <IO/ReadHelpers.h>
 #include <IO/WriteBufferFromTemporaryFile.h>
 #include <IO/WriteHelpers.h>
 #include <base/logger_useful.h>
+#include <Common/randomSeed.h>
 
 namespace CurrentMetrics
 {
@@ -371,9 +371,61 @@ void DiskLocal::removeDirectory(const String & path)
         throwFromErrnoWithPath("Cannot rmdir " + fs_path.string(), fs_path, ErrorCodes::CANNOT_RMDIR);
 }
 
+void DiskLocal::remove_all_for_test(const String & path)
+{
+    LOG_DEBUG(log, "mock remove_all path: {}", path);
+    if (path.find("/mnt/cfs") != 0)
+    {
+        fs::remove_all(fs::path(path));
+        return;
+    }
+    for (const auto & entry : std::filesystem::directory_iterator(fs::path(path)))
+    {
+        if (entry.is_regular_file() || entry.is_directory())
+        {
+            std::error_code ec = std::make_error_code(std::errc::directory_not_empty);
+            throw std::filesystem::filesystem_error("Directory not empty", entry.path(), ec);
+        }
+    }
+    fs::remove_all(path);
+}
+
+void DiskLocal::renameFiles(const fs::path & source_dir, const fs::path & target_dir)
+{
+    if (!fs::exists(target_dir.parent_path()))
+    {
+        fs::create_directories(target_dir.parent_path());
+    }
+    LOG_DEBUG(log, "move files from {} to {}", source_dir.string(), target_dir.string());
+    fs::rename(source_dir, target_dir);
+}
+
 void DiskLocal::removeRecursive(const String & path)
 {
-    fs::remove_all(fs::path(disk_path) / path);
+    try
+    {
+        fs::remove_all(fs::path(disk_path) / path);
+    }
+    catch (const fs::filesystem_error & e)
+    {
+        if (e.code() == std::errc::directory_not_empty && handle_remove_error_path != "" && disk_path.find("/mnt/cfs") == 0)
+        {
+            LOG_DEBUG(
+                log,
+                "remove_all get directory not empty error, try to move, error: {}, disk path: {}, remove path: {}",
+                e.what(),
+                disk_path,
+                (fs::path(disk_path) / path).string());
+            fs::path to_path = fs::path(handle_remove_error_path) / fs::relative(e.path1(), disk_path);
+            renameFiles(e.path1(), to_path);
+            if (fs::exists(fs::path(disk_path) / path))
+            {
+                removeRecursive(path);
+            }
+        }
+        else
+            throw;
+    }
 }
 
 void DiskLocal::listFiles(const String & path, std::vector<String> & file_names)
@@ -417,9 +469,8 @@ void DiskLocal::createFile(const String & path)
 
 void DiskLocal::setReadOnly(const String & path)
 {
-    fs::permissions(fs::path(disk_path) / path,
-                    fs::perms::owner_write | fs::perms::group_write | fs::perms::others_write,
-                    fs::perm_options::remove);
+    fs::permissions(
+        fs::path(disk_path) / path, fs::perms::owner_write | fs::perms::group_write | fs::perms::others_write, fs::perm_options::remove);
 }
 
 bool inline isSameDiskType(const IDisk & one, const IDisk & another)
@@ -465,10 +516,7 @@ void DiskLocal::applyNewSettings(const Poco::Util::AbstractConfiguration & confi
 }
 
 DiskLocal::DiskLocal(const String & name_, const String & path_, UInt64 keep_free_space_bytes_)
-    : name(name_)
-    , disk_path(path_)
-    , keep_free_space_bytes(keep_free_space_bytes_)
-    , logger(&Poco::Logger::get("DiskLocal"))
+    : name(name_), disk_path(path_), keep_free_space_bytes(keep_free_space_bytes_), logger(&Poco::Logger::get("DiskLocal"))
 {
 }
 
@@ -478,6 +526,8 @@ DiskLocal::DiskLocal(
 {
     if (local_disk_check_period_ms > 0)
         disk_checker = std::make_unique<DiskLocalCheckThread>(this, context, local_disk_check_period_ms);
+    const auto & settings = context->getSettingsRef();
+    handle_remove_error_path = String(settings.handle_remove_error_path);
 }
 
 void DiskLocal::startup()
@@ -668,6 +718,11 @@ void registerDiskLocal(DiskFactory & factory)
 
         std::shared_ptr<IDisk> disk
             = std::make_shared<DiskLocal>(name, path, keep_free_space_bytes, context, config.getUInt("local_disk_check_period_ms", 0));
+        LOG_DEBUG(
+            &Poco::Logger::get("DiskLocal"),
+            "register local disk, name: {}, handle remove error path: {}",
+            name,
+            String(context->getSettingsRef().handle_remove_error_path));
         disk->startup();
         return std::make_shared<DiskRestartProxy>(disk);
     };

diff --git a/src/Disks/DiskLocal.h b/src/Disks/DiskLocal.h
@@ -22,11 +22,7 @@ class DiskLocal : public IDisk
 
     DiskLocal(const String & name_, const String & path_, UInt64 keep_free_space_bytes_);
     DiskLocal(
-        const String & name_,
-        const String & path_,
-        UInt64 keep_free_space_bytes_,
-        ContextPtr context,
-        UInt64 local_disk_check_period_ms);
+        const String & name_, const String & path_, UInt64 keep_free_space_bytes_, ContextPtr context, UInt64 local_disk_check_period_ms);
 
     const String & getName() const override { return name; }
 
@@ -128,10 +124,13 @@ class DiskLocal : public IDisk
 
     /// Read magic number from disk checker file. Return std::nullopt if exception happens.
     std::optional<UInt32> readDiskCheckerMagicNumber() const noexcept;
+    void remove_all_for_test(const String & path);
+    void renameFiles(const fs::path & source_dir, const fs::path & target_dir);
 
     const String name;
     const String disk_path;
     const String disk_checker_path = ".disk_checker_file";
+    String handle_remove_error_path;
     std::atomic<UInt64> keep_free_space_bytes;
     Poco::Logger * logger;