Merge pull request #38949 from vitlibar/careful-destructor-in-backupimpl

More careful destructor in BackupImpl
This commit is contained in:
Vitaly Baranov 2022-07-08 13:11:13 +02:00 committed by GitHub
commit 3af50b35ae
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 45 additions and 46 deletions

View File

@ -1,9 +1,7 @@
#include <Backups/BackupIO_Disk.h>
#include <Common/Exception.h>
#include <Disks/IDisk.h>
#include <IO/ReadBufferFromFileBase.h>
#include <IO/WriteBufferFromFileBase.h>
#include <Common/logger_useful.h>
namespace DB
@ -49,17 +47,10 @@ std::unique_ptr<WriteBuffer> BackupWriterDisk::writeFile(const String & file_nam
void BackupWriterDisk::removeFilesAfterFailure(const Strings & file_names)
{
try
{
for (const auto & file_name : file_names)
disk->removeFileIfExists(path / file_name);
if (disk->isDirectory(path) && disk->isDirectoryEmpty(path))
disk->removeDirectory(path);
}
catch (...)
{
LOG_WARNING(&Poco::Logger::get("BackupWriterDisk"), "RemoveFilesAfterFailure: {}", getCurrentExceptionMessage(false));
}
for (const auto & file_name : file_names)
disk->removeFileIfExists(path / file_name);
if (disk->isDirectory(path) && disk->isDirectoryEmpty(path))
disk->removeDirectory(path);
}
}

View File

@ -1,8 +1,6 @@
#include <Backups/BackupIO_File.h>
#include <Common/Exception.h>
#include <Disks/IO/createReadBufferFromFileBase.h>
#include <IO/WriteBufferFromFile.h>
#include <Common/logger_useful.h>
namespace fs = std::filesystem;
@ -50,17 +48,10 @@ std::unique_ptr<WriteBuffer> BackupWriterFile::writeFile(const String & file_nam
void BackupWriterFile::removeFilesAfterFailure(const Strings & file_names)
{
try
{
for (const auto & file_name : file_names)
fs::remove(path / file_name);
if (fs::is_directory(path) && fs::is_empty(path))
fs::remove(path);
}
catch (...)
{
LOG_WARNING(&Poco::Logger::get("BackupWriterFile"), "RemoveFilesAfterFailure: {}", getCurrentExceptionMessage(false));
}
for (const auto & file_name : file_names)
fs::remove(path / file_name);
if (fs::is_directory(path) && fs::is_empty(path))
fs::remove(path);
}
}

View File

@ -167,7 +167,14 @@ BackupImpl::BackupImpl(
BackupImpl::~BackupImpl()
{
close();
try
{
close();
}
catch (...)
{
DB::tryLogCurrentException(__PRETTY_FUNCTION__);
}
}
@ -231,10 +238,11 @@ void BackupImpl::close()
archive_writer = {"", nullptr};
if (!is_internal_backup && writer && !writing_finalized)
{
LOG_INFO(log, "Removing all files of backup {} after failure", backup_name);
removeAllFilesAfterFailure();
}
writer.reset();
reader.reset();
coordination.reset();
}
time_t BackupImpl::getTimestamp() const
@ -733,24 +741,33 @@ std::shared_ptr<IArchiveWriter> BackupImpl::getArchiveWriter(const String & suff
void BackupImpl::removeAllFilesAfterFailure()
{
Strings files_to_remove;
if (use_archives)
try
{
files_to_remove.push_back(archive_params.archive_name);
for (const auto & suffix : coordination->getAllArchiveSuffixes())
LOG_INFO(log, "Removing all files of backup {} after failure", backup_name);
Strings files_to_remove;
if (use_archives)
{
String archive_name_with_suffix = getArchiveNameWithSuffix(suffix);
files_to_remove.push_back(std::move(archive_name_with_suffix));
files_to_remove.push_back(archive_params.archive_name);
for (const auto & suffix : coordination->getAllArchiveSuffixes())
{
String archive_name_with_suffix = getArchiveNameWithSuffix(suffix);
files_to_remove.push_back(std::move(archive_name_with_suffix));
}
}
else
{
files_to_remove.push_back(".backup");
for (const auto & file_info : coordination->getAllFileInfos())
files_to_remove.push_back(file_info.data_file_name);
}
writer->removeFilesAfterFailure(files_to_remove);
}
else
catch (...)
{
files_to_remove.push_back(".backup");
for (const auto & file_info : coordination->getAllFileInfos())
files_to_remove.push_back(file_info.data_file_name);
DB::tryLogCurrentException(__PRETTY_FUNCTION__);
}
writer->removeFilesAfterFailure(files_to_remove);
}
}

View File

@ -100,10 +100,10 @@ UUID BackupsWorker::startMakingBackup(const ASTPtr & query, const ContextPtr & c
/// Make a backup coordination.
std::shared_ptr<IBackupCoordination> backup_coordination;
SCOPE_EXIT({
SCOPE_EXIT_SAFE(
if (backup_coordination && !backup_settings.internal)
backup_coordination->drop();
});
);
ClusterPtr cluster;
if (on_cluster)
@ -278,10 +278,10 @@ UUID BackupsWorker::startRestoring(const ASTPtr & query, ContextMutablePtr conte
/// Make a restore coordination.
std::shared_ptr<IRestoreCoordination> restore_coordination;
SCOPE_EXIT({
SCOPE_EXIT_SAFE(
if (restore_coordination && !restore_settings.internal)
restore_coordination->drop();
});
);
if (on_cluster && restore_settings.coordination_zk_path.empty())
{