From dd1346dab4f53bd0cade1f245bcff1b1f556c688 Mon Sep 17 00:00:00 2001 From: kssenii Date: Wed, 22 Jun 2022 13:24:40 +0200 Subject: [PATCH] Avoid strange abort --- .../MetadataStorageFromDiskTransaction.h | 12 +- ...taStorageFromDiskTransactionOperations.cpp | 277 -------------- ...dataStorageFromDiskTransactionOperations.h | 215 ----------- .../MetadataStorageFromLocalDisk.cpp | 3 +- .../MetadataStorageFromLocalDisk.h | 2 - .../MetadataStorageFromRemoteDisk.cpp | 340 +++++++++++++++++- 6 files changed, 349 insertions(+), 500 deletions(-) delete mode 100644 src/Disks/ObjectStorages/MetadataStorageFromDiskTransactionOperations.cpp delete mode 100644 src/Disks/ObjectStorages/MetadataStorageFromDiskTransactionOperations.h diff --git a/src/Disks/ObjectStorages/MetadataStorageFromDiskTransaction.h b/src/Disks/ObjectStorages/MetadataStorageFromDiskTransaction.h index 8ae043eb9f5..d6f8a0f0a4b 100644 --- a/src/Disks/ObjectStorages/MetadataStorageFromDiskTransaction.h +++ b/src/Disks/ObjectStorages/MetadataStorageFromDiskTransaction.h @@ -1,7 +1,6 @@ #pragma once #include -#include namespace DB { @@ -16,6 +15,16 @@ enum class MetadataFromDiskTransactionState std::string toString(MetadataFromDiskTransactionState state); +struct IMetadataOperation +{ + virtual void execute() = 0; + virtual void undo() = 0; + virtual void finalize() {} + virtual ~IMetadataOperation() = default; +}; + +using MetadataOperationPtr = std::unique_ptr; + /** * -> MetadataStorageFromRemoteDiskTransaction * IMetadataTransaction -> MetadataStorageFromDiskTransaction | @@ -32,6 +41,7 @@ public: void commit() final; + protected: void addOperation(MetadataOperationPtr && operation); diff --git a/src/Disks/ObjectStorages/MetadataStorageFromDiskTransactionOperations.cpp b/src/Disks/ObjectStorages/MetadataStorageFromDiskTransactionOperations.cpp deleted file mode 100644 index 060c28f13e9..00000000000 --- a/src/Disks/ObjectStorages/MetadataStorageFromDiskTransactionOperations.cpp +++ /dev/null @@ -1,277 +0,0 @@ -#include -#include -#include -#include -#include -#include -#include - -namespace fs = std::filesystem; - - -namespace DB -{ - -static std::string getTempFileName(const std::string & dir) -{ - return fs::path(dir) / getRandomASCIIString(); -} - -SetLastModifiedOperation::SetLastModifiedOperation(const std::string & path_, Poco::Timestamp new_timestamp_, IDisk & disk_) - : path(path_) - , new_timestamp(new_timestamp_) - , disk(disk_) -{ -} - -void SetLastModifiedOperation::execute() -{ - old_timestamp = disk.getLastModified(path); - disk.setLastModified(path, new_timestamp); -} - -void SetLastModifiedOperation::undo() -{ - disk.setLastModified(path, old_timestamp); -} - -UnlinkFileOperation::UnlinkFileOperation(const std::string & path_, IDisk & disk_) - : path(path_) - , disk(disk_) -{ -} - -void UnlinkFileOperation::execute() -{ - auto buf = disk.readFile(path); - readStringUntilEOF(prev_data, *buf); - std::cerr << "\n\n\nRemove file: " << path << "\n\n\n"; - disk.removeFile(path); -} - -void UnlinkFileOperation::undo() -{ - auto buf = disk.writeFile(path); - writeString(prev_data, *buf); - buf->finalize(); -} - -CreateDirectoryOperation::CreateDirectoryOperation(const std::string & path_, IDisk & disk_) - : path(path_) - , disk(disk_) -{ -} - -void CreateDirectoryOperation::execute() -{ - disk.createDirectory(path); -} - -void CreateDirectoryOperation::undo() -{ - disk.removeDirectory(path); -} - -CreateDirectoryRecursiveOperation::CreateDirectoryRecursiveOperation(const std::string & path_, IDisk & disk_) - : path(path_) - , disk(disk_) -{ -} - -void CreateDirectoryRecursiveOperation::execute() -{ - namespace fs = std::filesystem; - fs::path p(path); - while (!disk.exists(p)) - { - paths_created.push_back(p); - if (!p.has_parent_path()) - break; - p = p.parent_path(); - } - for (const auto & path_to_create : paths_created | std::views::reverse) - disk.createDirectory(path_to_create); -} - -void CreateDirectoryRecursiveOperation::undo() -{ - for (const auto & path_created : paths_created) - disk.removeDirectory(path_created); -} - -RemoveDirectoryOperation::RemoveDirectoryOperation(const std::string & path_, IDisk & disk_) - : path(path_) - , disk(disk_) -{ -} - -void RemoveDirectoryOperation::execute() -{ - disk.removeDirectory(path); -} - -void RemoveDirectoryOperation::undo() -{ - disk.createDirectory(path); -} - -RemoveRecursiveOperation::RemoveRecursiveOperation(const std::string & path_, IDisk & disk_) - : path(path_) - , disk(disk_) - , temp_path(getTempFileName(fs::path(path).parent_path())) -{ -} - -void RemoveRecursiveOperation:: execute() -{ - if (disk.isFile(path)) - disk.moveFile(path, temp_path); - else if (disk.isDirectory(path)) - disk.moveDirectory(path, temp_path); -} - -void RemoveRecursiveOperation::undo() -{ - if (disk.isFile(temp_path)) - disk.moveFile(temp_path, path); - else if (disk.isDirectory(temp_path)) - disk.moveDirectory(temp_path, path); -} - -void RemoveRecursiveOperation::finalize() -{ - if (disk.exists(temp_path)) - disk.removeRecursive(temp_path); - - if (disk.exists(path)) - disk.removeRecursive(path); -} - -CreateHardlinkOperation::CreateHardlinkOperation(const std::string & path_from_, const std::string & path_to_, IDisk & disk_) - : path_from(path_from_) - , path_to(path_to_) - , disk(disk_) -{ -} - -void CreateHardlinkOperation::execute() -{ - disk.createHardLink(path_from, path_to); -} - -void CreateHardlinkOperation::undo() -{ - disk.removeFile(path_to); -} - -MoveFileOperation::MoveFileOperation(const std::string & path_from_, const std::string & path_to_, IDisk & disk_) - : path_from(path_from_) - , path_to(path_to_) - , disk(disk_) -{ -} - -void MoveFileOperation::execute() -{ - disk.moveFile(path_from, path_to); -} - -void MoveFileOperation::undo() -{ - disk.moveFile(path_to, path_from); -} - -MoveDirectoryOperation::MoveDirectoryOperation(const std::string & path_from_, const std::string & path_to_, IDisk & disk_) - : path_from(path_from_) - , path_to(path_to_) - , disk(disk_) -{ -} - -void MoveDirectoryOperation::execute() -{ - disk.moveDirectory(path_from, path_to); -} - -void MoveDirectoryOperation::undo() -{ - disk.moveDirectory(path_to, path_from); -} - - -ReplaceFileOperation::ReplaceFileOperation(const std::string & path_from_, const std::string & path_to_, IDisk & disk_) - : path_from(path_from_) - , path_to(path_to_) - , disk(disk_) - , temp_path_to(getTempFileName(fs::path(path_to).parent_path())) -{ -} - -void ReplaceFileOperation::execute() -{ - if (disk.exists(path_to)) - disk.moveFile(path_to, temp_path_to); - - disk.replaceFile(path_from, path_to); -} - -void ReplaceFileOperation::undo() -{ - disk.moveFile(path_to, path_from); - disk.moveFile(temp_path_to, path_to); -} - -void ReplaceFileOperation::finalize() -{ - disk.removeFileIfExists(temp_path_to); -} - -WriteFileOperation::WriteFileOperation(const std::string & path_, IDisk & disk_, const std::string & data_) - : path(path_) - , disk(disk_) - , data(data_) -{ -} - -void WriteFileOperation::execute() -{ - if (disk.exists(path)) - { - existed = true; - auto buf = disk.readFile(path); - readStringUntilEOF(prev_data, *buf); - } - auto buf = disk.writeFile(path); - writeString(data, *buf); - buf->finalize(); -} - -void WriteFileOperation::undo() -{ - if (!existed) - { - disk.removeFileIfExists(path); - } - else - { - auto buf = disk.writeFile(path); - writeString(prev_data, *buf); - } -} - -SetReadOnlyOperation::SetReadOnlyOperation(const std::string & path_, IDisk & disk_) - : path(path_) - , disk(disk_) -{ -} - -void SetReadOnlyOperation::execute() -{ - disk.setReadOnly(path); -} - -void SetReadOnlyOperation::undo() -{ -} - -} diff --git a/src/Disks/ObjectStorages/MetadataStorageFromDiskTransactionOperations.h b/src/Disks/ObjectStorages/MetadataStorageFromDiskTransactionOperations.h deleted file mode 100644 index bd2619c0221..00000000000 --- a/src/Disks/ObjectStorages/MetadataStorageFromDiskTransactionOperations.h +++ /dev/null @@ -1,215 +0,0 @@ -#pragma once - -#include - -namespace DB -{ -class IDisk; - - -struct IMetadataOperation -{ - virtual void execute() = 0; - virtual void undo() = 0; - virtual void finalize() {} - virtual ~IMetadataOperation() = default; -}; - -using MetadataOperationPtr = std::unique_ptr; - - -class SetLastModifiedOperation final : public IMetadataOperation -{ -public: - SetLastModifiedOperation(const std::string & path_, Poco::Timestamp new_timestamp_, IDisk & disk_); - - void execute() override; - - void undo() override; - -private: - std::string path; - Poco::Timestamp new_timestamp; - Poco::Timestamp old_timestamp; - IDisk & disk; -}; - - -class UnlinkFileOperation final : public IMetadataOperation -{ -public: - UnlinkFileOperation(const std::string & path_, IDisk & disk_); - - void execute() override; - - void undo() override; - -private: - std::string path; - IDisk & disk; - std::string prev_data; -}; - - -class CreateDirectoryOperation final : public IMetadataOperation -{ -public: - CreateDirectoryOperation(const std::string & path_, IDisk & disk_); - - void execute() override; - - void undo() override; - -private: - std::string path; - IDisk & disk; -}; - - -class CreateDirectoryRecursiveOperation final : public IMetadataOperation -{ -public: - CreateDirectoryRecursiveOperation(const std::string & path_, IDisk & disk_); - - void execute() override; - - void undo() override; - -private: - std::string path; - std::vector paths_created; - IDisk & disk; -}; - - -class RemoveDirectoryOperation final : public IMetadataOperation -{ -public: - RemoveDirectoryOperation(const std::string & path_, IDisk & disk_); - - void execute() override; - - void undo() override; - -private: - std::string path; - IDisk & disk; -}; - -class RemoveRecursiveOperation final : public IMetadataOperation -{ -public: - RemoveRecursiveOperation(const std::string & path_, IDisk & disk_); - - void execute() override; - - void undo() override; - - void finalize() override; - -private: - std::string path; - IDisk & disk; - std::string temp_path; -}; - - -class CreateHardlinkOperation final : public IMetadataOperation -{ -public: - CreateHardlinkOperation(const std::string & path_from_, const std::string & path_to_, IDisk & disk_); - - void execute() override; - - void undo() override; - -private: - std::string path_from; - std::string path_to; - IDisk & disk; -}; - - -class MoveFileOperation final : public IMetadataOperation -{ -public: - MoveFileOperation(const std::string & path_from_, const std::string & path_to_, IDisk & disk_); - - void execute() override; - - void undo() override; - -private: - std::string path_from; - std::string path_to; - IDisk & disk; -}; - - -class MoveDirectoryOperation final : public IMetadataOperation -{ -public: - MoveDirectoryOperation(const std::string & path_from_, const std::string & path_to_, IDisk & disk_); - - void execute() override; - - void undo() override; - -private: - std::string path_from; - std::string path_to; - IDisk & disk; -}; - - -class ReplaceFileOperation final : public IMetadataOperation -{ -public: - ReplaceFileOperation(const std::string & path_from_, const std::string & path_to_, IDisk & disk_); - - void execute() override; - - void undo() override; - - void finalize() override; - -private: - std::string path_from; - std::string path_to; - IDisk & disk; - std::string temp_path_to; -}; - - -class WriteFileOperation final : public IMetadataOperation -{ -public: - WriteFileOperation(const std::string & path_, IDisk & disk_, const std::string & data_); - - void execute() override; - - void undo() override; - -private: - std::string path; - IDisk & disk; - std::string data; - bool existed = false; - std::string prev_data; -}; - -class SetReadOnlyOperation final : public IMetadataOperation -{ -public: - SetReadOnlyOperation(const std::string & path_, IDisk & disk_); - - void execute() override; - - void undo() override; - -private: - std::string path; - IDisk & disk; -}; - -} diff --git a/src/Disks/ObjectStorages/MetadataStorageFromLocalDisk.cpp b/src/Disks/ObjectStorages/MetadataStorageFromLocalDisk.cpp index 9ebe545d5fd..1e13b338b32 100644 --- a/src/Disks/ObjectStorages/MetadataStorageFromLocalDisk.cpp +++ b/src/Disks/ObjectStorages/MetadataStorageFromLocalDisk.cpp @@ -176,7 +176,8 @@ void MetadataStorageFromLocalDiskTransaction::addBlobToMetadata( void MetadataStorageFromLocalDiskTransaction::unlinkMetadata(const std::string & path) { - addOperation(std::make_unique(path, *metadata_storage_for_local.getDisk())); + disk->removeFile(path); + /// addOperation(std::make_unique(path, *metadata_storage_for_local.getDisk())); } } diff --git a/src/Disks/ObjectStorages/MetadataStorageFromLocalDisk.h b/src/Disks/ObjectStorages/MetadataStorageFromLocalDisk.h index e9c9545543d..e8489101c7c 100644 --- a/src/Disks/ObjectStorages/MetadataStorageFromLocalDisk.h +++ b/src/Disks/ObjectStorages/MetadataStorageFromLocalDisk.h @@ -54,13 +54,11 @@ private: class MetadataStorageFromLocalDiskTransaction final : public MetadataStorageFromDiskTransaction { private: - const MetadataStorageFromLocalDisk & metadata_storage_for_local; DiskPtr disk; public: explicit MetadataStorageFromLocalDiskTransaction(const MetadataStorageFromLocalDisk & metadata_storage_, DiskPtr disk_) : MetadataStorageFromDiskTransaction(metadata_storage_) - , metadata_storage_for_local(metadata_storage_) , disk(disk_) {} diff --git a/src/Disks/ObjectStorages/MetadataStorageFromRemoteDisk.cpp b/src/Disks/ObjectStorages/MetadataStorageFromRemoteDisk.cpp index 0b24ea4258b..349d10cbdf6 100644 --- a/src/Disks/ObjectStorages/MetadataStorageFromRemoteDisk.cpp +++ b/src/Disks/ObjectStorages/MetadataStorageFromRemoteDisk.cpp @@ -1,10 +1,12 @@ #include -#include +#include +#include +#include +#include +#include +#include #include #include -#include -#include -#include namespace DB { @@ -14,6 +16,336 @@ namespace ErrorCodes extern const int FS_METADATA_ERROR; } +namespace +{ + +std::string getTempFileName(const std::string & dir) +{ + return fs::path(dir) / getRandomASCIIString(); +} + +class SetLastModifiedOperation final : public IMetadataOperation +{ + std::string path; + Poco::Timestamp new_timestamp; + Poco::Timestamp old_timestamp; + IDisk & disk; +public: + SetLastModifiedOperation(const std::string & path_, Poco::Timestamp new_timestamp_, IDisk & disk_) + : path(path_) + , new_timestamp(new_timestamp_) + , disk(disk_) + {} + + void execute() override + { + old_timestamp = disk.getLastModified(path); + disk.setLastModified(path, new_timestamp); + } + + void undo() override + { + disk.setLastModified(path, old_timestamp); + } +}; + +class UnlinkFileOperation final : public IMetadataOperation +{ + std::string path; + IDisk & disk; + std::string prev_data; +public: + UnlinkFileOperation(const std::string & path_, IDisk & disk_) + : path(path_) + , disk(disk_) + { + } + + void execute() override + { + auto buf = disk.readFile(path); + readStringUntilEOF(prev_data, *buf); + disk.removeFile(path); + } + + void undo() override + { + auto buf = disk.writeFile(path); + writeString(prev_data, *buf); + buf->finalize(); + } +}; + +class CreateDirectoryOperation final : public IMetadataOperation +{ +private: + std::string path; + IDisk & disk; +public: + CreateDirectoryOperation(const std::string & path_, IDisk & disk_) + : path(path_) + , disk(disk_) + { + } + + void execute() override + { + disk.createDirectory(path); + } + + void undo() override + { + disk.removeDirectory(path); + } +}; + +class CreateDirectoryRecursiveOperation final : public IMetadataOperation +{ +private: + std::string path; + std::vector paths_created; + IDisk & disk; +public: + CreateDirectoryRecursiveOperation(const std::string & path_, IDisk & disk_) + : path(path_) + , disk(disk_) + { + } + + void execute() override + { + namespace fs = std::filesystem; + fs::path p(path); + while (!disk.exists(p)) + { + paths_created.push_back(p); + if (!p.has_parent_path()) + break; + p = p.parent_path(); + } + for (const auto & path_to_create : paths_created | std::views::reverse) + disk.createDirectory(path_to_create); + } + + void undo() override + { + for (const auto & path_created : paths_created) + disk.removeDirectory(path_created); + } +}; + +class RemoveDirectoryOperation final : public IMetadataOperation +{ +private: + std::string path; + IDisk & disk; +public: + RemoveDirectoryOperation(const std::string & path_, IDisk & disk_) + : path(path_) + , disk(disk_) + {} + + void execute() override + { + disk.removeDirectory(path); + } + + void undo() override + { + disk.createDirectory(path); + } +}; + +class RemoveRecursiveOperation final : public IMetadataOperation +{ + std::string path; + IDisk & disk; + std::string temp_path; +public: + RemoveRecursiveOperation(const std::string & path_, IDisk & disk_) + : path(path_) + , disk(disk_) + , temp_path(getTempFileName(fs::path(path).parent_path())) + { + } + + void execute() override + { + if (disk.isFile(path)) + disk.moveFile(path, temp_path); + else if (disk.isDirectory(path)) + disk.moveDirectory(path, temp_path); + } + + void undo() override + { + if (disk.isFile(temp_path)) + disk.moveFile(temp_path, path); + else if (disk.isDirectory(temp_path)) + disk.moveDirectory(temp_path, path); + } + + void finalize() override + { + if (disk.exists(temp_path)) + disk.removeRecursive(temp_path); + + if (disk.exists(path)) + disk.removeRecursive(path); + } +}; + + +class CreateHardlinkOperation final : public IMetadataOperation +{ +private: + std::string path_from; + std::string path_to; + IDisk & disk; +public: + CreateHardlinkOperation(const std::string & path_from_, const std::string & path_to_, IDisk & disk_) + : path_from(path_from_) + , path_to(path_to_) + , disk(disk_) + {} + + void execute() override + { + disk.createHardLink(path_from, path_to); + } + + void undo() override + { + disk.removeFile(path_to); + } +}; + +class MoveFileOperation final : public IMetadataOperation +{ +private: + std::string path_from; + std::string path_to; + IDisk & disk; +public: + MoveFileOperation(const std::string & path_from_, const std::string & path_to_, IDisk & disk_) + : path_from(path_from_) + , path_to(path_to_) + , disk(disk_) + {} + + void execute() override + { + disk.moveFile(path_from, path_to); + } + + void undo() override + { + disk.moveFile(path_to, path_from); + } +}; + +class MoveDirectoryOperation final : public IMetadataOperation +{ +private: + std::string path_from; + std::string path_to; + IDisk & disk; +public: + MoveDirectoryOperation(const std::string & path_from_, const std::string & path_to_, IDisk & disk_) + : path_from(path_from_) + , path_to(path_to_) + , disk(disk_) + {} + + void execute() override + { + disk.moveDirectory(path_from, path_to); + } + + void undo() override + { + disk.moveDirectory(path_to, path_from); + } +}; + + +class ReplaceFileOperation final : public IMetadataOperation +{ +private: + std::string path_from; + std::string path_to; + IDisk & disk; + std::string temp_path_to; +public: + ReplaceFileOperation(const std::string & path_from_, const std::string & path_to_, IDisk & disk_) + : path_from(path_from_) + , path_to(path_to_) + , disk(disk_) + , temp_path_to(getTempFileName(fs::path(path_to).parent_path())) + { + } + + void execute() override + { + if (disk.exists(path_to)) + disk.moveFile(path_to, temp_path_to); + + disk.replaceFile(path_from, path_to); + } + + void undo() override + { + disk.moveFile(path_to, path_from); + disk.moveFile(temp_path_to, path_to); + } + + void finalize() override + { + disk.removeFileIfExists(temp_path_to); + } +}; + +class WriteFileOperation final : public IMetadataOperation +{ +private: + std::string path; + IDisk & disk; + std::string data; + bool existed = false; + std::string prev_data; +public: + WriteFileOperation(const std::string & path_, IDisk & disk_, const std::string & data_) + : path(path_) + , disk(disk_) + , data(data_) + {} + + void execute() override + { + if (disk.exists(path)) + { + existed = true; + auto buf = disk.readFile(path); + readStringUntilEOF(prev_data, *buf); + } + auto buf = disk.writeFile(path); + writeString(data, *buf); + buf->finalize(); + } + + void undo() override + { + if (!existed) + disk.removeFileIfExists(path); + else + { + auto buf = disk.writeFile(path); + writeString(prev_data, *buf); + } + } +}; + +} const std::string & MetadataStorageFromRemoteDisk::getPath() const {