diff --git a/src/Storages/MergeTree/DataPartStorageOnDiskBase.cpp b/src/Storages/MergeTree/DataPartStorageOnDiskBase.cpp index 9d0e0bf9dd43..f71d5437a7d0 100644 --- a/src/Storages/MergeTree/DataPartStorageOnDiskBase.cpp +++ b/src/Storages/MergeTree/DataPartStorageOnDiskBase.cpp @@ -754,6 +754,11 @@ void DataPartStorageOnDiskBase::remove( return; } + /// Evaluate can_remove_callback before moving the directory so zero-copy reference checks + /// use the current (existing) path. We intentionally don't update part_dir to avoid races. + if (!can_remove_description) + can_remove_description.emplace(can_remove_callback()); + try { disk->moveDirectory(from, to); @@ -766,9 +771,6 @@ void DataPartStorageOnDiskBase::remove( if (e.code() == ErrorCodes::FILE_DOESNT_EXIST) { LOG_ERROR(log, "Directory {} (part to remove) doesn't exist or one of nested files has gone. Most likely this is due to manual removing. This should be discouraged. Ignoring.", fullPath(disk, from)); - /// We will never touch this part again, so unlocking it from zero-copy - if (!can_remove_description) - can_remove_description.emplace(can_remove_callback()); return; } throw; @@ -779,10 +781,6 @@ void DataPartStorageOnDiskBase::remove( { LOG_ERROR(log, "Directory {} (part to remove) doesn't exist or one of nested files has gone. " "Most likely this is due to manual removing. This should be discouraged. Ignoring.", fullPath(disk, from)); - /// We will never touch this part again, so unlocking it from zero-copy - if (!can_remove_description) - can_remove_description.emplace(can_remove_callback()); - return; } throw;