From: Matthew N. Heler Date: Wed, 3 Jun 2026 19:20:11 +0000 (-0500) Subject: rgw/lc: retry cloud-tier 500 InternalError for bucket create and object PUT X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=9c07120d1055c4328bcc46dcfac356c9ea80aa6f;p=ceph.git rgw/lc: retry cloud-tier 500 InternalError for bucket create and object PUT The cloud-tier transfer only treated 503 (-EBUSY) as retryable. A 500 from the remote surfaced as -ERR_INTERNAL_ERROR and failed the object outright, but S3 backends use 500 as a transient "try again" signal. Retry it the same way we retry -EBUSY. The helper is no longer EBUSY-specific, so rename it to retry_on_transient_error; the restore path uses it too and gets the same behaviour. Even then a 500 never reached the retry: cloud_tier_create_bucket mapped an error response to -EIO and an error with no body to 0 (success). Return the real errno for transient failures, and stop treating an empty-body error as a successful create. Signed-off-by: Matthew N. Heler --- diff --git a/src/rgw/driver/rados/rgw_lc_tier.cc b/src/rgw/driver/rados/rgw_lc_tier.cc index 5c1106e21ef..8bdd8fcc317 100644 --- a/src/rgw/driver/rados/rgw_lc_tier.cc +++ b/src/rgw/driver/rados/rgw_lc_tier.cc @@ -31,7 +31,7 @@ using namespace std; -int retry_on_busy(optional_yield y, const DoutPrefixProvider *dpp, +int retry_on_transient_error(optional_yield y, const DoutPrefixProvider *dpp, CephContext *cct, const char *op_name, std::function op) { @@ -42,19 +42,20 @@ int retry_on_busy(optional_yield y, const DoutPrefixProvider *dpp, int ret = 0; for (int i = 0; i < max_attempts; i++) { ret = op(); - if (ret != -EBUSY) return ret; + // 503 -> -EBUSY and 500 -> -ERR_INTERNAL_ERROR are transient on S3 backends + if (ret != -EBUSY && ret != -ERR_INTERNAL_ERROR) return ret; if (i == max_attempts - 1) { ldpp_dout(dpp, 0) << op_name << ": exhausted " << max_attempts - << " -EBUSY retries, giving up" << dendl; + << " transient-error retries (ret=" << ret << "), giving up" << dendl; return ret; } int64_t base = std::min(initial_ms << std::min(i, 30), max_ms); int delay_ms = base - ceph::util::generate_random_number(0, base / 10); - ldpp_dout(dpp, 1) << op_name << ": -EBUSY, attempt " << (i + 1) << "/" - << max_attempts << "; retrying after " << delay_ms - << "ms" << dendl; + ldpp_dout(dpp, 1) << op_name << ": transient error (ret=" << ret << "), attempt " + << (i + 1) << "/" << max_attempts << "; retrying after " + << delay_ms << "ms" << dendl; if (y) { auto& yc = y.get_yield_context(); @@ -330,7 +331,7 @@ int rgw_cloud_tier_restore_object(RGWLCCloudTierCtx& tier_ctx, rgw_obj dest_obj(dest_bucket, rgw_obj_key(target_obj_name)); - ret = retry_on_busy(tier_ctx.y, tier_ctx.dpp, tier_ctx.cct, __func__, [&]() -> int { + ret = retry_on_transient_error(tier_ctx.y, tier_ctx.dpp, tier_ctx.cct, __func__, [&]() -> int { if (!in_progress) { // first time. Send RESTORE req. ret = cloud_tier_restore(tier_ctx.dpp, tier_ctx.conn, dest_obj, days, glacier_params, tier_ctx.y); ldpp_dout(tier_ctx.dpp, 20) << __func__ << "Restoring object=" << target_obj_name << "returned ret = " << ret << dendl; @@ -362,7 +363,7 @@ int rgw_cloud_tier_restore_object(RGWLCCloudTierCtx& tier_ctx, return 0; } - ret = retry_on_busy(tier_ctx.y, tier_ctx.dpp, tier_ctx.cct, __func__, [&]() { + ret = retry_on_transient_error(tier_ctx.y, tier_ctx.dpp, tier_ctx.cct, __func__, [&]() { return rgw_cloud_tier_get_object(tier_ctx, false, headers, pset_mtime, etag, accounted_size, attrs, cb); }); @@ -1036,7 +1037,7 @@ static int cloud_tier_send_multipart_part(RGWLCCloudTierCtx& tier_ctx, std::shared_ptr writef; // per-part retry: the outer retry restarts from part 1 since upload state // doesn't persist which parts have been sent - ret = retry_on_busy(tier_ctx.y, tier_ctx.dpp, tier_ctx.cct, __func__, [&]() { + ret = retry_on_transient_error(tier_ctx.y, tier_ctx.dpp, tier_ctx.cct, __func__, [&]() { auto rf = std::make_shared(tier_ctx.cct, tier_ctx.dpp, tier_ctx.obj, tier_ctx.o.meta.mtime, tier_ctx.y); auto wf = std::make_shared(tier_ctx.dpp, @@ -1327,7 +1328,7 @@ static int cloud_tier_abort_multipart_upload(RGWLCCloudTierCtx& tier_ctx, const std::string& upload_id) { int ret; - ret = retry_on_busy(tier_ctx.y, tier_ctx.dpp, tier_ctx.cct, __func__, [&]() { + ret = retry_on_transient_error(tier_ctx.y, tier_ctx.dpp, tier_ctx.cct, __func__, [&]() { return cloud_tier_abort_multipart(tier_ctx.dpp, tier_ctx.conn, dest_obj, upload_id, tier_ctx.y); }); @@ -1570,7 +1571,14 @@ static int cloud_tier_create_bucket(RGWLCCloudTierCtx& tier_ctx) { ret = tier_ctx.conn.send_resource(tier_ctx.dpp, "PUT", resource, nullptr, nullptr, out_bl, &bl, nullptr, tier_ctx.y); - if (ret == -EBUSY) return ret; + /* + * Transient remote errors (503 -> -EBUSY, 500 -> -ERR_INTERNAL_ERROR) are + * retried by the caller's retry_on_transient_error wrapper, so surface them + * as-is rather than flattening to -EIO. + */ + if (ret == -EBUSY || ret == -ERR_INTERNAL_ERROR) { + return ret; + } if (ret < 0) { ldpp_dout(tier_ctx.dpp, 0) << "create target bucket : " << tier_ctx.target_bucket_name << " returned ret:" << ret << dendl; } @@ -1597,11 +1605,14 @@ static int cloud_tier_create_bucket(RGWLCCloudTierCtx& tier_ctx) { 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; + return (ret < 0) ? ret : -EIO; } + // benign "already owned/exists" response + return 0; } - return 0; + // no response body: propagate any transport error instead of masking as success + return ret; } static int do_cloud_tier_transfer_object(RGWLCCloudTierCtx& tier_ctx, std::set& cloud_targets) { @@ -1669,6 +1680,6 @@ static int do_cloud_tier_transfer_object(RGWLCCloudTierCtx& tier_ctx, std::set& cloud_targets) { - return retry_on_busy(tier_ctx.y, tier_ctx.dpp, tier_ctx.cct, __func__, + return retry_on_transient_error(tier_ctx.y, tier_ctx.dpp, tier_ctx.cct, __func__, [&]() { return do_cloud_tier_transfer_object(tier_ctx, cloud_targets); }); } diff --git a/src/rgw/driver/rados/rgw_lc_tier.h b/src/rgw/driver/rados/rgw_lc_tier.h index 84fcf5c4f69..cf2eb7ce5d4 100644 --- a/src/rgw/driver/rados/rgw_lc_tier.h +++ b/src/rgw/driver/rados/rgw_lc_tier.h @@ -83,5 +83,5 @@ int cloud_tier_restore(const DoutPrefixProvider *dpp, bool is_restore_in_progress(const DoutPrefixProvider *dpp, std::map& headers); -int retry_on_busy(optional_yield y, const DoutPrefixProvider *dpp, CephContext *cct, +int retry_on_transient_error(optional_yield y, const DoutPrefixProvider *dpp, CephContext *cct, const char *op_name, std::function op); diff --git a/src/rgw/driver/rados/rgw_rados.cc b/src/rgw/driver/rados/rgw_rados.cc index 99863a5aead..044a2266eac 100644 --- a/src/rgw/driver/rados/rgw_rados.cc +++ b/src/rgw/driver/rados/rgw_rados.cc @@ -5653,7 +5653,7 @@ int RGWRados::restore_obj_from_cloud(RGWLCCloudTierCtx& tier_ctx, attrs, days, glacier_params, in_progress, &cb); } else { ldpp_dout(dpp, 20) << "Fetching object:" << dest_obj << "from the cloud" << dendl; - ret = retry_on_busy(tier_ctx.y, dpp, cct, __func__, [&]() { + ret = retry_on_transient_error(tier_ctx.y, dpp, cct, __func__, [&]() { return rgw_cloud_tier_get_object(tier_ctx, false, headers, &set_mtime, etag, accounted_size, attrs, &cb);