From: J. Eric Ivancich <ivancich@redhat.com>
Date: Thu, 31 Jan 2019 19:21:07 +0000 (-0500)
Subject: rgw: `radosgw-admin bucket rm ... --purge-objects` can hang...
X-Git-Tag: v14.1.0~235^2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=refs%2Fpull%2F26231%2Fhead;p=ceph.git

rgw: `radosgw-admin bucket rm ... --purge-objects` can hang...

This command can hang (i.e., enter an infinite loop) due to
problematic bucket index entries left as a result of bug
https://tracker.ceph.com/issues/38007 .

The fix is to ignore the false bucket index entries -- since they do
not represent actual objects -- and remove all actual objects in the
bucket, so that bucket itself can be removed.

This fixes the both code paths whether `--bypass-gc` is specified or
not.

Furthermore, to made these operations more efficient, the internal
listing of the bucket is done unordered. This would improve behavior
when removing buckets with a large number of objects.

Fixes: http://tracker.ceph.com/issues/38134
Signed-off-by: J. Eric Ivancich <ivancich@redhat.com>
---

diff --git a/src/rgw/rgw_bucket.cc b/src/rgw/rgw_bucket.cc
index e73efee7fa35..d08494afc275 100644
--- a/src/rgw/rgw_bucket.cc
+++ b/src/rgw/rgw_bucket.cc
@@ -542,11 +542,13 @@ int rgw_remove_bucket(RGWRados *store, rgw_bucket& bucket, bool delete_children)
   int max = 1000;
 
   list_op.params.list_versions = true;
+  list_op.params.allow_unordered = true;
 
+  bool is_truncated = false;
   do {
     objs.clear();
 
-    ret = list_op.list_objects(max, &objs, &common_prefixes, NULL);
+    ret = list_op.list_objects(max, &objs, &common_prefixes, &is_truncated);
     if (ret < 0)
       return ret;
 
@@ -558,11 +560,11 @@ int rgw_remove_bucket(RGWRados *store, rgw_bucket& bucket, bool delete_children)
     for (const auto& obj : objs) {
       rgw_obj_key key(obj.key);
       ret = rgw_remove_object(store, info, bucket, key);
-      if (ret < 0)
+      if (ret < 0 && ret != -ENOENT) {
         return ret;
+      }
     }
-
-  } while (!objs.empty());
+  } while(is_truncated);
 
   string prefix, delimiter;
 
@@ -578,9 +580,12 @@ int rgw_remove_bucket(RGWRados *store, rgw_bucket& bucket, bool delete_children)
 
   RGWObjVersionTracker objv_tracker;
 
-  ret = store->delete_bucket(info, objv_tracker);
+  // if we deleted children above we will force delete, as any that
+  // remain is detrius from a prior bug
+  ret = store->delete_bucket(info, objv_tracker, !delete_children);
   if (ret < 0) {
-    lderr(store->ctx()) << "ERROR: could not remove bucket " << bucket.name << dendl;
+    lderr(store->ctx()) << "ERROR: could not remove bucket " <<
+      bucket.name << dendl;
     return ret;
   }
 
@@ -648,16 +653,20 @@ int rgw_remove_bucket_bypass_gc(RGWRados *store, rgw_bucket& bucket,
   RGWRados::Bucket::List list_op(&target);
 
   list_op.params.list_versions = true;
+  list_op.params.allow_unordered = true;
 
   std::list<librados::AioCompletion*> handles;
 
   int max = 1000;
   int max_aio = concurrent_max;
-  ret = list_op.list_objects(max, &objs, &common_prefixes, NULL);
-  if (ret < 0)
-    return ret;
+  bool is_truncated = true;
+
+  while (is_truncated) {
+    objs.clear();
+    ret = list_op.list_objects(max, &objs, &common_prefixes, &is_truncated);
+    if (ret < 0)
+      return ret;
 
-  while (!objs.empty()) {
     std::vector<rgw_bucket_dir_entry>::iterator it = objs.begin();
     for (; it != objs.end(); ++it) {
       RGWObjState *astate = NULL;
@@ -720,11 +729,6 @@ int rgw_remove_bucket_bypass_gc(RGWRados *store, rgw_bucket& bucket,
         max_aio = concurrent_max;
       }
     } // for all RGW objects
-    objs.clear();
-
-    ret = list_op.list_objects(max, &objs, &common_prefixes, NULL);
-    if (ret < 0)
-      return ret;
   }
 
   ret = drain_handles(handles);
@@ -740,7 +744,10 @@ int rgw_remove_bucket_bypass_gc(RGWRados *store, rgw_bucket& bucket,
 
   RGWObjVersionTracker objv_tracker;
 
-  ret = store->delete_bucket(info, objv_tracker);
+  // this function can only be run if caller wanted children to be
+  // deleted, so we can ignore the check for children as any that
+  // remain are detritus from a prior bug
+  ret = store->delete_bucket(info, objv_tracker, false);
   if (ret < 0) {
     lderr(store->ctx()) << "ERROR: could not remove bucket " << bucket.name << dendl;
     return ret;