From c93262170df0c0fb09ccbb56f5f73b7a41bc58fc Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Mon, 31 Oct 2022 11:02:13 +0100 Subject: [PATCH] Remove ReadOnlyMetadataStorage Throw exceptions from IMetadataStorage instead to avoid introducing extra abstractions. Signed-off-by: Azat Khuzhin --- src/Disks/ObjectStorages/IMetadataStorage.h | 108 ++++++++++--- .../MetadataStorageFromPlainObjectStorage.h | 33 ++-- .../ObjectStorages/ReadOnlyMetadataStorage.h | 146 ------------------ .../MetadataStorageFromStaticFilesWebServer.h | 27 +++- 4 files changed, 136 insertions(+), 178 deletions(-) delete mode 100644 src/Disks/ObjectStorages/ReadOnlyMetadataStorage.h diff --git a/src/Disks/ObjectStorages/IMetadataStorage.h b/src/Disks/ObjectStorages/IMetadataStorage.h index 3d6c772157d..597d7744c78 100644 --- a/src/Disks/ObjectStorages/IMetadataStorage.h +++ b/src/Disks/ObjectStorages/IMetadataStorage.h @@ -11,10 +11,16 @@ #include #include #include +#include namespace DB { +namespace ErrorCodes +{ + extern const int NOT_IMPLEMENTED; +} + class IMetadataStorage; /// Tries to provide some "transactions" interface, which allow @@ -33,32 +39,71 @@ public: /// General purpose methods /// Write metadata string to file - virtual void writeStringToFile(const std::string & path, const std::string & data) = 0; + virtual void writeStringToFile(const std::string & /* path */, const std::string & /* data */) + { + throwNotImplemented(); + } - virtual void setLastModified(const std::string & path, const Poco::Timestamp & timestamp) = 0; + virtual void setLastModified(const std::string & /* path */, const Poco::Timestamp & /* timestamp */) + { + throwNotImplemented(); + } virtual bool supportsChmod() const = 0; - virtual void chmod(const String & path, mode_t mode) = 0; + virtual void chmod(const String & /* path */, mode_t /* mode */) + { + throwNotImplemented(); + } - virtual void setReadOnly(const std::string & path) = 0; + virtual void setReadOnly(const std::string & /* path */) + { + throwNotImplemented(); + } - virtual void unlinkFile(const std::string & path) = 0; + virtual void unlinkFile(const std::string & /* path */) + { + throwNotImplemented(); + } - virtual void createDirectory(const std::string & path) = 0; + virtual void createDirectory(const std::string & /* path */) + { + throwNotImplemented(); + } - virtual void createDirectoryRecursive(const std::string & path) = 0; + virtual void createDirectoryRecursive(const std::string & /* path */) + { + throwNotImplemented(); + } - virtual void removeDirectory(const std::string & path) = 0; + virtual void removeDirectory(const std::string & /* path */) + { + throwNotImplemented(); + } - virtual void removeRecursive(const std::string & path) = 0; + virtual void removeRecursive(const std::string & /* path */) + { + throwNotImplemented(); + } - virtual void createHardLink(const std::string & path_from, const std::string & path_to) = 0; + virtual void createHardLink(const std::string & /* path_from */, const std::string & /* path_to */) + { + throwNotImplemented(); + } - virtual void moveFile(const std::string & path_from, const std::string & path_to) = 0; + virtual void moveFile(const std::string & /* path_from */, const std::string & /* path_to */) + { + throwNotImplemented(); + } - virtual void moveDirectory(const std::string & path_from, const std::string & path_to) = 0; + virtual void moveDirectory(const std::string & /* path_from */, const std::string & /* path_to */) + { + throwNotImplemented(); + } - virtual void replaceFile(const std::string & path_from, const std::string & path_to) = 0; + virtual void replaceFile(const std::string & /* path_from */, const std::string & /* path_to */) + { + throwNotImplemented(); + } /// Metadata related methods @@ -69,7 +114,10 @@ public: virtual void createMetadataFile(const std::string & path, const std::string & blob_name, uint64_t size_in_bytes) = 0; /// Add to new blob to metadata file (way to implement appends) - virtual void addBlobToMetadata(const std::string & path, const std::string & blob_name, uint64_t size_in_bytes) = 0; + virtual void addBlobToMetadata(const std::string & /* path */, const std::string & /* blob_name */, uint64_t /* size_in_bytes */) + { + throwNotImplemented(); + } /// Unlink metadata file and do something special if required /// By default just remove file (unlink file). @@ -79,6 +127,12 @@ public: } virtual ~IMetadataTransaction() = default; + +private: + [[noreturn]] static void throwNotImplemented() + { + throw Exception(ErrorCodes::NOT_IMPLEMENTED, "Operation is not implemented"); + } }; using MetadataTransactionPtr = std::shared_ptr; @@ -106,12 +160,18 @@ public: virtual Poco::Timestamp getLastModified(const std::string & path) const = 0; - virtual time_t getLastChanged(const std::string & path) const = 0; + virtual time_t getLastChanged(const std::string & /* path */) const + { + throwNotImplemented(); + } virtual bool supportsChmod() const = 0; virtual bool supportsStat() const = 0; - virtual struct stat stat(const String & path) const = 0; + virtual struct stat stat(const String & /* path */) const + { + throwNotImplemented(); + } virtual std::vector listDirectory(const std::string & path) const = 0; @@ -120,20 +180,32 @@ public: virtual uint32_t getHardlinkCount(const std::string & path) const = 0; /// Read metadata file to string from path - virtual std::string readFileToString(const std::string & path) const = 0; + virtual std::string readFileToString(const std::string & /* path */) const + { + throwNotImplemented(); + } virtual ~IMetadataStorage() = default; /// ==== More specific methods. Previous were almost general purpose. ==== /// Read multiple metadata files into strings and return mapping from file_path -> metadata - virtual std::unordered_map getSerializedMetadata(const std::vector & file_paths) const = 0; + virtual std::unordered_map getSerializedMetadata(const std::vector & /* file_paths */) const + { + throwNotImplemented(); + } /// Return object information (absolute_path, bytes_size, ...) for metadata path. /// object_storage_path is absolute. virtual StoredObjects getStorageObjects(const std::string & path) const = 0; virtual std::string getObjectStorageRootPath() const = 0; + +private: + [[noreturn]] static void throwNotImplemented() + { + throw Exception(ErrorCodes::NOT_IMPLEMENTED, "Operation is not implemented"); + } }; using MetadataStoragePtr = std::shared_ptr; diff --git a/src/Disks/ObjectStorages/MetadataStorageFromPlainObjectStorage.h b/src/Disks/ObjectStorages/MetadataStorageFromPlainObjectStorage.h index e4c0fb90fb1..99cc960b9e4 100644 --- a/src/Disks/ObjectStorages/MetadataStorageFromPlainObjectStorage.h +++ b/src/Disks/ObjectStorages/MetadataStorageFromPlainObjectStorage.h @@ -2,7 +2,6 @@ #include #include -#include #include #include @@ -21,10 +20,7 @@ namespace DB /// It is used to allow BACKUP/RESTORE to ObjectStorage (S3/...) with the same /// structure as on disk MergeTree, and does not requires metadata from local /// disk to restore. -/// -/// NOTE: Inheritance from ReadOnlyMetadataStorage is used here to throw -/// NOT_IMPLEMENTED error for lost of unsupported methods (mtime/move/stat/...) -class MetadataStorageFromPlainObjectStorage final : public ReadOnlyMetadataStorage +class MetadataStorageFromPlainObjectStorage final : public IMetadataStorage { private: friend class MetadataStorageFromPlainObjectStorageTransaction; @@ -59,11 +55,25 @@ public: std::string getObjectStorageRootPath() const override { return object_storage_root_path; } + Poco::Timestamp getLastModified(const std::string & /* path */) const override + { + /// Required by MergeTree + return {}; + } + + uint32_t getHardlinkCount(const std::string & /* path */) const override + { + return 1; + } + + bool supportsChmod() const override { return false; } + bool supportsStat() const override { return false; } + private: std::filesystem::path getAbsolutePath(const std::string & path) const; }; -class MetadataStorageFromPlainObjectStorageTransaction final : public ReadOnlyMetadataTransaction +class MetadataStorageFromPlainObjectStorageTransaction final : public IMetadataTransaction { private: const MetadataStorageFromPlainObjectStorage & metadata_storage; @@ -74,9 +84,7 @@ public: : metadata_storage(metadata_storage_) {} - ~MetadataStorageFromPlainObjectStorageTransaction() override = default; - - const IMetadataStorage & getStorageForNonTransactionalReads() const final; + const IMetadataStorage & getStorageForNonTransactionalReads() const override; void addBlobToMetadata(const std::string & path, const std::string & blob_name, uint64_t size_in_bytes) override; @@ -97,6 +105,13 @@ public: void unlinkFile(const std::string & path) override; void unlinkMetadata(const std::string & path) override; + + void commit() override + { + /// Nothing to commit. + } + + bool supportsChmod() const override { return false; } }; } diff --git a/src/Disks/ObjectStorages/ReadOnlyMetadataStorage.h b/src/Disks/ObjectStorages/ReadOnlyMetadataStorage.h deleted file mode 100644 index f59bc0ad77f..00000000000 --- a/src/Disks/ObjectStorages/ReadOnlyMetadataStorage.h +++ /dev/null @@ -1,146 +0,0 @@ -#pragma once - -#include -#include - -namespace DB -{ - -namespace ErrorCodes -{ - extern const int NOT_IMPLEMENTED; -} - -class ReadOnlyMetadataStorage; - -/// Transaction for read-only storage, throws NOT_IMPLEMENTED error. -/// Can be used to add limited read-only support of MergeTree. -class ReadOnlyMetadataTransaction : public IMetadataTransaction -{ -public: - void commit() override - { - /// Noop, nothing to commit. - } - - void createEmptyMetadataFile(const std::string & /* path */) override - { - throwNotAllowed(); - } - void createMetadataFile(const std::string & /* path */, const std::string & /* blob_name */, uint64_t /* size_in_bytes */) override - { - throwNotAllowed(); - } - void writeStringToFile(const std::string & /* path */, const std::string & /* data */) override - { - throwNotAllowed(); - } - void setLastModified(const std::string & /* path */, const Poco::Timestamp & /* timestamp */) override - { - throwNotAllowed(); - } - void chmod(const String & /* path */, mode_t /* mode */) override - { - throwNotAllowed(); - } - void setReadOnly(const std::string & /* path */) override - { - throwNotAllowed(); - } - void unlinkFile(const std::string & /* path */) override - { - throwNotAllowed(); - } - void removeDirectory(const std::string & /* path */) override - { - throwNotAllowed(); - } - void removeRecursive(const std::string & /* path */) override - { - throwNotAllowed(); - } - void createHardLink(const std::string & /* path_from */, const std::string & /* path_to */) override - { - throwNotAllowed(); - } - void moveFile(const std::string & /* path_from */, const std::string & /* path_to */) override - { - throwNotAllowed(); - } - void moveDirectory(const std::string & /* path_from */, const std::string & /* path_to */) override - { - throwNotAllowed(); - } - void replaceFile(const std::string & /* path_from */, const std::string & /* path_to */) override - { - throwNotAllowed(); - } - void createDirectory(const std::string & /* path */) override - { - throwNotAllowed(); - } - void createDirectoryRecursive(const std::string & /* path */) override - { - throwNotAllowed(); - } - void addBlobToMetadata(const std::string & /* path */, const std::string & /* blob_name */, uint64_t /* size_in_bytes */) override - { - throwNotAllowed(); - } - void unlinkMetadata(const std::string & /* path */) override - { - throwNotAllowed(); - } - - bool supportsChmod() const override { return false; } - -private: - [[noreturn]] static void throwNotAllowed() - { - throw Exception(ErrorCodes::NOT_IMPLEMENTED, "Transaction for read-only storage is not supported"); - } -}; - -/// Readonly storage, throws NOT_IMPLEMENTED error. -/// Can be used to add limited read-only support of MergeTree. -class ReadOnlyMetadataStorage : public IMetadataStorage -{ -public: - Poco::Timestamp getLastModified(const std::string & /* path */) const override - { - /// Required by MergeTree - return {}; - } - uint32_t getHardlinkCount(const std::string & /* path */) const override - { - return 1; - } - - struct stat stat(const String & /* path */) const override - { - throwNotImplemented(); - } - time_t getLastChanged(const std::string & /* path */) const override - { - throwNotImplemented(); - } - std::string readFileToString(const std::string & /* path */) const override - { - throwNotImplemented(); - } - std::unordered_map getSerializedMetadata(const std::vector & /* file_paths */) const override - { - throwNotImplemented(); - } - - bool supportsChmod() const override { return false; } - bool supportsStat() const override { return false; } - -private: - [[noreturn]] static void throwNotImplemented() - { - throw Exception(ErrorCodes::NOT_IMPLEMENTED, "Operation is ont implemented"); - } -}; - -} diff --git a/src/Disks/ObjectStorages/Web/MetadataStorageFromStaticFilesWebServer.h b/src/Disks/ObjectStorages/Web/MetadataStorageFromStaticFilesWebServer.h index eb70b9c8108..338a2690b8f 100644 --- a/src/Disks/ObjectStorages/Web/MetadataStorageFromStaticFilesWebServer.h +++ b/src/Disks/ObjectStorages/Web/MetadataStorageFromStaticFilesWebServer.h @@ -1,7 +1,6 @@ #pragma once #include -#include #include #include #include @@ -10,7 +9,7 @@ namespace DB { -class MetadataStorageFromStaticFilesWebServer final : public ReadOnlyMetadataStorage +class MetadataStorageFromStaticFilesWebServer final : public IMetadataStorage { private: friend class MetadataStorageFromStaticFilesWebServerTransaction; @@ -46,9 +45,22 @@ public: std::string getObjectStorageRootPath() const override { return ""; } struct stat stat(const String & /* path */) const override { return {}; } + + Poco::Timestamp getLastModified(const std::string & /* path */) const override + { + /// Required by MergeTree + return {}; + } + uint32_t getHardlinkCount(const std::string & /* path */) const override + { + return 1; + } + + bool supportsChmod() const override { return false; } + bool supportsStat() const override { return false; } }; -class MetadataStorageFromStaticFilesWebServerTransaction final : public ReadOnlyMetadataTransaction +class MetadataStorageFromStaticFilesWebServerTransaction final : public IMetadataTransaction { private: DiskPtr disk; @@ -60,8 +72,6 @@ public: : metadata_storage(metadata_storage_) {} - ~MetadataStorageFromStaticFilesWebServerTransaction() override = default; - const IMetadataStorage & getStorageForNonTransactionalReads() const override; void createEmptyMetadataFile(const std::string & /* path */) override @@ -77,6 +87,13 @@ public: void createDirectory(const std::string & path) override; void createDirectoryRecursive(const std::string & path) override; + + void commit() override + { + /// Nothing to commit. + } + + bool supportsChmod() const override { return false; } }; }