Deprecetae local object storage on fake metadata storage

ClickHouse · Apr 15, 2023 · 9374666 · 9374666
1 parent c49ff08
commit 9374666
Show file tree

Hide file tree

Showing 7 changed files with 13 additions and 43 deletions.
diff --git a/src/Disks/DiskLocal.cpp b/src/Disks/DiskLocal.cpp
@@ -9,10 +9,6 @@
 #include <Common/quoteString.h>
 #include <Common/atomicRename.h>
 #include <Disks/IO/createReadBufferFromFileBase.h>
-#include <Disks/ObjectStorages/Local/LocalObjectStorage.h>
-#include <Disks/ObjectStorages/DiskObjectStorage.h>
-#include <Disks/ObjectStorages/FakeMetadataStorageFromDisk.h>
-#include <Disks/ObjectStorages/MetadataStorageFromDisk.h>
 #include <Disks/loadLocalDiskConfig.h>
 
 #include <fstream>
@@ -38,7 +34,6 @@ namespace DB
 namespace ErrorCodes
 {
     extern const int UNKNOWN_ELEMENT_IN_CONFIG;
-    extern const int EXCESSIVE_ELEMENT_IN_CONFIG;
     extern const int PATH_ACCESS_DENIED;
     extern const int LOGICAL_ERROR;
     extern const int CANNOT_TRUNCATE_FILE;
@@ -555,25 +550,6 @@ catch (...)
     return false;
 }
 
-DiskObjectStoragePtr DiskLocal::createDiskObjectStorage()
-{
-    auto object_storage = std::make_shared<LocalObjectStorage>();
-    auto metadata_storage = std::make_shared<FakeMetadataStorageFromDisk>(
-        /* metadata_storage */std::static_pointer_cast<DiskLocal>(shared_from_this()),
-        object_storage,
-        /* object_storage_root_path */getPath());
-
-    return std::make_shared<DiskObjectStorage>(
-        getName(),
-        disk_path,
-        "Local",
-        metadata_storage,
-        object_storage,
-        false,
-        /* threadpool_size */16
-    );
-}
-
 void DiskLocal::checkAccessImpl(const String & path)
 {
     try
@@ -701,13 +677,6 @@ void DiskLocal::chmod(const String & path, mode_t mode)
     DB::throwFromErrnoWithPath("Cannot chmod file: " + path, path, DB::ErrorCodes::PATH_ACCESS_DENIED);
 }
 
-MetadataStoragePtr DiskLocal::getMetadataStorage()
-{
-    auto object_storage = std::make_shared<LocalObjectStorage>();
-    return std::make_shared<FakeMetadataStorageFromDisk>(
-        std::static_pointer_cast<IDisk>(shared_from_this()), object_storage, getPath());
-}
-
 void registerDiskLocal(DiskFactory & factory, bool global_skip_access_check)
 {
     auto creator = [global_skip_access_check](

diff --git a/src/Disks/DiskLocal.h b/src/Disks/DiskLocal.h
@@ -121,16 +121,12 @@ class DiskLocal : public IDisk
     bool canRead() const noexcept;
     bool canWrite() const noexcept;
 
-    DiskObjectStoragePtr createDiskObjectStorage() override;
-
     bool supportsStat() const override { return true; }
     struct stat stat(const String & path) const override;
 
     bool supportsChmod() const override { return true; }
     void chmod(const String & path, mode_t mode) override;
 
-    MetadataStoragePtr getMetadataStorage() override;
-
 protected:
     void checkAccessImpl(const String & path) override;
 

diff --git a/src/Disks/IDisk.cpp b/src/Disks/IDisk.cpp
@@ -7,9 +7,6 @@
 #include <Common/logger_useful.h>
 #include <Common/setThreadName.h>
 #include <Core/ServerUUID.h>
-#include <Disks/ObjectStorages/MetadataStorageFromDisk.h>
-#include <Disks/ObjectStorages/FakeMetadataStorageFromDisk.h>
-#include <Disks/ObjectStorages/Local/LocalObjectStorage.h>
 #include <Disks/FakeDiskTransaction.h>
 
 namespace DB

diff --git a/src/Disks/IDisk.h b/src/Disks/IDisk.h
@@ -367,7 +367,13 @@ class IDisk : public Space
     /// Actually it's a part of IDiskRemote implementation but we have so
     /// complex hierarchy of disks (with decorators), so we cannot even
     /// dynamic_cast some pointer to IDisk to pointer to IDiskRemote.
-    virtual MetadataStoragePtr getMetadataStorage() = 0;
+    virtual MetadataStoragePtr getMetadataStorage()
+    {
+        throw Exception(
+            ErrorCodes::NOT_IMPLEMENTED,
+            "Method getMetadataStorage() is not implemented for disk type: {}",
+            toString(getDataSourceDescription().type));
+    }
 
     /// Very similar case as for getMetadataDiskIfExistsOrSelf(). If disk has "metadata"
     /// it will return mapping for each required path: path -> metadata as string.

diff --git a/src/Disks/ObjectStorages/Cached/registerDiskCache.cpp b/src/Disks/ObjectStorages/Cached/registerDiskCache.cpp
@@ -47,6 +47,9 @@ void registerDiskCache(DiskFactory & factory, bool /* global_skip_access_check *
         auto disk = disk_it->second;
 
         auto cache = FileCacheFactory::instance().getOrCreate(cache_base_path, file_cache_settings, name);
+        if (!dynamic_cast<const DiskObjectStorage *>(disk.get()))
+            throw Exception(ErrorCodes::BAD_ARGUMENTS, "Cached disk is allowed only on top of object storage");
+
         auto disk_object_storage = disk->createDiskObjectStorage();
 
         disk_object_storage->wrapWithCache(cache, file_cache_settings, name);

diff --git a/src/Disks/ObjectStorages/Local/LocalObjectStorage.cpp b/src/Disks/ObjectStorages/Local/LocalObjectStorage.cpp
@@ -20,7 +20,6 @@ namespace DB
 
 namespace ErrorCodes
 {
-    extern const int LOGICAL_ERROR;
     extern const int NOT_IMPLEMENTED;
 }
 

diff --git a/tests/config/config.d/storage_conf.xml b/tests/config/config.d/storage_conf.xml
@@ -120,15 +120,15 @@
             </s3_cache_small_segment_size>
             <!-- local disks -->
             <local_disk>
-                <type>local</type>
+                <type>local_blob_storage</type>
                 <path>local_disk/</path>
             </local_disk>
             <local_disk_2>
-                <type>local</type>
+                <type>local_blob_storage</type>
                 <path>local_disk_2/</path>
             </local_disk_2>
             <local_disk_3>
-                <type>local</type>
+                <type>local_blob_storage</type>
                 <path>local_disk_3/</path>
             </local_disk_3>
             <!-- cache for local disks -->