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.

Signed-off-by: kchheda3 <kchheda3@bloomberg.net>

diff --git a/src/rgw/rgw_lc.cc b/src/rgw/rgw_lc.cc
index 40f2ac05269f47385fe23fd79b62053cfb1dd352..bf518cd4496d30acdb8d5db19e0563fbd80b50ea 100644 (file)
--- a/src/rgw/rgw_lc.cc
+++ b/src/rgw/rgw_lc.cc
@@ -2381,7 +2381,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()
@@ -2397,7 +2399,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
@@ -2417,7 +2419,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());
@@ -2440,14 +2442,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;
       }
     }
 
@@ -2464,8 +2466,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 {
@@ -2476,17 +2478,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
@@ -2504,12 +2506,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
@@ -2555,16 +2557,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(this, null_yield);
   return 0;
 }