Skip to content

Commit

Permalink
[feature](Azure) Consider delete non-existent file always successful …
Browse files Browse the repository at this point in the history
…on Azure (apache#36932)

When deleting non-existent files on object storage like S3, the result
would be considered as always true. This pr makes azure client acts the
same way.

As for the added compare string, you can refer to this
[doc](https://github.com/microsoft/api-guidelines/blob/vNext/azure/ConsiderationsForServiceDesign.md)


![image](https://github.com/apache/doris/assets/43750022/4f67d801-d743-4aaa-94c1-673f4800e86e)
  • Loading branch information
ByteYue authored Jun 28, 2024
1 parent e613b54 commit 81aa496
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 21 deletions.
34 changes: 24 additions & 10 deletions be/src/io/fs/azure_obj_storage_client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ auto base64_encode_part_num(int part_num) {
}

constexpr char SAS_TOKEN_URL_TEMPLATE[] = "https://{}.blob.core.windows.net/{}/{}{}";
constexpr char BlobNotFound[] = "BlobNotFound";
} // namespace

namespace doris::io {
Expand Down Expand Up @@ -105,17 +106,30 @@ struct AzureBatchDeleter {
return resp;
}

auto get_defer_response = [](const auto& defer) {
// DeferredResponse<Models::DeleteBlobResult> might throw exception
if (!defer.GetResponse().Value.Deleted) {
throw Exception(Status::IOError<false>("Batch delete failed"));
}
};
for (auto&& defer_response : deferred_resps) {
auto response =
do_azure_client_call([&]() { get_defer_response(defer_response); }, _opts);
if (response.status.code != ErrorCode::OK) {
return response;
try {
auto r = defer_response.GetResponse();
if (!r.Value.Deleted) {
auto msg = fmt::format("Azure batch delete failed, path msg {}",
wrap_object_storage_path_msg(_opts));
LOG_WARNING(msg);
return {.status = convert_to_obj_response(
Status::InternalError<false>(std::move(msg)))};
}
} catch (Azure::Core::RequestFailedException& e) {
if (Azure::Core::Http::HttpStatusCode::NotFound == e.StatusCode &&
0 == strcmp(e.ErrorCode.c_str(), BlobNotFound)) {
continue;
}
auto msg = fmt::format(
"Azure request failed because {}, error msg {}, http code {}, path msg {}",
e.what(), e.Message, static_cast<int>(e.StatusCode),
wrap_object_storage_path_msg(_opts));
LOG_WARNING(msg);
return {.status = convert_to_obj_response(
Status::InternalError<false>(std::move(msg))),
.http_code = static_cast<int>(e.StatusCode),
.request_id = std::move(e.RequestId)};
}
}

Expand Down
32 changes: 21 additions & 11 deletions cloud/src/recycler/azure_obj_client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ using namespace Azure::Storage::Blobs;
namespace doris::cloud {

constexpr size_t BlobBatchMaxOperations = 256;
constexpr char BlobNotFound[] = "BlobNotFound";

template <typename Func>
ObjectStorageResponse do_azure_client_call(Func f, const ObjectStoragePathOptions& opts) {
Expand Down Expand Up @@ -78,18 +79,27 @@ struct AzureBatchDeleter {
return resp;
}

auto get_defer_response = [](const auto& defer) {
// DeferredResponse<Models::DeleteBlobResult> might throw exception
if (!defer.GetResponse().Value.Deleted) {
throw std::runtime_error("Batch delete blobs failed");
}
};

for (auto&& defer_response : deferred_resps) {
auto response =
do_azure_client_call([&]() { get_defer_response(defer_response); }, _opts);
if (response.ret != 0) {
return response;
try {
auto r = defer_response.GetResponse();
if (!r.Value.Deleted) {
LOG_INFO("Azure batch delete failed, bucket {}, key {}, prefix {}, endpoint {}",
_opts.bucket, _opts.key, _opts.prefix, _opts.endpoint);
return {-1};
}
} catch (Azure::Storage::StorageException& e) {
if (Azure::Core::Http::HttpStatusCode::NotFound == e.StatusCode &&
0 == strcmp(e.ErrorCode.c_str(), BlobNotFound)) {
continue;
}
auto msg = fmt::format(
"Azure request failed because {}, http code {}, request id {}, bucket {}, "
"key {}, "
"prefix {}, endpoint {}",
e.Message, static_cast<int>(e.StatusCode), e.RequestId, _opts.bucket,
_opts.key, _opts.prefix, _opts.endpoint);
LOG_WARNING(msg);
return {-1, std::move(msg)};
}
}

Expand Down

0 comments on commit 81aa496

Please sign in to comment.