Remove the removal of broken detached parts

src/Storages/MergeTree/MergeTreeData.cpp

@@ -2628,97 +2628,6 @@ void MergeTreeData::clearPartsFromFilesystemImpl(const DataPartsVector & parts_t
                         "({} != {} + {}), it's a bug", parts_to_remove.size(), sum_of_ranges, excluded_parts.size());
 }
 
-size_t MergeTreeData::clearOldBrokenPartsFromDetachedDirectory()
-{
-    /**
-     * Remove old (configured by setting) broken detached parts.
-     * Only parts with certain prefixes are removed. These prefixes
-     * are such that it is guaranteed that they will never be needed
-     * and need to be cleared. ctime is used to check when file was
-     * moved to detached/ directory (see https://unix.stackexchange.com/a/211134)
-     */
-
-    DetachedPartsInfo detached_parts = getDetachedParts();
-    if (detached_parts.empty())
-        return 0;
-
-    auto get_last_touched_time = [&](const DetachedPartInfo & part_info) -> time_t
-    {
-        auto path = fs::path(relative_data_path) / "detached" / part_info.dir_name;
-        time_t last_change_time = part_info.disk->getLastChanged(path);
-        time_t last_modification_time = part_info.disk->getLastModified(path).epochTime();
-        return std::max(last_change_time, last_modification_time);
-    };
-
-    time_t ttl_seconds = getSettings()->merge_tree_clear_old_broken_detached_parts_ttl_timeout_seconds;
-
-    size_t unfinished_deleting_parts = 0;
-    time_t current_time = time(nullptr);
-    for (const auto & part_info : detached_parts)
-    {
-        if (!part_info.dir_name.starts_with("deleting_"))
-            continue;
-
-        time_t startup_time = current_time - static_cast<time_t>(Context::getGlobalContextInstance()->getUptimeSeconds());
-        time_t last_touch_time = get_last_touched_time(part_info);
-
-        /// Maybe it's being deleted right now (for example, in ALTER DROP DETACHED)
-        bool had_restart = last_touch_time < startup_time;
-        bool ttl_expired = last_touch_time + ttl_seconds <= current_time;
-        if (!had_restart && !ttl_expired)
-            continue;
-
-        /// We were trying to delete this detached part but did not finish deleting, probably because the server crashed
-        LOG_INFO(log, "Removing detached part {} that we failed to remove previously", part_info.dir_name);
-        try
-        {
-            removeDetachedPart(part_info.disk, fs::path(relative_data_path) / "detached" / part_info.dir_name / "", part_info.dir_name);
-            ++unfinished_deleting_parts;
-        }
-        catch (...)
-        {
-            tryLogCurrentException(log);
-        }
-    }
-
-    if (!getSettings()->merge_tree_enable_clear_old_broken_detached)
-        return unfinished_deleting_parts;
-
-    const auto full_path = fs::path(relative_data_path) / "detached";
-    size_t removed_count = 0;
-    for (const auto & part_info : detached_parts)
-    {
-        if (!part_info.valid_name || part_info.prefix.empty())
-            continue;
-
-        const auto & removable_detached_parts_prefixes = DetachedPartInfo::DETACHED_REASONS_REMOVABLE_BY_TIMEOUT;
-        bool can_be_removed_by_timeout = std::find(
-            removable_detached_parts_prefixes.begin(),
-            removable_detached_parts_prefixes.end(),
-            part_info.prefix) != removable_detached_parts_prefixes.end();
-
-        if (!can_be_removed_by_timeout)
-            continue;
-
-        ssize_t threshold = current_time - ttl_seconds;
-        time_t last_touch_time = get_last_touched_time(part_info);
-
-        if (last_touch_time == 0 || last_touch_time >= threshold)
-            continue;
-
-        const String & old_name = part_info.dir_name;
-        String new_name = "deleting_" + part_info.dir_name;
-        part_info.disk->moveFile(fs::path(full_path) / old_name, fs::path(full_path) / new_name);
-
-        removeDetachedPart(part_info.disk, fs::path(relative_data_path) / "detached" / new_name / "", old_name);
-        LOG_WARNING(log, "Removed broken detached part {} due to a timeout for broken detached parts", old_name);
-        ++removed_count;
-    }
-
-    LOG_INFO(log, "Cleaned up {} detached parts", removed_count);
-
-    return removed_count + unfinished_deleting_parts;
-}
 
 size_t MergeTreeData::clearOldWriteAheadLogs()
 {

src/Storages/MergeTree/MergeTreeData.h

@@ -692,8 +692,6 @@ class MergeTreeData : public IStorage, public WithMutableContext
     /// Delete WAL files containing parts, that all already stored on disk.
     size_t clearOldWriteAheadLogs();
 
-    size_t clearOldBrokenPartsFromDetachedDirectory();
-
     /// Delete all directories which names begin with "tmp"
     /// Must be called with locked lockForShare() because it's using relative_data_path.
     size_t clearOldTemporaryDirectories(size_t custom_directories_lifetime_seconds, const NameSet & valid_prefixes = {"tmp_", "tmp-fetch_"});

src/Storages/MergeTree/MergeTreeSettings.h

@@ -63,11 +63,9 @@ struct Settings;
     M(Float, merge_selecting_sleep_slowdown_factor, 1.2f, "The sleep time for merge selecting task is multiplied by this factor when there's nothing to merge and divided when a merge was assigned", 0) \
     M(UInt64, merge_tree_clear_old_temporary_directories_interval_seconds, 60, "The period of executing the clear old temporary directories operation in background.", 0) \
     M(UInt64, merge_tree_clear_old_parts_interval_seconds, 1, "The period of executing the clear old parts operation in background.", 0) \
-    M(UInt64, merge_tree_clear_old_broken_detached_parts_ttl_timeout_seconds, 1ULL * 3600 * 24 * 30, "Remove old broken detached parts in the background if they remained intouched for a specified by this setting period of time.", 0) \
     M(UInt64, min_age_to_force_merge_seconds, 0, "If all parts in a certain range are older than this value, range will be always eligible for merging. Set to 0 to disable.", 0) \
     M(Bool, min_age_to_force_merge_on_partition_only, false, "Whether min_age_to_force_merge_seconds should be applied only on the entire partition and not on subset.", false) \
     M(UInt64, number_of_free_entries_in_pool_to_execute_optimize_entire_partition, 25, "When there is less than specified number of free entries in pool, do not try to execute optimize entire partition with a merge (this merge is created when set min_age_to_force_merge_seconds > 0 and min_age_to_force_merge_on_partition_only = true). This is to leave free threads for regular merges and avoid \"Too many parts\"", 0) \
-    M(UInt64, merge_tree_enable_clear_old_broken_detached, false, "Enable clearing old broken detached parts operation in background.", 0) \
     M(Bool, remove_rolled_back_parts_immediately, 1, "Setting for an incomplete experimental feature.", 0) \
     M(CleanDeletedRows, clean_deleted_rows, CleanDeletedRows::Never, "Is the Replicated Merge cleanup has to be done automatically at each merge or manually (possible values are 'Always'/'Never' (default))", 0) \
     M(UInt64, replicated_max_mutations_in_one_entry, 10000, "Max number of mutation commands that can be merged together and executed in one MUTATE_PART entry (0 means unlimited)", 0) \

src/Storages/MergeTree/ReplicatedMergeTreeCleanupThread.cpp

@@ -155,7 +155,6 @@ Float32 ReplicatedMergeTreeCleanupThread::iterate()
         /// do it under share lock
         cleaned_other += storage.clearOldWriteAheadLogs();
         cleaned_part_like += storage.clearOldTemporaryDirectories(storage.getSettings()->temporary_directories_lifetime.totalSeconds());
-        cleaned_part_like += storage.clearOldBrokenPartsFromDetachedDirectory();
     }
 
     /// This is loose condition: no problem if we actually had lost leadership at this moment

src/Storages/StorageMergeTree.cpp

@@ -1385,7 +1385,6 @@ bool StorageMergeTree::scheduleDataProcessingJob(BackgroundJobsAssignee & assign
                 cleared_count += clearOldWriteAheadLogs();
                 cleared_count += clearOldMutations();
                 cleared_count += clearEmptyParts();
-                cleared_count += clearOldBrokenPartsFromDetachedDirectory();
                 return cleared_count;
                 /// TODO maybe take into account number of cleared objects when calculating backoff
             }, common_assignee_trigger, getStorageID()), /* need_trigger */ false);