From c32b6076fba7ee91c613c8a36c5a2037a95ede94 Mon Sep 17 00:00:00 2001 From: alesapin Date: Mon, 30 May 2022 00:12:33 +0200 Subject: [PATCH] Remove stranges from code --- .../ObjectStorages/DiskObjectStorage.cpp | 7 +- .../DiskObjectStorageMetadata.cpp | 97 ++++++++----------- src/Storages/MergeTree/MergeTreeData.cpp | 2 +- 3 files changed, 48 insertions(+), 58 deletions(-) diff --git a/src/Disks/ObjectStorages/DiskObjectStorage.cpp b/src/Disks/ObjectStorages/DiskObjectStorage.cpp index 65b1d5a5bdf..8a5ebea29ec 100644 --- a/src/Disks/ObjectStorages/DiskObjectStorage.cpp +++ b/src/Disks/ObjectStorages/DiskObjectStorage.cpp @@ -237,7 +237,10 @@ void DiskObjectStorage::moveFile(const String & from_path, const String & to_pat metadata_helper->createFileOperationObject("rename", revision, object_metadata); } - metadata_disk->moveFile(from_path, to_path); + { + std::unique_lock lock(metadata_mutex); + metadata_disk->moveFile(from_path, to_path); + } } void DiskObjectStorage::moveFile(const String & from_path, const String & to_path) @@ -449,6 +452,8 @@ void DiskObjectStorage::removeMetadata(const String & path, std::vector LOG_WARNING(log, "Metadata file {} can't be read by reason: {}. Removing it forcibly.", backQuote(path), e.nested() ? e.nested()->message() : e.message()); + + std::unique_lock lock(metadata_mutex); metadata_disk->removeFile(path); } else diff --git a/src/Disks/ObjectStorages/DiskObjectStorageMetadata.cpp b/src/Disks/ObjectStorages/DiskObjectStorageMetadata.cpp index 2e1ef31f8f0..39d3fa07134 100644 --- a/src/Disks/ObjectStorages/DiskObjectStorageMetadata.cpp +++ b/src/Disks/ObjectStorages/DiskObjectStorageMetadata.cpp @@ -74,70 +74,55 @@ DiskObjectStorageMetadata DiskObjectStorageMetadata::createAndStoreMetadataIfNot void DiskObjectStorageMetadata::load() { - try + const ReadSettings read_settings; + auto buf = metadata_disk->readFile(metadata_file_path, read_settings, 1024); /* reasonable buffer size for small file */ + + UInt32 version; + readIntText(version, *buf); + + if (version < VERSION_ABSOLUTE_PATHS || version > VERSION_READ_ONLY_FLAG) + throw Exception( + ErrorCodes::UNKNOWN_FORMAT, + "Unknown metadata file version. Path: {}. Version: {}. Maximum expected version: {}", + metadata_disk->getPath() + metadata_file_path, toString(version), toString(VERSION_READ_ONLY_FLAG)); + + assertChar('\n', *buf); + + UInt32 remote_fs_objects_count; + readIntText(remote_fs_objects_count, *buf); + assertChar('\t', *buf); + readIntText(total_size, *buf); + assertChar('\n', *buf); + remote_fs_objects.resize(remote_fs_objects_count); + + for (size_t i = 0; i < remote_fs_objects_count; ++i) { - const ReadSettings read_settings; - auto buf = metadata_disk->readFile(metadata_file_path, read_settings, 1024); /* reasonable buffer size for small file */ - - UInt32 version; - readIntText(version, *buf); - - if (version < VERSION_ABSOLUTE_PATHS || version > VERSION_READ_ONLY_FLAG) - throw Exception( - ErrorCodes::UNKNOWN_FORMAT, - "Unknown metadata file version. Path: {}. Version: {}. Maximum expected version: {}", - metadata_disk->getPath() + metadata_file_path, toString(version), toString(VERSION_READ_ONLY_FLAG)); - - assertChar('\n', *buf); - - UInt32 remote_fs_objects_count; - readIntText(remote_fs_objects_count, *buf); + String remote_fs_object_path; + size_t remote_fs_object_size; + readIntText(remote_fs_object_size, *buf); assertChar('\t', *buf); - readIntText(total_size, *buf); - assertChar('\n', *buf); - remote_fs_objects.resize(remote_fs_objects_count); - - for (size_t i = 0; i < remote_fs_objects_count; ++i) + readEscapedString(remote_fs_object_path, *buf); + if (version == VERSION_ABSOLUTE_PATHS) { - String remote_fs_object_path; - size_t remote_fs_object_size; - readIntText(remote_fs_object_size, *buf); - assertChar('\t', *buf); - readEscapedString(remote_fs_object_path, *buf); - if (version == VERSION_ABSOLUTE_PATHS) - { - if (!remote_fs_object_path.starts_with(remote_fs_root_path)) - throw Exception(ErrorCodes::UNKNOWN_FORMAT, - "Path in metadata does not correspond to root path. Path: {}, root path: {}, disk path: {}", - remote_fs_object_path, remote_fs_root_path, metadata_disk->getPath()); + if (!remote_fs_object_path.starts_with(remote_fs_root_path)) + throw Exception(ErrorCodes::UNKNOWN_FORMAT, + "Path in metadata does not correspond to root path. Path: {}, root path: {}, disk path: {}", + remote_fs_object_path, remote_fs_root_path, metadata_disk->getPath()); - remote_fs_object_path = remote_fs_object_path.substr(remote_fs_root_path.size()); - } - assertChar('\n', *buf); - remote_fs_objects[i].relative_path = remote_fs_object_path; - remote_fs_objects[i].bytes_size = remote_fs_object_size; + remote_fs_object_path = remote_fs_object_path.substr(remote_fs_root_path.size()); } - - readIntText(ref_count, *buf); assertChar('\n', *buf); - - if (version >= VERSION_READ_ONLY_FLAG) - { - readBoolText(read_only, *buf); - assertChar('\n', *buf); - } + remote_fs_objects[i].relative_path = remote_fs_object_path; + remote_fs_objects[i].bytes_size = remote_fs_object_size; } - catch (Exception & e) + + readIntText(ref_count, *buf); + assertChar('\n', *buf); + + if (version >= VERSION_READ_ONLY_FLAG) { - tryLogCurrentException(__PRETTY_FUNCTION__); - - if (e.code() == ErrorCodes::UNKNOWN_FORMAT) - throw; - - if (e.code() == ErrorCodes::MEMORY_LIMIT_EXCEEDED) - throw; - - throw Exception("Failed to read metadata file: " + metadata_file_path, ErrorCodes::UNKNOWN_FORMAT); + readBoolText(read_only, *buf); + assertChar('\n', *buf); } } diff --git a/src/Storages/MergeTree/MergeTreeData.cpp b/src/Storages/MergeTree/MergeTreeData.cpp index 16699260ea0..7623ff1976f 100644 --- a/src/Storages/MergeTree/MergeTreeData.cpp +++ b/src/Storages/MergeTree/MergeTreeData.cpp @@ -400,7 +400,7 @@ static void checkKeyExpression(const ExpressionActions & expr, const Block & sam if (!allow_nullable_key && hasNullable(element.type)) throw Exception( - ErrorCodes::ILLEGAL_COLUMN, "{} key contains nullable columns, but `setting allow_nullable_key` is disabled", key_name); + ErrorCodes::ILLEGAL_COLUMN, "{} key contains nullable columns, but merge tree setting `allow_nullable_key` is disabled", key_name); } }