From 2272c7a8dbf05325802953c95c0e04b8659fee25 Mon Sep 17 00:00:00 2001 From: Soumya Koduri Date: Sat, 5 Nov 2022 22:23:53 +0530 Subject: [PATCH] rgw/cloud-transition: Fix issues with MCG endpoint Few s3 endpoints (like Noobaa MCG or AWS) may return BucketAlreadyOwned or BucketAlreadyExists error if the target path (configured for cloud transition) already exists. Do not fail transition in those cases. Also fixed below issues - * with racing lc threads, when one lc thread is trying to check and create the target bucket, others may go ahead with the object transition assuming target path exists. * initialize few fields to avoid setting wrong content_len during transition Signed-off-by: Soumya Koduri --- src/rgw/rgw_lc_tier.cc | 28 +++++++++++++++++----------- src/rgw/rgw_lc_tier.h | 2 +- src/rgw/rgw_rest_conn.cc | 7 ++++++- src/rgw/rgw_sal_rados.cc | 12 +----------- 4 files changed, 25 insertions(+), 24 deletions(-) diff --git a/src/rgw/rgw_lc_tier.cc b/src/rgw/rgw_lc_tier.cc index 41ef42b358a..2a90c10ca5f 100644 --- a/src/rgw/rgw_lc_tier.cc +++ b/src/rgw/rgw_lc_tier.cc @@ -326,17 +326,17 @@ class RGWLCStreamRead rgw::sal::Object *obj; const real_time &mtime; - bool multipart; - uint64_t m_part_size; - off_t m_part_off; - off_t m_part_end; + bool multipart{false}; + uint64_t m_part_size{0}; + off_t m_part_off{0}; + off_t m_part_end{0}; std::unique_ptr read_op; - off_t ofs; - off_t end; + off_t ofs{0}; + off_t end{0}; rgw_rest_obj rest_obj; - int retcode; + int retcode{0}; public: RGWLCStreamRead(CephContext *_cct, const DoutPrefixProvider *_dpp, @@ -1245,8 +1245,7 @@ static int cloud_tier_create_bucket(RGWLCCloudTierCtx& tier_ctx) { out_bl, &bl, nullptr, null_yield); if (ret < 0 ) { - ldpp_dout(tier_ctx.dpp, 0) << "ERROR: failed to create target bucket: " << tier_ctx.target_bucket_name << ", ret:" << ret << dendl; - return ret; + ldpp_dout(tier_ctx.dpp, 0) << "create target bucket : " << tier_ctx.target_bucket_name << " returned ret:" << ret << dendl; } if (out_bl.length() > 0) { RGWXMLDecoder::XMLParser parser; @@ -1269,7 +1268,7 @@ static int cloud_tier_create_bucket(RGWLCCloudTierCtx& tier_ctx) { return -EIO; } - if (result.code != "BucketAlreadyOwnedByYou") { + if (result.code != "BucketAlreadyOwnedByYou" && result.code != "BucketAlreadyExists") { ldpp_dout(tier_ctx.dpp, 0) << "ERROR: Creating target bucket failed with error: " << result.code << dendl; return -EIO; } @@ -1278,9 +1277,15 @@ static int cloud_tier_create_bucket(RGWLCCloudTierCtx& tier_ctx) { return 0; } -int rgw_cloud_tier_transfer_object(RGWLCCloudTierCtx& tier_ctx) { +int rgw_cloud_tier_transfer_object(RGWLCCloudTierCtx& tier_ctx, std::set& cloud_targets) { int ret = 0; + // check if target_path is already created + std::set::iterator it; + + it = cloud_targets.find(tier_ctx.target_bucket_name); + tier_ctx.target_bucket_created = (it != cloud_targets.end()); + /* If run first time attempt to create the target bucket */ if (!tier_ctx.target_bucket_created) { ret = cloud_tier_create_bucket(tier_ctx); @@ -1290,6 +1295,7 @@ int rgw_cloud_tier_transfer_object(RGWLCCloudTierCtx& tier_ctx) { return ret; } tier_ctx.target_bucket_created = true; + cloud_targets.insert(tier_ctx.target_bucket_name); } /* Since multiple zones may try to transition the same object to the cloud, diff --git a/src/rgw/rgw_lc_tier.h b/src/rgw/rgw_lc_tier.h index ed0e826509a..a268dd41718 100644 --- a/src/rgw/rgw_lc_tier.h +++ b/src/rgw/rgw_lc_tier.h @@ -49,6 +49,6 @@ struct RGWLCCloudTierCtx { }; /* Transition object to cloud endpoint */ -int rgw_cloud_tier_transfer_object(RGWLCCloudTierCtx& tier_ctx); +int rgw_cloud_tier_transfer_object(RGWLCCloudTierCtx& tier_ctx, std::set& cloud_targets); #endif diff --git a/src/rgw/rgw_rest_conn.cc b/src/rgw/rgw_rest_conn.cc index 510cdd40049..7f02097ca30 100644 --- a/src/rgw/rgw_rest_conn.cc +++ b/src/rgw/rgw_rest_conn.cc @@ -389,7 +389,12 @@ int RGWRESTConn::send_resource(const DoutPrefixProvider *dpp, const std::string& return ret; } - return req.complete_request(y); + ret = req.complete_request(y); + if (ret < 0) { + ldpp_dout(dpp, 5) << __func__ << ": complete_request() resource=" << resource << " returned ret=" << ret << dendl; + } + + return ret; } RGWRESTReadResource::RGWRESTReadResource(RGWRESTConn *_conn, diff --git a/src/rgw/rgw_sal_rados.cc b/src/rgw/rgw_sal_rados.cc index 0a41f23c7af..1b44424a9d7 100644 --- a/src/rgw/rgw_sal_rados.cc +++ b/src/rgw/rgw_sal_rados.cc @@ -1850,24 +1850,14 @@ int RadosObject::transition_to_cloud(Bucket* bucket, tier_ctx.multipart_sync_threshold = rtier->get_rt().t.s3.multipart_sync_threshold; tier_ctx.storage_class = tier->get_storage_class(); - // check if target_path is already created - std::pair::iterator, bool> it; - - it = cloud_targets.insert(bucket_name); - tier_ctx.target_bucket_created = !(it.second); - ldpp_dout(dpp, 0) << "Transitioning object(" << o.key << ") to the cloud endpoint(" << endpoint << ")" << dendl; /* Transition object to cloud end point */ - int ret = rgw_cloud_tier_transfer_object(tier_ctx); + int ret = rgw_cloud_tier_transfer_object(tier_ctx, cloud_targets); if (ret < 0) { ldpp_dout(dpp, 0) << "ERROR: failed to transfer object(" << o.key << ") to the cloud endpoint(" << endpoint << ") ret=" << ret << dendl; return ret; - - if (!tier_ctx.target_bucket_created) { - cloud_targets.erase(it.first); - } } if (update_object) { -- 2.39.5