diff --git a/src/Disks/DiskDecorator.cpp b/src/Disks/DiskDecorator.cpp index 52e323487f2..f61496ab078 100644 --- a/src/Disks/DiskDecorator.cpp +++ b/src/Disks/DiskDecorator.cpp @@ -236,9 +236,9 @@ void DiskDecorator::applyNewSettings(const Poco::Util::AbstractConfiguration & c delegate->applyNewSettings(config, context, config_prefix, map); } -DiskObjectStoragePtr DiskDecorator::createDiskObjectStorage(const String & name) +DiskObjectStoragePtr DiskDecorator::createDiskObjectStorage() { - return delegate->createDiskObjectStorage(name); + return delegate->createDiskObjectStorage(); } } diff --git a/src/Disks/DiskDecorator.h b/src/Disks/DiskDecorator.h index 8d5e91e395e..81b0c24cbc7 100644 --- a/src/Disks/DiskDecorator.h +++ b/src/Disks/DiskDecorator.h @@ -88,8 +88,8 @@ public: StoredObjects getStorageObjects(const String & path) const override { return delegate->getStorageObjects(path); } void getRemotePathsRecursive(const String & path, std::vector & paths_map) override { return delegate->getRemotePathsRecursive(path, paths_map); } - DiskObjectStoragePtr createDiskObjectStorage(const String &) override; - const std::unordered_set & getCacheLayersNames() const override { return delegate->getCacheLayersNames(); } + DiskObjectStoragePtr createDiskObjectStorage() override; + NameSet getCacheLayersNames() const override { return delegate->getCacheLayersNames(); } MetadataStoragePtr getMetadataStorage() override { return delegate->getMetadataStorage(); } @@ -97,8 +97,6 @@ public: UInt32 getRefCount(const String & path) const override { return delegate->getRefCount(path); } - DiskPtr getWrappedDisk() const override { return delegate; } - void syncRevision(UInt64 revision) override { delegate->syncRevision(revision); } UInt64 getRevision() const override { return delegate->getRevision(); } diff --git a/src/Disks/DiskLocal.cpp b/src/Disks/DiskLocal.cpp index a96917e05c0..5af08179a0a 100644 --- a/src/Disks/DiskLocal.cpp +++ b/src/Disks/DiskLocal.cpp @@ -596,7 +596,7 @@ catch (...) return false; } -DiskObjectStoragePtr DiskLocal::createDiskObjectStorage(const String & name_) +DiskObjectStoragePtr DiskLocal::createDiskObjectStorage() { auto object_storage = std::make_shared(); auto metadata_storage = std::make_shared( @@ -605,9 +605,9 @@ DiskObjectStoragePtr DiskLocal::createDiskObjectStorage(const String & name_) /* object_storage_root_path */getPath()); return std::make_shared( - name_, + getName(), disk_path, - "DiskLocalObjectStorage", + "Local", metadata_storage, object_storage, DiskType::Local, diff --git a/src/Disks/DiskLocal.h b/src/Disks/DiskLocal.h index b202439bb2b..7d91abfe6b8 100644 --- a/src/Disks/DiskLocal.h +++ b/src/Disks/DiskLocal.h @@ -122,7 +122,7 @@ public: bool canRead() const noexcept; bool canWrite() const noexcept; - DiskObjectStoragePtr createDiskObjectStorage(const String & name_) override; + DiskObjectStoragePtr createDiskObjectStorage() override; private: std::optional tryReserve(UInt64 bytes); diff --git a/src/Disks/IDisk.h b/src/Disks/IDisk.h index 5cbff679bab..f4e37a3810a 100644 --- a/src/Disks/IDisk.h +++ b/src/Disks/IDisk.h @@ -222,16 +222,11 @@ public: virtual bool supportsCache() const { return false; } - virtual const std::unordered_set & getCacheLayersNames() const + virtual NameSet getCacheLayersNames() const { throw Exception(ErrorCodes::NOT_IMPLEMENTED, "Method `getCacheLayersNames()` is not implemented for disk: {}", getType()); } - virtual DiskPtr getWrappedDisk() const - { - throw Exception(ErrorCodes::NOT_IMPLEMENTED, "Method `getWrappedDisk()` is not implemented for disk: {}", getType()); - } - /// Returns a list of storage objects (contains path, size, ...). /// (A list is returned because for Log family engines there might /// be multiple files in remote fs for single clickhouse file. @@ -351,7 +346,7 @@ public: /// Return current disk revision. virtual UInt64 getRevision() const { return 0; } - virtual DiskObjectStoragePtr createDiskObjectStorage(const String &) + virtual DiskObjectStoragePtr createDiskObjectStorage() { throw Exception( ErrorCodes::NOT_IMPLEMENTED, diff --git a/src/Disks/ObjectStorages/Cached/CachedObjectStorage.cpp b/src/Disks/ObjectStorages/Cached/CachedObjectStorage.cpp index 81d69db6669..d7b4e9f495d 100644 --- a/src/Disks/ObjectStorages/Cached/CachedObjectStorage.cpp +++ b/src/Disks/ObjectStorages/Cached/CachedObjectStorage.cpp @@ -21,10 +21,14 @@ namespace ErrorCodes extern const int CANNOT_STAT; } -CachedObjectStorage:: CachedObjectStorage(ObjectStoragePtr object_storage_, FileCachePtr cache_) +CachedObjectStorage:: CachedObjectStorage( + ObjectStoragePtr object_storage_, + FileCachePtr cache_, + const std::string & cache_config_name_) : object_storage(object_storage_) , cache(cache_) - , log(&Poco::Logger::get(getName() + "(" + object_storage_->getName() +")")) + , cache_config_name(cache_config_name_) + , log(&Poco::Logger::get(getName())) { cache->initialize(); } diff --git a/src/Disks/ObjectStorages/Cached/CachedObjectStorage.h b/src/Disks/ObjectStorages/Cached/CachedObjectStorage.h index 7b5f9e75750..1442077a358 100644 --- a/src/Disks/ObjectStorages/Cached/CachedObjectStorage.h +++ b/src/Disks/ObjectStorages/Cached/CachedObjectStorage.h @@ -14,7 +14,7 @@ namespace DB class CachedObjectStorage : public IObjectStorage { public: - CachedObjectStorage(ObjectStoragePtr object_storage_, FileCachePtr cache_); + CachedObjectStorage(ObjectStoragePtr object_storage_, FileCachePtr cache_, const String & cache_config_name_); std::string getName() const override { return "CachedObjectStorage(" + object_storage->getName() + ")"; } @@ -95,6 +95,10 @@ public: bool isReadOnly() const override { return object_storage->isReadOnly(); } + const std::string & getCacheConfigName() const { return cache_config_name; } + + ObjectStoragePtr getWrappedObjectStorage() { return object_storage; } + private: IFileCache::Key getCacheKey(const std::string & path) const; String getCachePath(const std::string & path) const; @@ -102,6 +106,7 @@ private: ObjectStoragePtr object_storage; FileCachePtr cache; + std::string cache_config_name; Poco::Logger * log; }; diff --git a/src/Disks/ObjectStorages/Cached/registerDiskCache.cpp b/src/Disks/ObjectStorages/Cached/registerDiskCache.cpp index b3de9ed70d6..72b50013f41 100644 --- a/src/Disks/ObjectStorages/Cached/registerDiskCache.cpp +++ b/src/Disks/ObjectStorages/Cached/registerDiskCache.cpp @@ -35,6 +35,7 @@ void registerDiskCache(DiskFactory & factory) "There is not disk with name `{}`, disk name should be initialized before cache disk", disk_name); } + /// TODO: Add a check that there underlying storage is unique among cached storages. FileCacheSettings file_cache_settings; file_cache_settings.loadFromConfig(config, config_prefix); @@ -44,9 +45,10 @@ void registerDiskCache(DiskFactory & factory) fs::create_directories(cache_base_path); auto disk = disk_it->second; - auto disk_object_storage = disk->createDiskObjectStorage(disk_name); auto cache = FileCacheFactory::instance().getOrCreate(cache_base_path, file_cache_settings, name); + auto disk_object_storage = disk->createDiskObjectStorage(); + disk_object_storage->wrapWithCache(cache, name); LOG_TEST( diff --git a/src/Disks/ObjectStorages/DiskObjectStorage.cpp b/src/Disks/ObjectStorages/DiskObjectStorage.cpp index c3005440f3f..36818ec22f8 100644 --- a/src/Disks/ObjectStorages/DiskObjectStorage.cpp +++ b/src/Disks/ObjectStorages/DiskObjectStorage.cpp @@ -445,12 +445,12 @@ bool DiskObjectStorage::isReadOnly() const return object_storage->isReadOnly(); } -DiskObjectStoragePtr DiskObjectStorage::createDiskObjectStorage(const String & name_) +DiskObjectStoragePtr DiskObjectStorage::createDiskObjectStorage() { return std::make_shared( - name_, + getName(), object_storage_root_path, - name, + getName(), metadata_storage, object_storage, disk_type, @@ -460,8 +460,21 @@ DiskObjectStoragePtr DiskObjectStorage::createDiskObjectStorage(const String & n void DiskObjectStorage::wrapWithCache(FileCachePtr cache, const String & layer_name) { - object_storage = std::make_shared(object_storage, cache); - cache_layers.insert(layer_name); + object_storage = std::make_shared(object_storage, cache, layer_name); +} + +NameSet DiskObjectStorage::getCacheLayersNames() const +{ + NameSet cache_layers; + auto current_object_storage = object_storage; + while (current_object_storage->supportsCache()) + { + auto * cached_object_storage = assert_cast(current_object_storage.get()); + cache_layers.insert(cached_object_storage->getCacheConfigName()); + std::cerr << "\n\n" << "having cache layer: " << cached_object_storage->getCacheConfigName() << " for " << object_storage->getName() << "\n\n"; + current_object_storage = cached_object_storage->getWrappedObjectStorage(); + } + return cache_layers; } template diff --git a/src/Disks/ObjectStorages/DiskObjectStorage.h b/src/Disks/ObjectStorages/DiskObjectStorage.h index 0ebe5ed1ee7..a073b9caccc 100644 --- a/src/Disks/ObjectStorages/DiskObjectStorage.h +++ b/src/Disks/ObjectStorages/DiskObjectStorage.h @@ -164,7 +164,7 @@ public: UInt64 getRevision() const override; - DiskObjectStoragePtr createDiskObjectStorage(const String & name_) override; + DiskObjectStoragePtr createDiskObjectStorage() override; bool supportsCache() const override; @@ -186,7 +186,7 @@ public: String getStructure() const { return "DiskObjectStorage(" + object_storage->getName() + ")"; } /// Get names of all cache layers. Name is how cache is defined in configuration file. - const std::unordered_set & getCacheLayersNames() const override { return cache_layers; } + NameSet getCacheLayersNames() const override; private: @@ -212,8 +212,6 @@ private: size_t threadpool_size; std::unique_ptr metadata_helper; - - std::unordered_set cache_layers; }; using DiskObjectStoragePtr = std::shared_ptr; diff --git a/tests/config/config.d/storage_conf.xml b/tests/config/config.d/storage_conf.xml index c84d0d1af19..aedd389fa52 100644 --- a/tests/config/config.d/storage_conf.xml +++ b/tests/config/config.d/storage_conf.xml @@ -30,6 +30,13 @@ clickhouse clickhouse + + s3 + s3_disk_5/ + http://localhost:11111/test/00170_test/ + clickhouse + clickhouse + cache @@ -61,6 +68,12 @@ 1 1 + + cache + s3_disk_5 + s3_cache_5/ + 22548578304 + local @@ -96,6 +109,19 @@ 1 1 + + + cache + s3_cache_5 + s3_cache_multi/ + 22548578304 + + + cache + s3_cache_multi + s3_cache_multi_2/ + 22548578304 + @@ -126,6 +152,13 @@ + + +
+ s3_cache_multi_2 +
+
+
diff --git a/tests/queries/0_stateless/02313_filesystem_cache_seeks.reference b/tests/queries/0_stateless/02313_filesystem_cache_seeks.reference index d6c89d758f8..73871f55856 100644 --- a/tests/queries/0_stateless/02313_filesystem_cache_seeks.reference +++ b/tests/queries/0_stateless/02313_filesystem_cache_seeks.reference @@ -2,3 +2,5 @@ Using storage policy: s3_cache Using storage policy: local_cache +Using storage policy: s3_cache_multi + diff --git a/tests/queries/0_stateless/02313_filesystem_cache_seeks.sh b/tests/queries/0_stateless/02313_filesystem_cache_seeks.sh index 6576975c521..d1b059679b6 100755 --- a/tests/queries/0_stateless/02313_filesystem_cache_seeks.sh +++ b/tests/queries/0_stateless/02313_filesystem_cache_seeks.sh @@ -11,7 +11,7 @@ TMP_PATH=${CLICKHOUSE_TEST_UNIQUE_NAME} QUERIES_FILE=02313_filesystem_cache_seeks.queries TEST_FILE=$CUR_DIR/filesystem_cache_queries/$QUERIES_FILE -for storagePolicy in 's3_cache' 'local_cache'; do +for storagePolicy in 's3_cache' 'local_cache' 's3_cache_multi'; do echo "Using storage policy: $storagePolicy" cat $TEST_FILE | sed -e "s/_storagePolicy/${storagePolicy}/" > $TMP_PATH ${CLICKHOUSE_CLIENT} --queries-file $TMP_PATH