From a15dfe015471fcbbcdb020e850926f67be33915d Mon Sep 17 00:00:00 2001 From: Pavel Emelyanov Date: Wed, 16 Oct 2024 12:23:20 +0300 Subject: [PATCH 1/2] s3/client: Catch do_upload_file::upload_part() exceptions This method spawns part uploading in the background, but still may throw, e.g. preparing http request or claiming memory. In this case any outstanding part upload fibers are not waited on, and the whole do_upload_file object can be freed from under their feet. Also, the multipart upload is not aborted, thus losing track of it until g.c. happens. To fix it, catch any exception from upload_part() too, and if it happens, do what the regular upload_sink would do -- close the gate thus picking up any outstanding activity that may happen there and abort the multipart upload. Indentation is deliberately left broken Signed-off-by: Pavel Emelyanov --- utils/s3/client.cc | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/utils/s3/client.cc b/utils/s3/client.cc index a572036f37..8776c2af41 100644 --- a/utils/s3/client.cc +++ b/utils/s3/client.cc @@ -1009,19 +1009,22 @@ class client::do_upload_file : private multipart_upload { future<> multi_part_upload(file&& f, uint64_t total_size, size_t part_size) { co_await start_upload(); + std::exception_ptr ex; + try { for (size_t offset = 0; offset < total_size; offset += part_size) { part_size = std::min(total_size - offset, part_size); s3l.trace("upload_part: {}~{}/{}", offset, part_size, total_size); co_await upload_part(file{f}, offset, part_size); } - std::exception_ptr ex; - try { co_await finalize_upload(); } catch (...) { ex = std::current_exception(); } if (ex) { + if (!_bg_flushes.is_closed()) { + co_await _bg_flushes.close(); + } co_await abort_upload(); std::rethrow_exception(ex); } From 4a8ab9b3bcf27f5ba41186a2898d1c2e6079d8af Mon Sep 17 00:00:00 2001 From: Pavel Emelyanov Date: Wed, 16 Oct 2024 12:23:27 +0300 Subject: [PATCH 2/2] s3/client: Restore indentation after previous patch Signed-off-by: Pavel Emelyanov --- utils/s3/client.cc | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/utils/s3/client.cc b/utils/s3/client.cc index 8776c2af41..b41c0ec0be 100644 --- a/utils/s3/client.cc +++ b/utils/s3/client.cc @@ -1011,11 +1011,11 @@ class client::do_upload_file : private multipart_upload { std::exception_ptr ex; try { - for (size_t offset = 0; offset < total_size; offset += part_size) { - part_size = std::min(total_size - offset, part_size); - s3l.trace("upload_part: {}~{}/{}", offset, part_size, total_size); - co_await upload_part(file{f}, offset, part_size); - } + for (size_t offset = 0; offset < total_size; offset += part_size) { + part_size = std::min(total_size - offset, part_size); + s3l.trace("upload_part: {}~{}/{}", offset, part_size, total_size); + co_await upload_part(file{f}, offset, part_size); + } co_await finalize_upload(); } catch (...) {