From 58d0335d9c8c74151e00b7b938e697ad059e6bd8 Mon Sep 17 00:00:00 2001 From: "Chen, Qi" Date: Thu, 22 Jun 2023 20:24:51 +0100 Subject: [PATCH] #447 Use PermissionException in more places --- cpp/arcticdb/storage/library.hpp | 1 - cpp/arcticdb/storage/library_manager.hpp | 1 - cpp/arcticdb/storage/s3/s3_storage-inl.hpp | 76 +++++++++------------ cpp/arcticdb/storage/storage_exceptions.hpp | 37 ---------- cpp/arcticdb/version/symbol_list.hpp | 1 - 5 files changed, 32 insertions(+), 84 deletions(-) delete mode 100644 cpp/arcticdb/storage/storage_exceptions.hpp diff --git a/cpp/arcticdb/storage/library.hpp b/cpp/arcticdb/storage/library.hpp index 48ca2bae8ec..d4c690f69bd 100644 --- a/cpp/arcticdb/storage/library.hpp +++ b/cpp/arcticdb/storage/library.hpp @@ -12,7 +12,6 @@ #include #include #include -#include #include #include #include diff --git a/cpp/arcticdb/storage/library_manager.hpp b/cpp/arcticdb/storage/library_manager.hpp index f13e0eb80c2..d8f65f296ec 100644 --- a/cpp/arcticdb/storage/library_manager.hpp +++ b/cpp/arcticdb/storage/library_manager.hpp @@ -13,7 +13,6 @@ #include #include #include -#include #include #include #include diff --git a/cpp/arcticdb/storage/s3/s3_storage-inl.hpp b/cpp/arcticdb/storage/s3/s3_storage-inl.hpp index 4a3eaafcaab..4fa12f0c7a0 100644 --- a/cpp/arcticdb/storage/s3/s3_storage-inl.hpp +++ b/cpp/arcticdb/storage/s3/s3_storage-inl.hpp @@ -49,6 +49,32 @@ static const size_t DELETE_OBJECTS_LIMIT = 1000; template using Range = folly::Range; +template +void handle_common_errors(const Aws::Utils::Outcome& outcome, + const char* op, + const KeyType& key) { + using namespace Aws::S3; + if (!outcome.IsSuccess()) { + auto& err = outcome.GetError(); + auto type = err.GetErrorType(); + if (type == S3Errors::NO_SUCH_KEY || type == S3Errors::RESOURCE_NOT_FOUND) { + return; // Caller need to handle these + } + +#define S3_ERROR_FMT " error while {} {}: S3Error#{} {}: {}", \ + op, key, int(err.GetErrorType()), err.GetExceptionName().c_str(), err.GetMessage().c_str() + + if (type == S3Errors::ACCESS_DENIED || type == S3Errors::INVALID_ACCESS_KEY_ID) { + raise("Permission" S3_ERROR_FMT); + } + if (err.ShouldRetry()) { + raise_retryable("Retry-able" S3_ERROR_FMT); + } + raise("Unexpected" S3_ERROR_FMT); +#undef S3_ERROR_FMT + } +} + struct FlatBucketizer { static std::string bucketize(const std::string& root_folder, const VariantKey&) { return root_folder; @@ -102,14 +128,8 @@ void do_write_impl( ARCTICDB_SUBSAMPLE(S3StoragePutObject, 0) auto put_object_outcome = s3_client.PutObject(object_request); - if (!put_object_outcome.IsSuccess()) { - auto& error = put_object_outcome.GetError(); - util::raise_rte("Failed to write s3 with key '{}' {}: {}", - k, - error.GetExceptionName().c_str(), - error.GetMessage().c_str()); - } - ARCTICDB_RUNTIME_DEBUG(log::storage(), "Wrote key {}: {}, with {} bytes of data", + handle_common_errors(put_object_outcome, "writing key", k); + ARCTICDB_RUNTIME_DEBUG(log::storage(), "Wrote key {}: {}, with {} bytes of data", variant_key_type(k), variant_key_view(k), seg.total_segment_size(hdr_size)); @@ -128,16 +148,6 @@ void do_update_impl( do_write_impl(std::move(kvs), root_folder, bucket_name, s3_client, std::forward(bucketizer)); } -struct UnexpectedS3ErrorException : public std::exception {}; - -inline bool is_expected_error_type(Aws::S3::S3Errors err) { - return err == Aws::S3::S3Errors::NO_SUCH_KEY - || err == Aws::S3::S3Errors::NO_SUCH_BUCKET - || err == Aws::S3::S3Errors::INVALID_ACCESS_KEY_ID - || err == Aws::S3::S3Errors::ACCESS_DENIED - || err == Aws::S3::S3Errors::RESOURCE_NOT_FOUND; -} - //TODO Use buffer pool once memory profile and lifetime is well understood struct S3StreamBuffer : public std::streambuf { ARCTICDB_NO_MOVE_OR_COPY(S3StreamBuffer) @@ -196,14 +206,7 @@ auto get_object( request.SetResponseStreamFactory(S3StreamFactory()); auto res = s3_client.GetObject(request); - if (!res.IsSuccess() && !is_expected_error_type(res.GetError().GetErrorType())) { - log::storage().error("Got unexpected error: '{}' {}: {}", - int(res.GetError().GetErrorType()), - res.GetError().GetExceptionName().c_str(), - res.GetError().GetMessage().c_str()); - - throw UnexpectedS3ErrorException{}; - } + handle_common_errors(res, "getting key", key); ARCTICDB_RUNTIME_DEBUG(log::storage(), "Returning object {}", s3_object_name); return res; } @@ -223,14 +226,7 @@ auto head_object( request.WithBucket(bucket_name.c_str()).WithKey(s3_object_name.c_str()); auto res = s3_client.HeadObject(request); - if (!res.IsSuccess() && !is_expected_error_type(res.GetError().GetErrorType())) { - log::storage().error("Got unexpected error: '{}' {}: {}", - int(res.GetError().GetErrorType()), - res.GetError().GetExceptionName().c_str(), - res.GetError().GetMessage().c_str()); - - throw UnexpectedS3ErrorException{}; - } + handle_common_errors(res, "checking", key); ARCTICDB_RUNTIME_DEBUG(log::storage(), "Returning head of object {}", s3_object_name); return res; } @@ -269,8 +265,8 @@ void do_read_impl(Composite && ks, ARCTICDB_DEBUG(log::storage(), "Read key {}: {}", variant_key_type(k), variant_key_view(k)); } else { - auto& error = get_object_outcome.GetError(); if (!opts.dont_warn_about_missing_key) { + auto& error = get_object_outcome.GetError(); log::storage().warn("Failed to find segment for key '{}' {}: {}", variant_key_view(k), error.GetExceptionName().c_str(), @@ -370,6 +366,7 @@ void do_iterate_type_impl(KeyType key_type, bool more; do { auto list_objects_outcome = s3_client.ListObjectsV2(objects_request); + handle_common_errors(list_objects_outcome, "iterating keys of type ", key_type); if (list_objects_outcome.IsSuccess()) { auto object_list = list_objects_outcome.GetResult().GetContents(); @@ -392,15 +389,6 @@ void do_iterate_type_impl(KeyType key_type, more = list_objects_outcome.GetResult().GetIsTruncated(); if (more) objects_request.SetContinuationToken(list_objects_outcome.GetResult().GetNextContinuationToken()); - - } - else { - const auto& error = list_objects_outcome.GetError(); - log::storage().warn("Failed to iterate key type with key '{}' {}: {}", - key_type, - error.GetExceptionName().c_str(), - error.GetMessage().c_str()); - return; } } while (more); } diff --git a/cpp/arcticdb/storage/storage_exceptions.hpp b/cpp/arcticdb/storage/storage_exceptions.hpp deleted file mode 100644 index d04afe227ab..00000000000 --- a/cpp/arcticdb/storage/storage_exceptions.hpp +++ /dev/null @@ -1,37 +0,0 @@ -/* Copyright 2023 Man Group Operations Limited - * - * Use of this software is governed by the Business Source License 1.1 included in the file licenses/BSL.txt. - * - * As of the Change Date specified in that file, in accordance with the Business Source License, use of this software will be governed by the Apache License, version 2.0. - */ - -#pragma once - -#include -#include -#include - -namespace arcticdb::storage { - -class PermissionException : public std::exception { - public: - PermissionException(const LibraryPath &path, OpenMode mode, std::string_view operation) : - lib_path_(path), mode_(mode), msg_(fmt::format( - "{} not permitted. lib={}, mode={}", operation, lib_path_, mode_) - ) {} - - const char *what() const noexcept override { - return msg_.data(); - } - - const LibraryPath &library_path() const { - return lib_path_; - } - - private: - LibraryPath lib_path_; - OpenMode mode_; - std::string msg_; -}; - -} \ No newline at end of file diff --git a/cpp/arcticdb/version/symbol_list.hpp b/cpp/arcticdb/version/symbol_list.hpp index 1dcfec3607b..3625655077a 100644 --- a/cpp/arcticdb/version/symbol_list.hpp +++ b/cpp/arcticdb/version/symbol_list.hpp @@ -19,7 +19,6 @@ #include #include #include -#include #include #include