From c622ab08fe04a49f446d6c742d927b681062c801 Mon Sep 17 00:00:00 2001 From: Joao Eduardo Luis Date: Thu, 23 Nov 2023 05:54:36 +0000 Subject: [PATCH 1/3] rgw/sfs: fail gracefully if objref not created It may happen if we conflict in-flight with another thread. Ensure we just fail gracefully to the client. Fixes: aquarist-labs/s3gw#820 Signed-off-by: Joao Eduardo Luis --- src/rgw/driver/sfs/multipart.cc | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/rgw/driver/sfs/multipart.cc b/src/rgw/driver/sfs/multipart.cc index c6a9f396943ce..39b7bf72a176a 100644 --- a/src/rgw/driver/sfs/multipart.cc +++ b/src/rgw/driver/sfs/multipart.cc @@ -443,6 +443,12 @@ int SFSMultipartUploadV2::complete( << dendl; return -ERR_INTERNAL_ERROR; } + if (!objref) { + // We were unable to create a version, but this might very well have been + // because we conflicted with another in-flight transaction. Lets return + // back to the user, and they can complete it again if they so desire. + return -ERR_INTERNAL_ERROR; + } auto destpath = store->get_data_path() / objref->get_storage_path(); lsfs_debug(dpp ) << fmt::format("moving final object from {} to {}", objpath, destpath) From ec5fa45ad226328c35628417f4085f339c9d8bb7 Mon Sep 17 00:00:00 2001 From: Joao Eduardo Luis Date: Thu, 23 Nov 2023 05:56:00 +0000 Subject: [PATCH 2/3] rgw/sfs: rm dangling multipart final build file Signed-off-by: Joao Eduardo Luis --- src/rgw/driver/sfs/multipart.cc | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/rgw/driver/sfs/multipart.cc b/src/rgw/driver/sfs/multipart.cc index 39b7bf72a176a..523ba6cf9d20a 100644 --- a/src/rgw/driver/sfs/multipart.cc +++ b/src/rgw/driver/sfs/multipart.cc @@ -18,6 +18,7 @@ #include #include +#include #include #include "rgw/driver/sfs/fmt.h" @@ -441,12 +442,14 @@ int SFSMultipartUploadV2::complete( bucketref->get_bucket_id(), mp->object_name, e.what() ) << dendl; + std::filesystem::remove(objpath, ec); return -ERR_INTERNAL_ERROR; } if (!objref) { // We were unable to create a version, but this might very well have been // because we conflicted with another in-flight transaction. Lets return // back to the user, and they can complete it again if they so desire. + std::filesystem::remove(objpath, ec); return -ERR_INTERNAL_ERROR; } auto destpath = store->get_data_path() / objref->get_storage_path(); @@ -462,6 +465,7 @@ int SFSMultipartUploadV2::complete( destpath, ec.message() ) << dendl; + std::filesystem::remove(objpath, ec); return -ERR_INTERNAL_ERROR; } @@ -473,6 +477,7 @@ int SFSMultipartUploadV2::complete( objpath, destpath, cpp_strerror(errno) ) << dendl; + std::filesystem::remove(objpath, ec); return -ERR_INTERNAL_ERROR; } From 15dba00718abddce31ff34244ba83f3413f904b9 Mon Sep 17 00:00:00 2001 From: Joao Eduardo Luis Date: Thu, 23 Nov 2023 13:58:58 +0000 Subject: [PATCH 3/3] rgw/sfs: handle result from metadata update Signed-off-by: Joao Eduardo Luis --- src/rgw/driver/sfs/multipart.cc | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/rgw/driver/sfs/multipart.cc b/src/rgw/driver/sfs/multipart.cc index 523ba6cf9d20a..0bf31bad92e94 100644 --- a/src/rgw/driver/sfs/multipart.cc +++ b/src/rgw/driver/sfs/multipart.cc @@ -510,7 +510,9 @@ int SFSMultipartUploadV2::complete( .delete_at = ceph::real_time()} ); try { - objref->metadata_finish(store, bucketref->get_info().versioning_enabled()); + res = objref->metadata_finish( + store, bucketref->get_info().versioning_enabled() + ); } catch (const std::system_error& e) { lsfs_err(dpp) << fmt::format( "failed to update db object {}: {}", objref->name, @@ -519,6 +521,11 @@ int SFSMultipartUploadV2::complete( << dendl; return -ERR_INTERNAL_ERROR; } + if (!res) { + lsfs_err(dpp) << fmt::format("failed to update db object {}", objref->name) + << dendl; + return -ERR_INTERNAL_ERROR; + } // mark multipart upload done res = mpdb.mark_done(upload_id);