Disable using fs cache for backup/restore #2.

vitlibar · vitlibar · commit e127d2745371 · 2023-08-03T13:52:35.000+02:00
diff --git a/src/Backups/BackupEntriesCollector.cpp b/src/Backups/BackupEntriesCollector.cpp
@@ -77,10 +77,12 @@ BackupEntriesCollector::BackupEntriesCollector(
     const ASTBackupQuery::Elements & backup_query_elements_,
     const BackupSettings & backup_settings_,
     std::shared_ptr<IBackupCoordination> backup_coordination_,
+    const ReadSettings & read_settings_,
     const ContextPtr & context_)
     : backup_query_elements(backup_query_elements_)
     , backup_settings(backup_settings_)
     , backup_coordination(backup_coordination_)
+    , read_settings(read_settings_)
     , context(context_)
     , on_cluster_first_sync_timeout(context->getConfigRef().getUInt64("backups.on_cluster_first_sync_timeout", 180000))
     , consistent_metadata_snapshot_timeout(context->getConfigRef().getUInt64("backups.consistent_metadata_snapshot_timeout", 600000))
diff --git a/src/Backups/BackupEntriesCollector.h b/src/Backups/BackupEntriesCollector.h
@@ -30,6 +30,7 @@ class BackupEntriesCollector : private boost::noncopyable
     BackupEntriesCollector(const ASTBackupQuery::Elements & backup_query_elements_,
                            const BackupSettings & backup_settings_,
                            std::shared_ptr<IBackupCoordination> backup_coordination_,
+                           const ReadSettings & read_settings_,
                            const ContextPtr & context_);
     ~BackupEntriesCollector();
 
@@ -40,6 +41,7 @@ class BackupEntriesCollector : private boost::noncopyable
 
     const BackupSettings & getBackupSettings() const { return backup_settings; }
     std::shared_ptr<IBackupCoordination> getBackupCoordination() const { return backup_coordination; }
+    const ReadSettings & getReadSettings() const { return read_settings; }
     ContextPtr getContext() const { return context; }
 
     /// Adds a backup entry which will be later returned by run().
@@ -93,6 +95,7 @@ class BackupEntriesCollector : private boost::noncopyable
     const ASTBackupQuery::Elements backup_query_elements;
     const BackupSettings backup_settings;
     std::shared_ptr<IBackupCoordination> backup_coordination;
+    const ReadSettings read_settings;
     ContextPtr context;
     std::chrono::milliseconds on_cluster_first_sync_timeout;
     std::chrono::milliseconds consistent_metadata_snapshot_timeout;
diff --git a/src/Backups/BackupEntryFromSmallFile.cpp b/src/Backups/BackupEntryFromSmallFile.cpp
@@ -11,37 +11,37 @@ namespace DB
 {
 namespace
 {
-    String readFile(const String & file_path)
+    String readFile(const String & file_path, const ReadSettings & read_settings)
     {
-        auto buf = createReadBufferFromFileBase(file_path, /* settings= */ {});
+        auto buf = createReadBufferFromFileBase(file_path, read_settings);
         String s;
         readStringUntilEOF(s, *buf);
         return s;
     }
 
-    String readFile(const DiskPtr & disk, const String & file_path, bool copy_encrypted)
+    String readFile(const DiskPtr & disk, const String & file_path, const ReadSettings & read_settings, bool copy_encrypted)
     {
-        auto buf = copy_encrypted ? disk->readEncryptedFile(file_path, {}) : disk->readFile(file_path);
+        auto buf = copy_encrypted ? disk->readEncryptedFile(file_path, read_settings) : disk->readFile(file_path, read_settings);
         String s;
         readStringUntilEOF(s, *buf);
         return s;
     }
 }
 
 
-BackupEntryFromSmallFile::BackupEntryFromSmallFile(const String & file_path_)
+BackupEntryFromSmallFile::BackupEntryFromSmallFile(const String & file_path_, const ReadSettings & read_settings_)
     : file_path(file_path_)
     , data_source_description(DiskLocal::getLocalDataSourceDescription(file_path_))
-    , data(readFile(file_path_))
+    , data(readFile(file_path_, read_settings_))
 {
 }
 
-BackupEntryFromSmallFile::BackupEntryFromSmallFile(const DiskPtr & disk_, const String & file_path_, bool copy_encrypted_)
+BackupEntryFromSmallFile::BackupEntryFromSmallFile(const DiskPtr & disk_, const String & file_path_, const ReadSettings & read_settings_, bool copy_encrypted_)
     : disk(disk_)
     , file_path(file_path_)
     , data_source_description(disk_->getDataSourceDescription())
     , copy_encrypted(copy_encrypted_ && data_source_description.is_encrypted)
-    , data(readFile(disk_, file_path, copy_encrypted))
+    , data(readFile(disk_, file_path, read_settings_, copy_encrypted))
 {
 }
 
diff --git a/src/Backups/BackupEntryFromSmallFile.h b/src/Backups/BackupEntryFromSmallFile.h
@@ -13,8 +13,8 @@ using DiskPtr = std::shared_ptr<IDisk>;
 class BackupEntryFromSmallFile : public BackupEntryWithChecksumCalculation<IBackupEntry>
 {
 public:
-    explicit BackupEntryFromSmallFile(const String & file_path_);
-    BackupEntryFromSmallFile(const DiskPtr & disk_, const String & file_path_, bool copy_encrypted_ = false);
+    explicit BackupEntryFromSmallFile(const String & file_path_, const ReadSettings & read_settings_);
+    BackupEntryFromSmallFile(const DiskPtr & disk_, const String & file_path_, const ReadSettings & read_settings_, bool copy_encrypted_ = false);
 
     std::unique_ptr<SeekableReadBuffer> getReadBuffer(const ReadSettings &) const override;
     UInt64 getSize() const override { return data.size(); }
diff --git a/src/Backups/BackupsWorker.cpp b/src/Backups/BackupsWorker.cpp
@@ -186,7 +186,7 @@ namespace
         auto read_settings = context->getReadSettings();
         read_settings.remote_throttler = context->getBackupsThrottler();
         read_settings.local_throttler = context->getBackupsThrottler();
-        read_settings.enable_filesystem_cache = false;
+        read_settings.enable_filesystem_cache = backup_settings.read_from_filesystem_cache_if_exists_otherwise_bypass_cache;
         read_settings.read_from_filesystem_cache_if_exists_otherwise_bypass_cache = backup_settings.read_from_filesystem_cache_if_exists_otherwise_bypass_cache;
         return read_settings;
     }
@@ -416,7 +416,7 @@ void BackupsWorker::doBackup(
             /// Prepare backup entries.
             BackupEntries backup_entries;
             {
-                BackupEntriesCollector backup_entries_collector{backup_query->elements, backup_settings, backup_coordination, context};
+                BackupEntriesCollector backup_entries_collector{backup_query->elements, backup_settings, backup_coordination, backup_create_params.read_settings, context};
                 backup_entries = backup_entries_collector.run();
             }
 
diff --git a/src/Storages/MergeTree/DataPartStorageOnDiskBase.cpp b/src/Storages/MergeTree/DataPartStorageOnDiskBase.cpp
@@ -331,6 +331,7 @@ void DataPartStorageOnDiskBase::backup(
     const NameSet & files_without_checksums,
     const String & path_in_backup,
     const BackupSettings & backup_settings,
+    const ReadSettings & read_settings,
     bool make_temporary_hard_links,
     BackupEntries & backup_entries,
     TemporaryFilesOnDisks * temp_dirs) const
@@ -382,7 +383,7 @@ void DataPartStorageOnDiskBase::backup(
 
         if (files_without_checksums.contains(filepath))
         {
-            backup_entries.emplace_back(filepath_in_backup, std::make_unique<BackupEntryFromSmallFile>(disk, filepath_on_disk, copy_encrypted));
+            backup_entries.emplace_back(filepath_in_backup, std::make_unique<BackupEntryFromSmallFile>(disk, filepath_on_disk, read_settings, copy_encrypted));
             continue;
         }
 
diff --git a/src/Storages/MergeTree/DataPartStorageOnDiskBase.h b/src/Storages/MergeTree/DataPartStorageOnDiskBase.h
@@ -55,6 +55,7 @@ class DataPartStorageOnDiskBase : public IDataPartStorage
         const NameSet & files_without_checksums,
         const String & path_in_backup,
         const BackupSettings & backup_settings,
+        const ReadSettings & read_settings,
         bool make_temporary_hard_links,
         BackupEntries & backup_entries,
         TemporaryFilesOnDisks * temp_dirs) const override;
diff --git a/src/Storages/MergeTree/IDataPartStorage.h b/src/Storages/MergeTree/IDataPartStorage.h
@@ -221,6 +221,7 @@ class IDataPartStorage : public boost::noncopyable
         const NameSet & files_without_checksums,
         const String & path_in_backup,
         const BackupSettings & backup_settings,
+        const ReadSettings & read_settings,
         bool make_temporary_hard_links,
         BackupEntries & backup_entries,
         TemporaryFilesOnDisks * temp_dirs) const = 0;
diff --git a/src/Storages/MergeTree/MergeTreeData.cpp b/src/Storages/MergeTree/MergeTreeData.cpp
@@ -5148,6 +5148,7 @@ MergeTreeData::PartsBackupEntries MergeTreeData::backupParts(
     const DataPartsVector & data_parts,
     const String & data_path_in_backup,
     const BackupSettings & backup_settings,
+    const ReadSettings & read_settings,
     const ContextPtr & local_context)
 {
     MergeTreeData::PartsBackupEntries res;
@@ -5186,6 +5187,7 @@ MergeTreeData::PartsBackupEntries MergeTreeData::backupParts(
             part->getFileNamesWithoutChecksums(),
             data_path_in_backup,
             backup_settings,
+            read_settings,
             make_temporary_hard_links,
             backup_entries_from_part,
             &temp_dirs);
@@ -5198,6 +5200,7 @@ MergeTreeData::PartsBackupEntries MergeTreeData::backupParts(
                 projection_part->getFileNamesWithoutChecksums(),
                 fs::path{data_path_in_backup} / part->name,
                 backup_settings,
+                read_settings,
                 make_temporary_hard_links,
                 backup_entries_from_part,
                 &temp_dirs);
diff --git a/src/Storages/MergeTree/MergeTreeData.h b/src/Storages/MergeTree/MergeTreeData.h
@@ -1336,7 +1336,7 @@ class MergeTreeData : public IStorage, public WithMutableContext
     using PartsBackupEntries = std::vector<PartBackupEntries>;
 
     /// Makes backup entries to backup the parts of this table.
-    PartsBackupEntries backupParts(const DataPartsVector & data_parts, const String & data_path_in_backup, const BackupSettings & backup_settings, const ContextPtr & local_context);
+    PartsBackupEntries backupParts(const DataPartsVector & data_parts, const String & data_path_in_backup, const BackupSettings & backup_settings, const ReadSettings & read_settings, const ContextPtr & local_context);
 
     class RestoredPartsHolder;
 
diff --git a/src/Storages/StorageLog.cpp b/src/Storages/StorageLog.cpp
@@ -949,6 +949,7 @@ void StorageLog::backupData(BackupEntriesCollector & backup_entries_collector, c
     fs::path temp_dir = temp_dir_owner->getRelativePath();
     disk->createDirectories(temp_dir);
 
+    const auto & read_settings = backup_entries_collector.getReadSettings();
     bool copy_encrypted = !backup_entries_collector.getBackupSettings().decrypt_files_from_encrypted_disks;
 
     /// *.bin
@@ -980,7 +981,7 @@ void StorageLog::backupData(BackupEntriesCollector & backup_entries_collector, c
     /// sizes.json
     String files_info_path = file_checker.getPath();
     backup_entries_collector.addBackupEntry(
-        data_path_in_backup_fs / fileName(files_info_path), std::make_unique<BackupEntryFromSmallFile>(disk, files_info_path, copy_encrypted));
+        data_path_in_backup_fs / fileName(files_info_path), std::make_unique<BackupEntryFromSmallFile>(disk, files_info_path, read_settings, copy_encrypted));
 
     /// columns.txt
     backup_entries_collector.addBackupEntry(
diff --git a/src/Storages/StorageMemory.cpp b/src/Storages/StorageMemory.cpp
@@ -277,11 +277,13 @@ namespace
             const std::shared_ptr<const Blocks> blocks_,
             const String & data_path_in_backup,
             const DiskPtr & temp_disk_,
+            const ReadSettings & read_settings_,
             UInt64 max_compress_block_size_)
             : context(context_)
             , metadata_snapshot(metadata_snapshot_)
             , blocks(blocks_)
             , temp_disk(temp_disk_)
+            , read_settings(read_settings_)
             , max_compress_block_size(max_compress_block_size_)
         {
             fs::path data_path_in_backup_fs = data_path_in_backup;
@@ -371,7 +373,7 @@ namespace
                     file_checker.update(temp_dir / fs::path{file_paths[i]}.filename());
                 }
                 file_checker.save();
-                backup_entries[sizes_json_pos] = {file_paths[sizes_json_pos], std::make_shared<BackupEntryFromSmallFile>(temp_disk, sizes_json_path)};
+                backup_entries[sizes_json_pos] = {file_paths[sizes_json_pos], std::make_shared<BackupEntryFromSmallFile>(temp_disk, sizes_json_path, read_settings)};
             }
 
             /// We don't need to keep `blocks` any longer.
@@ -386,6 +388,7 @@ namespace
         std::shared_ptr<const Blocks> blocks;
         DiskPtr temp_disk;
         std::optional<TemporaryFileOnDisk> temp_dir_owner;
+        ReadSettings read_settings;
         UInt64 max_compress_block_size;
         Strings file_paths;
         size_t data_bin_pos, index_mrk_pos, columns_txt_pos, count_txt_pos, sizes_json_pos;
@@ -395,13 +398,16 @@ namespace
 void StorageMemory::backupData(BackupEntriesCollector & backup_entries_collector, const String & data_path_in_backup, const std::optional<ASTs> & /* partitions */)
 {
     auto temp_disk = backup_entries_collector.getContext()->getGlobalTemporaryVolume()->getDisk(0);
+    const auto & read_settings = backup_entries_collector.getReadSettings();
     auto max_compress_block_size = backup_entries_collector.getContext()->getSettingsRef().max_compress_block_size;
+
     backup_entries_collector.addBackupEntries(std::make_shared<MemoryBackup>(
         backup_entries_collector.getContext(),
         getInMemoryMetadataPtr(),
         data.get(),
         data_path_in_backup,
         temp_disk,
+        read_settings,
         max_compress_block_size)->getBackupEntries());
 }
 
diff --git a/src/Storages/StorageMergeTree.cpp b/src/Storages/StorageMergeTree.cpp
@@ -2253,6 +2253,7 @@ CheckResults StorageMergeTree::checkData(const ASTPtr & query, ContextPtr local_
 void StorageMergeTree::backupData(BackupEntriesCollector & backup_entries_collector, const String & data_path_in_backup, const std::optional<ASTs> & partitions)
 {
     const auto & backup_settings = backup_entries_collector.getBackupSettings();
+    const auto & read_settings = backup_entries_collector.getReadSettings();
     auto local_context = backup_entries_collector.getContext();
 
     DataPartsVector data_parts;
@@ -2265,7 +2266,7 @@ void StorageMergeTree::backupData(BackupEntriesCollector & backup_entries_collec
     for (const auto & data_part : data_parts)
         min_data_version = std::min(min_data_version, data_part->info.getDataVersion() + 1);
 
-    auto parts_backup_entries = backupParts(data_parts, data_path_in_backup, backup_settings, local_context);
+    auto parts_backup_entries = backupParts(data_parts, data_path_in_backup, backup_settings, read_settings, local_context);
     for (auto & part_backup_entries : parts_backup_entries)
         backup_entries_collector.addBackupEntries(std::move(part_backup_entries.backup_entries));
 
diff --git a/src/Storages/StorageReplicatedMergeTree.cpp b/src/Storages/StorageReplicatedMergeTree.cpp
@@ -9826,6 +9826,7 @@ void StorageReplicatedMergeTree::backupData(
     /// because we need to coordinate them with other replicas (other replicas can have better parts).
 
     const auto & backup_settings = backup_entries_collector.getBackupSettings();
+    const auto & read_settings = backup_entries_collector.getReadSettings();
     auto local_context = backup_entries_collector.getContext();
 
     DataPartsVector data_parts;
@@ -9834,7 +9835,7 @@ void StorageReplicatedMergeTree::backupData(
     else
         data_parts = getVisibleDataPartsVector(local_context);
 
-    auto parts_backup_entries = backupParts(data_parts, /* data_path_in_backup */ "", backup_settings, local_context);
+    auto parts_backup_entries = backupParts(data_parts, /* data_path_in_backup */ "", backup_settings, read_settings, local_context);
 
     auto coordination = backup_entries_collector.getBackupCoordination();
     String shared_id = getTableSharedID();
diff --git a/src/Storages/StorageStripeLog.cpp b/src/Storages/StorageStripeLog.cpp
@@ -547,6 +547,7 @@ void StorageStripeLog::backupData(BackupEntriesCollector & backup_entries_collec
     fs::path temp_dir = temp_dir_owner->getRelativePath();
     disk->createDirectories(temp_dir);
 
+    const auto & read_settings = backup_entries_collector.getReadSettings();
     bool copy_encrypted = !backup_entries_collector.getBackupSettings().decrypt_files_from_encrypted_disks;
 
     /// data.bin
@@ -576,7 +577,7 @@ void StorageStripeLog::backupData(BackupEntriesCollector & backup_entries_collec
     /// sizes.json
     String files_info_path = file_checker.getPath();
     backup_entries_collector.addBackupEntry(
-        data_path_in_backup_fs / fileName(files_info_path), std::make_unique<BackupEntryFromSmallFile>(disk, files_info_path, copy_encrypted));
+        data_path_in_backup_fs / fileName(files_info_path), std::make_unique<BackupEntryFromSmallFile>(disk, files_info_path, read_settings, copy_encrypted));
 
     /// columns.txt
     backup_entries_collector.addBackupEntry(

Original file line number	Diff line number	Diff line change
`@@ -11,37 +11,37 @@ namespace DB`
`11`	`11`	`{`
`12`	`12`	`namespace`
`13`	`13`	`{`
`14`		`- String readFile(const String & file_path)`
	`14`	`+ String readFile(const String & file_path, const ReadSettings & read_settings)`
`15`	`15`	`{`
`16`		`- auto buf = createReadBufferFromFileBase(file_path, /* settings= */ {});`
	`16`	`+ auto buf = createReadBufferFromFileBase(file_path, read_settings);`
`17`	`17`	`String s;`
`18`	`18`	`readStringUntilEOF(s, *buf);`
`19`	`19`	`return s;`
`20`	`20`	`}`
`21`	`21`
`22`		`- String readFile(const DiskPtr & disk, const String & file_path, bool copy_encrypted)`
	`22`	`+ String readFile(const DiskPtr & disk, const String & file_path, const ReadSettings & read_settings, bool copy_encrypted)`
`23`	`23`	`{`
`24`		`- auto buf = copy_encrypted ? disk->readEncryptedFile(file_path, {}) : disk->readFile(file_path);`
	`24`	`+ auto buf = copy_encrypted ? disk->readEncryptedFile(file_path, read_settings) : disk->readFile(file_path, read_settings);`
`25`	`25`	`String s;`
`26`	`26`	`readStringUntilEOF(s, *buf);`
`27`	`27`	`return s;`
`28`	`28`	`}`
`29`	`29`	`}`
`30`	`30`
`31`	`31`
`32`		`-BackupEntryFromSmallFile::BackupEntryFromSmallFile(const String & file_path_)`
	`32`	`+BackupEntryFromSmallFile::BackupEntryFromSmallFile(const String & file_path_, const ReadSettings & read_settings_)`
`33`	`33`	`: file_path(file_path_)`
`34`	`34`	`, data_source_description(DiskLocal::getLocalDataSourceDescription(file_path_))`
`35`		`- , data(readFile(file_path_))`
	`35`	`+ , data(readFile(file_path_, read_settings_))`
`36`	`36`	`{`
`37`	`37`	`}`
`38`	`38`
`39`		`-BackupEntryFromSmallFile::BackupEntryFromSmallFile(const DiskPtr & disk_, const String & file_path_, bool copy_encrypted_)`
	`39`	`+BackupEntryFromSmallFile::BackupEntryFromSmallFile(const DiskPtr & disk_, const String & file_path_, const ReadSettings & read_settings_, bool copy_encrypted_)`
`40`	`40`	`: disk(disk_)`
`41`	`41`	`, file_path(file_path_)`
`42`	`42`	`, data_source_description(disk_->getDataSourceDescription())`
`43`	`43`	`, copy_encrypted(copy_encrypted_ && data_source_description.is_encrypted)`
`44`		`- , data(readFile(disk_, file_path, copy_encrypted))`
	`44`	`+ , data(readFile(disk_, file_path, read_settings_, copy_encrypted))`
`45`	`45`	`{`
`46`	`46`	`}`
`47`	`47`
Original file line number	Diff line number	Diff line change
`@@ -186,7 +186,7 @@ namespace`
`186`	`186`	`auto read_settings = context->getReadSettings();`
`187`	`187`	`read_settings.remote_throttler = context->getBackupsThrottler();`
`188`	`188`	`read_settings.local_throttler = context->getBackupsThrottler();`
`189`		`- read_settings.enable_filesystem_cache = false;`
	`189`	`+ read_settings.enable_filesystem_cache = backup_settings.read_from_filesystem_cache_if_exists_otherwise_bypass_cache;`
`190`	`190`	`read_settings.read_from_filesystem_cache_if_exists_otherwise_bypass_cache = backup_settings.read_from_filesystem_cache_if_exists_otherwise_bypass_cache;`
`191`	`191`	`return read_settings;`
`192`	`192`	`}`
`@@ -416,7 +416,7 @@ void BackupsWorker::doBackup(`
`416`	`416`	`/// Prepare backup entries.`
`417`	`417`	`BackupEntries backup_entries;`
`418`	`418`	`{`
`419`		`- BackupEntriesCollector backup_entries_collector{backup_query->elements, backup_settings, backup_coordination, context};`
	`419`	`+ BackupEntriesCollector backup_entries_collector{backup_query->elements, backup_settings, backup_coordination, backup_create_params.read_settings, context};`
`420`	`420`	`backup_entries = backup_entries_collector.run();`
`421`	`421`	`}`
`422`	`422`
Original file line number	Diff line number	Diff line change
`@@ -331,6 +331,7 @@ void DataPartStorageOnDiskBase::backup(`
`331`	`331`	`const NameSet & files_without_checksums,`
`332`	`332`	`const String & path_in_backup,`
`333`	`333`	`const BackupSettings & backup_settings,`
	`334`	`+ const ReadSettings & read_settings,`
`334`	`335`	`bool make_temporary_hard_links,`
`335`	`336`	`BackupEntries & backup_entries,`
`336`	`337`	`TemporaryFilesOnDisks * temp_dirs) const`
`@@ -382,7 +383,7 @@ void DataPartStorageOnDiskBase::backup(`
`382`	`383`
`383`	`384`	`if (files_without_checksums.contains(filepath))`
`384`	`385`	`{`
`385`		`- backup_entries.emplace_back(filepath_in_backup, std::make_unique<BackupEntryFromSmallFile>(disk, filepath_on_disk, copy_encrypted));`
	`386`	`+ backup_entries.emplace_back(filepath_in_backup, std::make_unique<BackupEntryFromSmallFile>(disk, filepath_on_disk, read_settings, copy_encrypted));`
`386`	`387`	`continue;`
`387`	`388`	`}`
`388`	`389`