From 06ccdb1ecf04e3002832d45ed8b5eb2e710b5562 Mon Sep 17 00:00:00 2001 From: alesapin Date: Fri, 16 Sep 2022 13:49:39 +0200 Subject: [PATCH] Fix nasty bug --- src/Storages/MergeTree/DataPartStorageOnDisk.cpp | 5 ++--- src/Storages/MergeTree/DataPartStorageOnDisk.h | 2 +- src/Storages/MergeTree/IDataPartStorage.h | 2 +- src/Storages/MergeTree/IMergeTreeDataPart.cpp | 2 +- src/Storages/MergeTree/MergeTreeData.cpp | 6 +----- src/Storages/StorageReplicatedMergeTree.cpp | 2 +- 6 files changed, 7 insertions(+), 12 deletions(-) diff --git a/src/Storages/MergeTree/DataPartStorageOnDisk.cpp b/src/Storages/MergeTree/DataPartStorageOnDisk.cpp index 30c7cba5a08..bb9aab3c9b6 100644 --- a/src/Storages/MergeTree/DataPartStorageOnDisk.cpp +++ b/src/Storages/MergeTree/DataPartStorageOnDisk.cpp @@ -209,7 +209,7 @@ void DataPartStorageOnDisk::remove( std::list projections, bool is_temp, MergeTreeDataPartState state, - Poco::Logger * log) const + Poco::Logger * log) { /// NOTE We rename part to delete_tmp_ instead of delete_tmp_ to avoid race condition /// when we try to remove two parts with the same name, but different relative paths, @@ -259,6 +259,7 @@ void DataPartStorageOnDisk::remove( try { disk->moveDirectory(from, to); + onRename(root_path, part_dir_without_slash); } catch (const fs::filesystem_error & e) { @@ -271,9 +272,7 @@ void DataPartStorageOnDisk::remove( } if (!can_remove_description) - { can_remove_description.emplace(can_remove_callback()); - } // Record existing projection directories so we don't remove them twice std::unordered_set projection_directories; diff --git a/src/Storages/MergeTree/DataPartStorageOnDisk.h b/src/Storages/MergeTree/DataPartStorageOnDisk.h index c02f0b30fe5..51b557767d4 100644 --- a/src/Storages/MergeTree/DataPartStorageOnDisk.h +++ b/src/Storages/MergeTree/DataPartStorageOnDisk.h @@ -50,7 +50,7 @@ public: std::list projections, bool is_temp, MergeTreeDataPartState state, - Poco::Logger * log) const override; + Poco::Logger * log) override; std::string getRelativePathForPrefix(Poco::Logger * log, const String & prefix, bool detached) const override; diff --git a/src/Storages/MergeTree/IDataPartStorage.h b/src/Storages/MergeTree/IDataPartStorage.h index 9eb25a253b6..bd449d46075 100644 --- a/src/Storages/MergeTree/IDataPartStorage.h +++ b/src/Storages/MergeTree/IDataPartStorage.h @@ -125,7 +125,7 @@ public: std::list projections, bool is_temp, MergeTreeDataPartState state, - Poco::Logger * log) const = 0; + Poco::Logger * log) = 0; /// Get a name like 'prefix_partdir_tryN' which does not exist in a root dir. /// TODO: remove it. diff --git a/src/Storages/MergeTree/IMergeTreeDataPart.cpp b/src/Storages/MergeTree/IMergeTreeDataPart.cpp index c8f3f498abd..da8b6c932df 100644 --- a/src/Storages/MergeTree/IMergeTreeDataPart.cpp +++ b/src/Storages/MergeTree/IMergeTreeDataPart.cpp @@ -1433,7 +1433,7 @@ void IMergeTreeDataPart::remove() const assert(assertHasValidVersionMetadata()); part_is_probably_removed_from_disk = true; - auto can_remove_callback = [&] () + auto can_remove_callback = [this] () { auto [can_remove, files_not_to_remove] = canRemovePart(); if (!can_remove) diff --git a/src/Storages/MergeTree/MergeTreeData.cpp b/src/Storages/MergeTree/MergeTreeData.cpp index e89b21a13d4..59c6a676ac2 100644 --- a/src/Storages/MergeTree/MergeTreeData.cpp +++ b/src/Storages/MergeTree/MergeTreeData.cpp @@ -2187,11 +2187,7 @@ void MergeTreeData::dropAllData() try { LOG_INFO(log, "dropAllData: removing table directory recursive to cleanup garbage"); - - if (disk->supportZeroCopyReplication() && settings_ptr->allow_remote_fs_zero_copy_replication) - disk->removeSharedRecursive(relative_data_path, true, {}); - else - disk->removeRecursive(relative_data_path); + disk->removeRecursive(relative_data_path); } catch (const fs::filesystem_error & e) { diff --git a/src/Storages/StorageReplicatedMergeTree.cpp b/src/Storages/StorageReplicatedMergeTree.cpp index d704721abcc..295f201191a 100644 --- a/src/Storages/StorageReplicatedMergeTree.cpp +++ b/src/Storages/StorageReplicatedMergeTree.cpp @@ -7604,7 +7604,7 @@ std::pair StorageReplicatedMergeTree::unlockSharedData(const IMer } else { - LOG_TRACE(log, "Part {} looks temporary, because checksums file doesn't exists, blobs can be removed", part.name); + LOG_TRACE(log, "Part {} looks temporary, because {} file doesn't exists, blobs can be removed", part.name, IMergeTreeDataPart::FILE_FOR_REFERENCES_CHECK); /// Temporary part with some absent file cannot be locked in shared mode return std::make_pair(true, NameSet{}); }