diff --git a/src/Storages/MergeTree/IMergeTreeDataPart.cpp b/src/Storages/MergeTree/IMergeTreeDataPart.cpp index 41783ffddb0..effa8e85665 100644 --- a/src/Storages/MergeTree/IMergeTreeDataPart.cpp +++ b/src/Storages/MergeTree/IMergeTreeDataPart.cpp @@ -1894,7 +1894,6 @@ bool IMergeTreeDataPart::shallParticipateInMerges(const StoragePolicyPtr & stora } void IMergeTreeDataPart::renameTo(const String & new_relative_path, bool remove_new_dir_if_exists) -try { assertOnDisk(); @@ -1920,18 +1919,6 @@ try for (const auto & [_, part] : projection_parts) part->getDataPartStorage().changeRootPath(old_projection_root_path, new_projection_root_path); } -catch (...) -{ - if (startsWith(new_relative_path, fs::path(MergeTreeData::DETACHED_DIR_NAME) / "")) - { - // Don't throw when the destination is to the detached folder. It might be able to - // recover in some cases, such as fetching parts into multi-disks while some of the - // disks are broken. - tryLogCurrentException(__PRETTY_FUNCTION__); - } - else - throw; -} std::pair IMergeTreeDataPart::canRemovePart() const { @@ -2038,11 +2025,26 @@ std::optional IMergeTreeDataPart::getRelativePathForDetachedPart(const S return {}; } -void IMergeTreeDataPart::renameToDetached(const String & prefix) +void IMergeTreeDataPart::renameToDetached(const String & prefix, bool ignore_error) { auto path_to_detach = getRelativePathForDetachedPart(prefix, /* broken */ false); assert(path_to_detach); - renameTo(path_to_detach.value(), true); + try + { + renameTo(path_to_detach.value(), true); + } + catch (...) + { + if (ignore_error) + { + // Don't throw when the destination is to the detached folder. It might be able to + // recover in some cases, such as fetching parts into multi-disks while some of the + // disks are broken. + tryLogCurrentException(__PRETTY_FUNCTION__); + } + else + throw; + } part_is_probably_removed_from_disk = true; } diff --git a/src/Storages/MergeTree/IMergeTreeDataPart.h b/src/Storages/MergeTree/IMergeTreeDataPart.h index b41a1d840e1..a02b1ea80d1 100644 --- a/src/Storages/MergeTree/IMergeTreeDataPart.h +++ b/src/Storages/MergeTree/IMergeTreeDataPart.h @@ -396,7 +396,7 @@ public: auto getFilesChecksums() const { return checksums.files; } /// Moves a part to detached/ directory and adds prefix to its name - void renameToDetached(const String & prefix); + void renameToDetached(const String & prefix, bool ignore_error = false); /// Makes checks and move part to new directory /// Changes only relative_dir_name, you need to update other metadata (name, is_temp) explicitly diff --git a/src/Storages/MergeTree/MergeTreeData.cpp b/src/Storages/MergeTree/MergeTreeData.cpp index 4dd9fb7b228..9bac18c094a 100644 --- a/src/Storages/MergeTree/MergeTreeData.cpp +++ b/src/Storages/MergeTree/MergeTreeData.cpp @@ -2009,9 +2009,10 @@ void MergeTreeData::loadDataParts(bool skip_sanity_checks, std::optional(this) != nullptr; if (!is_static_storage) for (auto & part : broken_parts_to_detach) - part->renameToDetached("broken-on-start"); /// detached parts must not have '_' in prefixes + part->renameToDetached("broken-on-start", /*ignore_error=*/ replicated); /// detached parts must not have '_' in prefixes resetObjectColumnsFromActiveParts(part_lock); resetSerializationHints(part_lock); @@ -2099,6 +2100,7 @@ try ThreadPoolCallbackRunnerLocal runner(getUnexpectedPartsLoadingThreadPool().get(), "UnexpectedParts"); + bool replicated = dynamic_cast(this) != nullptr; for (auto & load_state : unexpected_data_parts) { std::lock_guard lock(unexpected_data_parts_mutex); @@ -2114,9 +2116,7 @@ try chassert(load_state.part); if (load_state.is_broken) - { - load_state.part->renameToDetached("broken-on-start"); /// detached parts must not have '_' in prefixes - } + load_state.part->renameToDetached("broken-on-start", /*ignore_error=*/ replicated); /// detached parts must not have '_' in prefixes }, Priority{}); } runner.waitForAllToFinishAndRethrowFirstError(); @@ -2167,6 +2167,7 @@ try ThreadPoolCallbackRunnerLocal runner(getOutdatedPartsLoadingThreadPool().get(), "OutdatedParts"); + bool replicated = dynamic_cast(this) != nullptr; while (true) { ThreadFuzzer::maybeInjectSleep(); @@ -2207,7 +2208,7 @@ try if (res.is_broken) { forcefullyRemoveBrokenOutdatedPartFromZooKeeperBeforeDetaching(res.part->name); - res.part->renameToDetached("broken-on-start"); /// detached parts must not have '_' in prefixes + res.part->renameToDetached("broken-on-start", /*ignore_error=*/ replicated); /// detached parts must not have '_' in prefixes } else if (res.part->is_duplicate) res.part->remove(); @@ -4530,6 +4531,7 @@ void MergeTreeData::forcefullyMovePartToDetachedAndRemoveFromMemory(const MergeT auto lock = lockParts(); bool removed_active_part = false; bool restored_active_part = false; + bool replicated = dynamic_cast(this) != nullptr; auto it_part = data_parts_by_info.find(part_to_detach->info); if (it_part == data_parts_by_info.end()) @@ -4548,7 +4550,7 @@ void MergeTreeData::forcefullyMovePartToDetachedAndRemoveFromMemory(const MergeT } modifyPartState(it_part, DataPartState::Deleting); - asMutableDeletingPart(part)->renameToDetached(prefix); + asMutableDeletingPart(part)->renameToDetached(prefix, /*ignore_error=*/ replicated); LOG_TEST(log, "forcefullyMovePartToDetachedAndRemoveFromMemory: removing {} from data_parts_indexes", part->getNameWithState()); data_parts_indexes.erase(it_part); @@ -5939,6 +5941,7 @@ MergeTreeData::MutableDataPartPtr MergeTreeData::loadPartRestoredFromBackup(cons .withPartType(MergeTreeDataPartType::Wide) .build(); } + /// Errors should not be ignored RESTORE, since you should not restore to broken disks. part->renameToDetached("broken-from-backup"); }; diff --git a/src/Storages/StorageReplicatedMergeTree.cpp b/src/Storages/StorageReplicatedMergeTree.cpp index 793fd02c656..62c05db1a52 100644 --- a/src/Storages/StorageReplicatedMergeTree.cpp +++ b/src/Storages/StorageReplicatedMergeTree.cpp @@ -1844,9 +1844,7 @@ bool StorageReplicatedMergeTree::checkPartsImpl(bool skip_sanity_checks) /// detached all unexpected data parts after sanity check. for (auto & part_state : unexpected_data_parts) - { - part_state.part->renameToDetached("ignored"); - } + part_state.part->renameToDetached("ignored", /* ignore_error= */ true); unexpected_data_parts.clear(); return true;