From 5b3161e0b532f25a0984cb9a98bf6994ce22ceda Mon Sep 17 00:00:00 2001 From: alesapin Date: Fri, 5 Mar 2021 20:24:06 +0300 Subject: [PATCH] Get rid of const_cast --- src/Storages/MergeTree/IMergeTreeDataPart.cpp | 5 +-- src/Storages/MergeTree/MergeTreeData.h | 2 +- .../MergeTree/MergeTreePartsMover.cpp | 35 ++++++++++++++++--- src/Storages/StorageReplicatedMergeTree.cpp | 7 ++-- src/Storages/StorageReplicatedMergeTree.h | 2 +- 5 files changed, 35 insertions(+), 16 deletions(-) diff --git a/src/Storages/MergeTree/IMergeTreeDataPart.cpp b/src/Storages/MergeTree/IMergeTreeDataPart.cpp index 591987404b5..1f18c894465 100644 --- a/src/Storages/MergeTree/IMergeTreeDataPart.cpp +++ b/src/Storages/MergeTree/IMergeTreeDataPart.cpp @@ -1174,10 +1174,7 @@ void IMergeTreeDataPart::makeCloneOnDisk(const DiskPtr & disk, const String & di disk->removeRecursive(path_to_clone + relative_path + '/'); } disk->createDirectories(path_to_clone); - - bool is_fetched = storage.tryToFetchIfShared(*this, disk, path_to_clone + "/" + name); - if (!is_fetched) - volume->getDisk()->copy(getFullRelativePath(), disk, path_to_clone); + volume->getDisk()->copy(getFullRelativePath(), disk, path_to_clone); volume->getDisk()->removeFileIfExists(path_to_clone + '/' + DELETE_ON_DESTROY_MARKER_FILE_NAME); } diff --git a/src/Storages/MergeTree/MergeTreeData.h b/src/Storages/MergeTree/MergeTreeData.h index 679518f8d5d..1f1505fe552 100644 --- a/src/Storages/MergeTree/MergeTreeData.h +++ b/src/Storages/MergeTree/MergeTreeData.h @@ -753,7 +753,7 @@ public: /// Fetch part only if some replica has it on shared storage like S3 /// Overridden in StorageReplicatedMergeTree - virtual bool tryToFetchIfShared(const IMergeTreeDataPart &, const DiskPtr &, const String &) const { return false; } + virtual bool tryToFetchIfShared(const IMergeTreeDataPart &, const DiskPtr &, const String &) { return false; } protected: diff --git a/src/Storages/MergeTree/MergeTreePartsMover.cpp b/src/Storages/MergeTree/MergeTreePartsMover.cpp index 7b8c88b1bff..41eae7fed38 100644 --- a/src/Storages/MergeTree/MergeTreePartsMover.cpp +++ b/src/Storages/MergeTree/MergeTreePartsMover.cpp @@ -194,15 +194,40 @@ MergeTreeData::DataPartPtr MergeTreePartsMover::clonePart(const MergeTreeMoveEnt if (moves_blocker.isCancelled()) throw Exception("Cancelled moving parts.", ErrorCodes::ABORTED); - LOG_TRACE(log, "Cloning part {}", moving_part.part->name); + auto settings = data->getSettings(); + auto part = moving_part.part; + LOG_TRACE(log, "Cloning part {}", part->name); + + auto disk = moving_part.reserved_space->getDisk(); const String directory_to_move = "moving"; - moving_part.part->makeCloneOnDisk(moving_part.reserved_space->getDisk(), directory_to_move); + if (settings->allow_s3_zero_copy_replication) + { + /// Try to fetch part from S3 without copy and fallback to default copy + /// if it's not possible + moving_part.part->assertOnDisk(); + String path_to_clone = data->getRelativeDataPath() + directory_to_move + '/'; + String relative_path = part->relative_path; + if (disk->exists(path_to_clone + relative_path)) + { + LOG_WARNING(log, "Path " + fullPath(disk, path_to_clone + relative_path) + " already exists. Will remove it and clone again."); + disk->removeRecursive(path_to_clone + relative_path + '/'); + } + disk->createDirectories(path_to_clone); + bool is_fetched = data->tryToFetchIfShared(*part, disk, path_to_clone + "/" + part->name); + if (!is_fetched) + part->volume->getDisk()->copy(data->getRelativeDataPath() + relative_path, disk, path_to_clone); + part->volume->getDisk()->removeFileIfExists(path_to_clone + '/' + IMergeTreeDataPart::DELETE_ON_DESTROY_MARKER_FILE_NAME); + } + else + { + part->makeCloneOnDisk(disk, directory_to_move); + } - auto single_disk_volume = std::make_shared("volume_" + moving_part.part->name, moving_part.reserved_space->getDisk(), 0); + auto single_disk_volume = std::make_shared("volume_" + part->name, moving_part.reserved_space->getDisk(), 0); MergeTreeData::MutableDataPartPtr cloned_part = - data->createPart(moving_part.part->name, single_disk_volume, directory_to_move + '/' + moving_part.part->name); - LOG_TRACE(log, "Part {} was cloned to {}", moving_part.part->name, cloned_part->getFullPath()); + data->createPart(part->name, single_disk_volume, directory_to_move + '/' + part->name); + LOG_TRACE(log, "Part {} was cloned to {}", part->name, cloned_part->getFullPath()); cloned_part->loadColumnsChecksumsIndexes(true, true); return cloned_part; diff --git a/src/Storages/StorageReplicatedMergeTree.cpp b/src/Storages/StorageReplicatedMergeTree.cpp index 1f2bd4f4775..ddc63793640 100644 --- a/src/Storages/StorageReplicatedMergeTree.cpp +++ b/src/Storages/StorageReplicatedMergeTree.cpp @@ -6552,7 +6552,7 @@ bool StorageReplicatedMergeTree::unlockSharedData(const IMergeTreeDataPart & par bool StorageReplicatedMergeTree::tryToFetchIfShared( const IMergeTreeDataPart & part, const DiskPtr & disk, - const String & path) const + const String & path) { const auto data_settings = getSettings(); if (!data_settings->allow_s3_zero_copy_replication) @@ -6567,10 +6567,7 @@ bool StorageReplicatedMergeTree::tryToFetchIfShared( if (replica.empty()) return false; - /// TODO: Fix const usage - StorageReplicatedMergeTree * replicated_storage_nc = const_cast(this); - - return replicated_storage_nc->executeFetchShared(replica, part.name, disk, path); + return executeFetchShared(replica, part.name, disk, path); } diff --git a/src/Storages/StorageReplicatedMergeTree.h b/src/Storages/StorageReplicatedMergeTree.h index 5bd10d93c8e..e3d7e6b2556 100644 --- a/src/Storages/StorageReplicatedMergeTree.h +++ b/src/Storages/StorageReplicatedMergeTree.h @@ -224,7 +224,7 @@ public: bool unlockSharedData(const IMergeTreeDataPart & part) const override; /// Fetch part only if some replica has it on shared storage like S3 - bool tryToFetchIfShared(const IMergeTreeDataPart & part, const DiskPtr & disk, const String & path) const override; + bool tryToFetchIfShared(const IMergeTreeDataPart & part, const DiskPtr & disk, const String & path) override; /// Get best replica having this partition on S3 String getSharedDataReplica(const IMergeTreeDataPart & part) const;