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 <matthew.heler@hotmail.com>

diff --git a/src/rgw/driver/rados/rgw_lc_tier.cc b/src/rgw/driver/rados/rgw_lc_tier.cc
index 5c1106e21efc50652e0ca850fad71fa05e71fc21..8bdd8fcc317e50e12c99049d2e916ce9aaadbbaa 100644 (file)
--- 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<int()> 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<int>(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<RGWLCCloudStreamPut> 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<RGWLCStreamRead>(tier_ctx.cct, tier_ctx.dpp,
           tier_ctx.obj, tier_ctx.o.meta.mtime, tier_ctx.y);
     auto wf = std::make_shared<RGWLCCloudStreamPut>(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<std::string>& cloud_targets) {
@@ -1669,6 +1680,6 @@ static int do_cloud_tier_transfer_object(RGWLCCloudTierCtx& tier_ctx, std::set<s
 }
 
 int rgw_cloud_tier_transfer_object(RGWLCCloudTierCtx& tier_ctx, std::set<std::string>& 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 84fcf5c4f699ddb9a4e59a0c972d226b361316f4..cf2eb7ce5d49c590ae4b6bce09ab93446fa4189b 100644 (file)
--- 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<std::string, std::string>& 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<int()> op);

diff --git a/src/rgw/driver/rados/rgw_rados.cc b/src/rgw/driver/rados/rgw_rados.cc
index 99863a5aead479fea5f6a54c212abf638d933697..044a2266eac6cd1194ff080c09123598f44b1609 100644 (file)
--- 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);