From cc50fcc60a4102b194f30b56896d045390833d8d Mon Sep 17 00:00:00 2001 From: Vitaly Baranov Date: Sun, 23 Apr 2023 12:25:46 +0200 Subject: [PATCH] Remove the 'temporary_file_' argument from BackupEntryFromImmutableFile's constructor. --- src/Backups/BackupEntryFromAppendOnlyFile.cpp | 5 ++--- src/Backups/BackupEntryFromAppendOnlyFile.h | 3 +-- src/Backups/BackupEntryFromImmutableFile.cpp | 4 +--- src/Backups/BackupEntryFromImmutableFile.h | 5 +---- src/Backups/BackupEntryWrappedWith.h | 8 +++++++- .../MergeTree/DataPartStorageOnDiskBase.cpp | 10 +++++++--- src/Storages/StorageLog.cpp | 17 +++++++++-------- src/Storages/StorageStripeLog.cpp | 17 +++++++++-------- 8 files changed, 37 insertions(+), 32 deletions(-) diff --git a/src/Backups/BackupEntryFromAppendOnlyFile.cpp b/src/Backups/BackupEntryFromAppendOnlyFile.cpp index 5384a69d890..83117f686bf 100644 --- a/src/Backups/BackupEntryFromAppendOnlyFile.cpp +++ b/src/Backups/BackupEntryFromAppendOnlyFile.cpp @@ -10,9 +10,8 @@ BackupEntryFromAppendOnlyFile::BackupEntryFromAppendOnlyFile( const String & file_path_, const ReadSettings & settings_, const std::optional & file_size_, - const std::optional & checksum_, - const std::shared_ptr & temporary_file_) - : BackupEntryFromImmutableFile(disk_, file_path_, settings_, file_size_, checksum_, temporary_file_) + const std::optional & checksum_) + : BackupEntryFromImmutableFile(disk_, file_path_, settings_, file_size_, checksum_) , limit(BackupEntryFromImmutableFile::getSize()) { } diff --git a/src/Backups/BackupEntryFromAppendOnlyFile.h b/src/Backups/BackupEntryFromAppendOnlyFile.h index b0cee38c6be..b7a39c935a9 100644 --- a/src/Backups/BackupEntryFromAppendOnlyFile.h +++ b/src/Backups/BackupEntryFromAppendOnlyFile.h @@ -18,8 +18,7 @@ public: const String & file_path_, const ReadSettings & settings_, const std::optional & file_size_ = {}, - const std::optional & checksum_ = {}, - const std::shared_ptr & temporary_file_ = {}); + const std::optional & checksum_ = {}); UInt64 getSize() const override { return limit; } std::unique_ptr getReadBuffer() const override; diff --git a/src/Backups/BackupEntryFromImmutableFile.cpp b/src/Backups/BackupEntryFromImmutableFile.cpp index 48783a3bb63..790ea567496 100644 --- a/src/Backups/BackupEntryFromImmutableFile.cpp +++ b/src/Backups/BackupEntryFromImmutableFile.cpp @@ -13,14 +13,12 @@ BackupEntryFromImmutableFile::BackupEntryFromImmutableFile( const String & file_path_, const ReadSettings & settings_, const std::optional & file_size_, - const std::optional & checksum_, - const std::shared_ptr & temporary_file_) + const std::optional & checksum_) : disk(disk_) , file_path(file_path_) , settings(settings_) , file_size(file_size_) , checksum(checksum_) - , temporary_file_on_disk(temporary_file_) { } diff --git a/src/Backups/BackupEntryFromImmutableFile.h b/src/Backups/BackupEntryFromImmutableFile.h index 66f1fade294..4f2f902d31e 100644 --- a/src/Backups/BackupEntryFromImmutableFile.h +++ b/src/Backups/BackupEntryFromImmutableFile.h @@ -7,7 +7,6 @@ namespace DB { -class TemporaryFileOnDisk; class IDisk; using DiskPtr = std::shared_ptr; @@ -22,8 +21,7 @@ public: const String & file_path_, const ReadSettings & settings_, const std::optional & file_size_ = {}, - const std::optional & checksum_ = {}, - const std::shared_ptr & temporary_file_ = {}); + const std::optional & checksum_ = {}); ~BackupEntryFromImmutableFile() override; @@ -43,7 +41,6 @@ private: mutable std::optional file_size TSA_GUARDED_BY(get_file_size_mutex); mutable std::mutex get_file_size_mutex; const std::optional checksum; - const std::shared_ptr temporary_file_on_disk; }; } diff --git a/src/Backups/BackupEntryWrappedWith.h b/src/Backups/BackupEntryWrappedWith.h index 97244650b6b..da3b70e9ba9 100644 --- a/src/Backups/BackupEntryWrappedWith.h +++ b/src/Backups/BackupEntryWrappedWith.h @@ -27,11 +27,17 @@ private: T custom_value; }; +template +BackupEntryPtr wrapBackupEntryWith(BackupEntryPtr && backup_entry, const T & custom_value) +{ + return std::make_shared>(std::move(backup_entry), custom_value); +} + template void wrapBackupEntriesWith(std::vector> & backup_entries, const T & custom_value) { for (auto & [_, backup_entry] : backup_entries) - backup_entry = std::make_shared>(std::move(backup_entry), custom_value); + backup_entry = wrapBackupEntryWith(std::move(backup_entry), custom_value); } } diff --git a/src/Storages/MergeTree/DataPartStorageOnDiskBase.cpp b/src/Storages/MergeTree/DataPartStorageOnDiskBase.cpp index ec00cc3d2b9..4df490f41fe 100644 --- a/src/Storages/MergeTree/DataPartStorageOnDiskBase.cpp +++ b/src/Storages/MergeTree/DataPartStorageOnDiskBase.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include @@ -392,9 +393,12 @@ void DataPartStorageOnDiskBase::backup( file_hash = {it->second.file_hash.first, it->second.file_hash.second}; } - backup_entries.emplace_back( - filepath_in_backup, - std::make_unique(disk, filepath_on_disk, read_settings, file_size, file_hash, temp_dir_owner)); + BackupEntryPtr backup_entry = std::make_unique(disk, filepath_on_disk, read_settings, file_size, file_hash); + + if (temp_dir_owner) + backup_entry = wrapBackupEntryWith(std::move(backup_entry), temp_dir_owner); + + backup_entries.emplace_back(filepath_in_backup, std::move(backup_entry)); } } diff --git a/src/Storages/StorageLog.cpp b/src/Storages/StorageLog.cpp index 8264d67aaba..19887d6695e 100644 --- a/src/Storages/StorageLog.cpp +++ b/src/Storages/StorageLog.cpp @@ -28,6 +28,7 @@ #include #include #include +#include #include #include #include @@ -951,10 +952,10 @@ void StorageLog::backupData(BackupEntriesCollector & backup_entries_collector, c String data_file_name = fileName(data_file.path); String hardlink_file_path = temp_dir / data_file_name; disk->createHardLink(data_file.path, hardlink_file_path); - backup_entries_collector.addBackupEntry( - data_path_in_backup_fs / data_file_name, - std::make_unique( - disk, hardlink_file_path, read_settings, file_checker.getFileSize(data_file.path), std::nullopt, temp_dir_owner)); + BackupEntryPtr backup_entry = std::make_unique( + disk, hardlink_file_path, read_settings, file_checker.getFileSize(data_file.path)); + backup_entry = wrapBackupEntryWith(std::move(backup_entry), temp_dir_owner); + backup_entries_collector.addBackupEntry(data_path_in_backup_fs / data_file_name, std::move(backup_entry)); } /// __marks.mrk @@ -964,10 +965,10 @@ void StorageLog::backupData(BackupEntriesCollector & backup_entries_collector, c String marks_file_name = fileName(marks_file_path); String hardlink_file_path = temp_dir / marks_file_name; disk->createHardLink(marks_file_path, hardlink_file_path); - backup_entries_collector.addBackupEntry( - data_path_in_backup_fs / marks_file_name, - std::make_unique( - disk, hardlink_file_path, read_settings, file_checker.getFileSize(marks_file_path), std::nullopt, temp_dir_owner)); + BackupEntryPtr backup_entry = std::make_unique( + disk, hardlink_file_path, read_settings, file_checker.getFileSize(marks_file_path)); + backup_entry = wrapBackupEntryWith(std::move(backup_entry), temp_dir_owner); + backup_entries_collector.addBackupEntry(data_path_in_backup_fs / marks_file_name, std::move(backup_entry)); } /// sizes.json diff --git a/src/Storages/StorageStripeLog.cpp b/src/Storages/StorageStripeLog.cpp index d54725b8b39..ddb55c119c4 100644 --- a/src/Storages/StorageStripeLog.cpp +++ b/src/Storages/StorageStripeLog.cpp @@ -32,6 +32,7 @@ #include #include #include +#include #include #include #include @@ -551,10 +552,10 @@ void StorageStripeLog::backupData(BackupEntriesCollector & backup_entries_collec String data_file_name = fileName(data_file_path); String hardlink_file_path = temp_dir / data_file_name; disk->createHardLink(data_file_path, hardlink_file_path); - backup_entries_collector.addBackupEntry( - data_path_in_backup_fs / data_file_name, - std::make_unique( - disk, hardlink_file_path, read_settings, file_checker.getFileSize(data_file_path), std::nullopt, temp_dir_owner)); + BackupEntryPtr backup_entry = std::make_unique( + disk, hardlink_file_path, read_settings, file_checker.getFileSize(data_file_path)); + backup_entry = wrapBackupEntryWith(std::move(backup_entry), temp_dir_owner); + backup_entries_collector.addBackupEntry(data_path_in_backup_fs / data_file_name, std::move(backup_entry)); } /// index.mrk @@ -563,10 +564,10 @@ void StorageStripeLog::backupData(BackupEntriesCollector & backup_entries_collec String index_file_name = fileName(index_file_path); String hardlink_file_path = temp_dir / index_file_name; disk->createHardLink(index_file_path, hardlink_file_path); - backup_entries_collector.addBackupEntry( - data_path_in_backup_fs / index_file_name, - std::make_unique( - disk, hardlink_file_path, read_settings, file_checker.getFileSize(index_file_path), std::nullopt, temp_dir_owner)); + BackupEntryPtr backup_entry = std::make_unique( + disk, hardlink_file_path, read_settings, file_checker.getFileSize(index_file_path)); + backup_entry = wrapBackupEntryWith(std::move(backup_entry), temp_dir_owner); + backup_entries_collector.addBackupEntry(data_path_in_backup_fs / index_file_name, std::move(backup_entry)); } /// sizes.json