Add config param allow_remove_stale_moving_parts

2024-11-25 09:02:00 +00:00 · 2023-06-08 10:17:02 +00:00 · 2023-06-08 10:17:02 +00:00 · 44c68ffdab
commit 44c68ffdab
parent 989540e5b1
16 changed files with 34 additions and 5 deletions
--- a/src/Storages/MergeTree/MergeTreeData.cpp
+++ b/src/Storages/MergeTree/MergeTreeData.cpp
@ -19,6 +19,7 @@
 #include <Common/CurrentMetrics.h>
 #include <Common/ThreadFuzzer.h>
 #include <Common/getNumberOfPhysicalCPUCores.h>
+#include <Common/Config/ConfigHelper.h>
 #include <Compression/CompressedReadBuffer.h>
 #include <Core/QueryProcessingStage.h>
 #include <DataTypes/DataTypeEnum.h>
@ -1998,14 +1999,19 @@ static bool isOldPartDirectory(const DiskPtr & disk, const String & directory_pa
    return true;
 }

+
 size_t MergeTreeData::clearOldTemporaryDirectories(size_t custom_directories_lifetime_seconds, const NameSet & valid_prefixes)
 {
    size_t cleared_count = 0;

    cleared_count += clearOldTemporaryDirectories(relative_data_path, custom_directories_lifetime_seconds, valid_prefixes);

+    if (allowRemoveStaleMovingParts())
+    {
        /// Clear _all_ parts from the `moving` directory
        cleared_count += clearOldTemporaryDirectories(fs::path(relative_data_path) / "moving", custom_directories_lifetime_seconds, {""});
+    }
+
    return cleared_count;
 }

@ -8412,6 +8418,11 @@ MergeTreeData::MutableDataPartPtr MergeTreeData::createEmptyPart(
    return new_data_part;
 }

+bool MergeTreeData::allowRemoveStaleMovingParts() const
+{
+    return ConfigHelper::getBool(getContext()->getConfigRef(), "allow_remove_stale_moving_parts");
+}
+
 CurrentlySubmergingEmergingTagger::~CurrentlySubmergingEmergingTagger()
 {
    std::lock_guard lock(storage.currently_submerging_emerging_mutex);
--- a/src/Storages/MergeTree/MergeTreeData.h
+++ b/src/Storages/MergeTree/MergeTreeData.h
@ -1060,6 +1060,9 @@ public:
    void waitForOutdatedPartsToBeLoaded() const;
    bool canUsePolymorphicParts() const;

+    /// TODO: make enabled by default in the next release if no problems found.
+    bool allowRemoveStaleMovingParts() const;
+
 protected:
    friend class IMergeTreeDataPart;
    friend class MergeTreeDataMergerMutator;
--- a/src/Storages/MergeTree/MergeTreePartsMover.cpp
+++ b/src/Storages/MergeTree/MergeTreePartsMover.cpp
@ -250,7 +250,7 @@ MergeTreePartsMover::TemporaryClonedPart MergeTreePartsMover::clonePart(const Me
    cloned_part.part = std::move(builder).withPartFormatFromDisk().build();
    LOG_TRACE(log, "Part {} was cloned to {}", part->name, cloned_part.part->getDataPartStorage().getFullPath());

-    cloned_part.part->is_temp = true;
+    cloned_part.part->is_temp = data->allowRemoveStaleMovingParts();
    cloned_part.part->loadColumnsChecksumsIndexes(true, true);
    cloned_part.part->loadVersionMetadata();
    cloned_part.part->modification_time = cloned_part.part->getDataPartStorage().getLastModified().epochTime();
@ -270,10 +270,11 @@ void MergeTreePartsMover::swapClonedPart(TemporaryClonedPart & cloned_part) cons
    {
        LOG_INFO(log,
            "Failed to swap {}. Active part doesn't exist (containing part {}). "
-            "Possible it was merged or mutated. Will remove copy on path '{}'",
+            "Possible it was merged or mutated. Part on path '{}' {}",
            cloned_part.part->name,
            active_part ? active_part->name : "doesn't exist",
-            cloned_part.part->getDataPartStorage().getFullPath());
+            cloned_part.part->getDataPartStorage().getFullPath(),
+            data->allowRemoveStaleMovingParts() ? "will be removed" : "will remain intact (set <allow_remove_stale_moving_parts> in config.xml, exercise caution when using)");
        return;
    }

--- a/tests/integration/test_alter_moving_garbage/configs/config.d/storage_conf.xml
+++ b/tests/integration/test_alter_moving_garbage/configs/config.d/storage_conf.xml
@ -21,4 +21,6 @@
            </two_disks>
        </policies>
    </storage_configuration>
+
+    <allow_remove_stale_moving_parts>true</allow_remove_stale_moving_parts>
 </clickhouse>
--- a/tests/integration/test_consistant_parts_after_move_partition/configs/remote_servers.xml
+++ b/tests/integration/test_consistant_parts_after_move_partition/configs/remote_servers.xml
@ -16,4 +16,5 @@
            </shard>
        </test_cluster>
    </remote_servers>
+    <allow_remove_stale_moving_parts>true</allow_remove_stale_moving_parts>
 </clickhouse>
--- a/tests/integration/test_encrypted_disk/configs/storage.xml
+++ b/tests/integration/test_encrypted_disk/configs/storage.xml
@ -105,4 +105,5 @@
            </s3_encrypted_cache_policy>
         </policies>
    </storage_configuration>
+    <allow_remove_stale_moving_parts>true</allow_remove_stale_moving_parts>
 </clickhouse>
--- a/tests/integration/test_merge_tree_azure_blob_storage/configs/config.xml
+++ b/tests/integration/test_merge_tree_azure_blob_storage/configs/config.xml
@ -15,4 +15,5 @@
    <max_concurrent_queries>500</max_concurrent_queries>
    <path>./clickhouse/</path>
    <users_config>users.xml</users_config>
+    <allow_remove_stale_moving_parts>true</allow_remove_stale_moving_parts>
 </clickhouse>
--- a/tests/integration/test_merge_tree_hdfs/configs/config.d/storage_conf.xml
+++ b/tests/integration/test_merge_tree_hdfs/configs/config.d/storage_conf.xml
@ -29,4 +29,5 @@
    <merge_tree>
        <min_bytes_for_wide_part>0</min_bytes_for_wide_part>
    </merge_tree>
+    <allow_remove_stale_moving_parts>true</allow_remove_stale_moving_parts>
 </clickhouse>
--- a/tests/integration/test_merge_tree_s3/configs/config.xml
+++ b/tests/integration/test_merge_tree_s3/configs/config.xml
@ -8,4 +8,5 @@
    </s3>

    <enable_system_unfreeze>true</enable_system_unfreeze>
+    <allow_remove_stale_moving_parts>true</allow_remove_stale_moving_parts>
 </clickhouse>
--- a/tests/integration/test_move_partition_to_disk_on_cluster/configs/config.d/storage_configuration.xml
+++ b/tests/integration/test_move_partition_to_disk_on_cluster/configs/config.d/storage_configuration.xml
@ -24,5 +24,6 @@
    </policies>

 </storage_configuration>
+<allow_remove_stale_moving_parts>true</allow_remove_stale_moving_parts>

 </clickhouse>
--- a/tests/integration/test_multiple_disks/configs/config.d/storage_configuration.xml
+++ b/tests/integration/test_multiple_disks/configs/config.d/storage_configuration.xml
@ -122,6 +122,7 @@
    </policies>

 </storage_configuration>
+<allow_remove_stale_moving_parts>true</allow_remove_stale_moving_parts>

 <merge_tree>
    <temporary_directories_lifetime>1</temporary_directories_lifetime>
--- a/tests/integration/test_rename_column/configs/config.d/storage_configuration.xml
+++ b/tests/integration/test_rename_column/configs/config.d/storage_configuration.xml
@ -29,5 +29,6 @@
    <min_bytes_for_wide_part>0</min_bytes_for_wide_part>
    <temporary_directories_lifetime>1</temporary_directories_lifetime>
 </merge_tree>
+<allow_remove_stale_moving_parts>true</allow_remove_stale_moving_parts>

 </clickhouse>
--- a/tests/integration/test_replicated_merge_tree_hdfs_zero_copy/configs/config.d/storage_conf.xml
+++ b/tests/integration/test_replicated_merge_tree_hdfs_zero_copy/configs/config.d/storage_conf.xml
@ -89,4 +89,5 @@
        <cluster>test_cluster</cluster>
        <shard>1</shard>
    </macros>
+    <allow_remove_stale_moving_parts>true</allow_remove_stale_moving_parts>
 </clickhouse>
--- a/tests/integration/test_s3_zero_copy_replication/configs/config.d/s3.xml
+++ b/tests/integration/test_s3_zero_copy_replication/configs/config.d/s3.xml
@ -93,4 +93,5 @@
        <cluster>test_cluster</cluster>
    </macros>

+    <allow_remove_stale_moving_parts>true</allow_remove_stale_moving_parts>
 </clickhouse>
--- a/tests/integration/test_ttl_move/configs/config.d/storage_configuration.xml
+++ b/tests/integration/test_ttl_move/configs/config.d/storage_configuration.xml
@ -107,4 +107,5 @@

 </storage_configuration>

+<allow_remove_stale_moving_parts>true</allow_remove_stale_moving_parts>
 </clickhouse>
--- a/tests/integration/test_zero_copy_fetch/configs/storage_conf.xml
+++ b/tests/integration/test_zero_copy_fetch/configs/storage_conf.xml
@ -38,4 +38,5 @@
    <merge_tree>
        <allow_remote_fs_zero_copy_replication>true</allow_remote_fs_zero_copy_replication>
    </merge_tree>
+    <allow_remove_stale_moving_parts>true</allow_remove_stale_moving_parts>
 </clickhouse>