rgw/lc: At least wait for |rgw_lc_lock_max_time| while trying to fetch the lc-shard...

Currently each lc worker would try 1 second to get the lock on lc_shard to decide on which bucket to process and again 1 second to update the bucket status once bucket is lc processed. However when there are multiple rgws running lc, often shard is locked by the other lc worker or if there are issues when the rados is slow the lock is not processed within 1 second and worker either skips processing the bucket or skips updating the bucket, resulting in miss of LC or miss in updating the bucket status.
So in worst case when other lc worker is already processing a shard, wait for rgw_lc_lock_max_time to get the lock, as any given worker can max hold onto rgw_lc_lock_max_time a given shard.

Fixes: https://tracker.ceph.com/issues/72572
Resolves: rhbz#2401203

Signed-off-by: kchheda3 <kchheda3@bloomberg.net>
(cherry picked from commit 937ac626afd3bf443edf96aa177854e8eb291af5)
Signed-off-by: Matt Benjamin <mbenjamin@redhat.com>

diff --git a/src/rgw/rgw_lc.cc b/src/rgw/rgw_lc.cc
index 628160a74c04996d1c3e734fdb2864383813a8ad..c10ba31131b648b8d3888afe4cf0f8ccfcb66800 100644 (file)
--- a/src/rgw/rgw_lc.cc
+++ b/src/rgw/rgw_lc.cc
@@ -2341,7 +2341,9 @@ int RGWLC::process(int index, int max_lock_secs, LCWorker* worker,
     return false;
   };
 
-  SimpleBackoff shard_lock(5 /* max retries */, 50ms);
+  // retrying longer, so that you wait for at least for |max_lock_secs| which
+  // has a default value of 90 seconds.
+  SimpleBackoff shard_lock(50 /* max retries */, 50ms);
   if (! shard_lock.wait_backoff(lock_lambda)) {
     ldpp_dout(this, 0) << "RGWLC::process(): failed to acquire lock on "
                       << lc_shard << " after " << shard_lock.get_retries()
@@ -2357,7 +2359,7 @@ int RGWLC::process(int index, int max_lock_secs, LCWorker* worker,
     if (ret < 0) {
       ldpp_dout(this, 0) << "RGWLC::process() failed to get obj head "
           << lc_shard << ", ret=" << ret << dendl;
-      goto exit;
+      break;
     }
 
     /* if there is nothing at head, try to reinitialize head.marker with the
@@ -2377,7 +2379,7 @@ int RGWLC::process(int index, int max_lock_secs, LCWorker* worker,
       if (ret < 0) {
        ldpp_dout(this, 0) << "RGWLC::process() sal_lc->list_entries(lc_shard, head.marker, 1, "
                           << "entries) returned error ret==" << ret << dendl;
-       goto exit;
+        break;
       }
       if (entries.size() > 0) {
        entry = std::move(entries.front());
@@ -2400,14 +2402,14 @@ int RGWLC::process(int index, int max_lock_secs, LCWorker* worker,
         tmp_entry.bucket = head.marker;
 
         if (update_head(lc_shard, head, tmp_entry, now, worker->ix) != 0) {
-          goto exit;
+          break;
         }
         continue;
       }
       if (ret < 0) {
        ldpp_dout(this, 0) << "RGWLC::process() sal_lc->get_entry(lc_shard, head.marker, entry) "
                           << "returned error ret==" << ret << dendl;
-       goto exit;
+        break;
       }
     }
 
@@ -2424,8 +2426,8 @@ int RGWLC::process(int index, int max_lock_secs, LCWorker* worker,
               << " index: " << index << " worker ix: " << worker->ix << dendl;
          /* skip to next entry */
          if (update_head(lc_shard, head, entry, now, worker->ix) != 0) {
-            goto exit;
-         }
+            break;
+          }
           continue;
         }
       } else {
@@ -2436,17 +2438,17 @@ int RGWLC::process(int index, int max_lock_secs, LCWorker* worker,
                             << dendl;
          /* skip to next entry */
              if (update_head(lc_shard, head, entry, now, worker->ix) != 0) {
-               goto exit;
-             }
-         continue;
-       }
+            break;
+          }
+          continue;
+        }
       }
     } else {
       ldpp_dout(this, 5) << "RGWLC::process() entry.bucket.empty() == true at START 1"
                         << " (this is possible mainly before any lc policy has been stored"
                         << " or after removal of an lc_shard object)"
                          << dendl;
-      goto exit;
+      break;
     }
 
     /* When there are no more entries to process, entry will be
@@ -2464,12 +2466,12 @@ int RGWLC::process(int index, int max_lock_secs, LCWorker* worker,
     if (ret < 0) {
       ldpp_dout(this, 0) << "RGWLC::process() failed to set obj entry "
              << lc_shard << entry.bucket << entry.status << dendl;
-      goto exit;
+      break;
     }
 
     /* advance head for next waiter, then process */
     if (advance_head(lc_shard, head, entry, now) < 0) {
-      goto exit;
+      break;
     }
 
     ldpp_dout(this, 5) << "RGWLC::process(): START entry 2: " << entry
@@ -2515,16 +2517,15 @@ int RGWLC::process(int index, int max_lock_secs, LCWorker* worker,
                            << lc_shard << " entry=" << entry
                            << dendl;
         /* fatal, locked */
-        goto exit;
+        break;
       }
     }
 
-    if (check_if_shard_done(lc_shard, head, worker->ix) != 0 ) {
-      goto exit;
+    if (check_if_shard_done(lc_shard, head, worker->ix) != 0) {
+      break;
     }
   } while(1 && !once && !going_down());
 
-exit:
   lock->unlock();
   return 0;
 }