rgw: fix 'gc list --include-all' command infinite loop the first 1000 items

When the items to gc over 1000, 'gc list --include-all' command will infinite loop the first 1000 items.
Add next_marker to move to the next 1000 items.

Fixes: http://tracker.ceph.com/issues/19978
Signed-off-by: fang yuxiang <fang.yuxiang@eisoo.com>
Signed-off-by: Shasha Lu <lu.shasha@eisoo.com>
(cherry picked from commit fc29f52ebca63104a05515484088ff136dfb0b15)

diff --git a/src/cls/rgw/cls_rgw.cc b/src/cls/rgw/cls_rgw.cc
index 9873590487bc29245e4c877bb737253894d8268d..b568a931e0f3e9eb52cd6b9fd40d0cbb490c3407 100644 (file)
--- a/src/cls/rgw/cls_rgw.cc
+++ b/src/cls/rgw/cls_rgw.cc
@@ -3172,10 +3172,10 @@ static int gc_iterate_entries(cls_method_context_t hctx, const string& marker, b
     *truncated = false;
 
   string start_key;
-  if (key_iter.empty()) {
+  if (marker.empty()) {
     prepend_index_prefix(marker, GC_OBJ_TIME_INDEX, &start_key);
   } else {
-    start_key = key_iter;
+    start_key = marker;
   }
 
   if (expired_only) {
@@ -3219,7 +3219,8 @@ static int gc_iterate_entries(cls_method_context_t hctx, const string& marker, b
       if (max_entries && (i >= max_entries)) {
         if (truncated)
           *truncated = true;
-        key_iter = key;
+        --iter;
+        key_iter = iter->first;
         return 0;
       }
 
@@ -3244,11 +3245,10 @@ static int gc_list_cb(cls_method_context_t hctx, const string& key, cls_rgw_gc_o
 
 static int gc_list_entries(cls_method_context_t hctx, const string& marker,
                           uint32_t max, bool expired_only,
-                           list<cls_rgw_gc_obj_info>& entries, bool *truncated)
+                           list<cls_rgw_gc_obj_info>& entries, bool *truncated, string& next_marker)
 {
-  string key_iter;
   int ret = gc_iterate_entries(hctx, marker, expired_only,
-                              key_iter, max, truncated,
+                              next_marker, max, truncated,
                               gc_list_cb, &entries);
   return ret;
 }
@@ -3266,7 +3266,8 @@ static int rgw_cls_gc_list(cls_method_context_t hctx, bufferlist *in, bufferlist
   }
 
   cls_rgw_gc_list_ret op_ret;
-  int ret = gc_list_entries(hctx, op.marker, op.max, op.expired_only, op_ret.entries, &op_ret.truncated);
+  int ret = gc_list_entries(hctx, op.marker, op.max, op.expired_only, 
+   op_ret.entries, &op_ret.truncated, op_ret.next_marker);
   if (ret < 0)
     return ret;
 

diff --git a/src/cls/rgw/cls_rgw_client.cc b/src/cls/rgw/cls_rgw_client.cc
index 437c888098c70a7065c748a739c47ba59b4ad487..60b4ccf051bd19530f9ea3f4dc0372746cebdb84 100644 (file)
--- a/src/cls/rgw/cls_rgw_client.cc
+++ b/src/cls/rgw/cls_rgw_client.cc
@@ -634,7 +634,7 @@ void cls_rgw_gc_defer_entry(ObjectWriteOperation& op, uint32_t expiration_secs,
 }
 
 int cls_rgw_gc_list(IoCtx& io_ctx, string& oid, string& marker, uint32_t max, bool expired_only,
-                    list<cls_rgw_gc_obj_info>& entries, bool *truncated)
+                    list<cls_rgw_gc_obj_info>& entries, bool *truncated, string& next_marker)
 {
   bufferlist in, out;
   cls_rgw_gc_list_op call;
@@ -658,8 +658,8 @@ int cls_rgw_gc_list(IoCtx& io_ctx, string& oid, string& marker, uint32_t max, bo
 
   if (truncated)
     *truncated = ret.truncated;
-
- return r;
+  next_marker = std::move(ret.next_marker);
+  return r;
 }
 
 void cls_rgw_gc_remove(librados::ObjectWriteOperation& op, const list<string>& tags)

diff --git a/src/cls/rgw/cls_rgw_client.h b/src/cls/rgw/cls_rgw_client.h
index be7106a6c34f9658dee0bd2cc4e9864dd884b836..7601fa52a90f79235cd163cf1e4511f56d819095 100644 (file)
--- a/src/cls/rgw/cls_rgw_client.h
+++ b/src/cls/rgw/cls_rgw_client.h
@@ -474,7 +474,7 @@ void cls_rgw_gc_set_entry(librados::ObjectWriteOperation& op, uint32_t expiratio
 void cls_rgw_gc_defer_entry(librados::ObjectWriteOperation& op, uint32_t expiration_secs, const string& tag);
 
 int cls_rgw_gc_list(librados::IoCtx& io_ctx, string& oid, string& marker, uint32_t max, bool expired_only,
-                    list<cls_rgw_gc_obj_info>& entries, bool *truncated);
+                    list<cls_rgw_gc_obj_info>& entries, bool *truncated, string& next_marker);
 
 void cls_rgw_gc_remove(librados::ObjectWriteOperation& op, const list<string>& tags);
 

diff --git a/src/cls/rgw/cls_rgw_ops.cc b/src/cls/rgw/cls_rgw_ops.cc
index 0b3701bf9ff315f1a59164286324129582a57518..e500c62427a6afeebfc919552b01baaf361e83d1 100644 (file)
--- a/src/cls/rgw/cls_rgw_ops.cc
+++ b/src/cls/rgw/cls_rgw_ops.cc
@@ -64,6 +64,7 @@ void cls_rgw_gc_list_op::generate_test_instances(list<cls_rgw_gc_list_op*>& ls)
 void cls_rgw_gc_list_ret::dump(Formatter *f) const
 {
   encode_json("entries", entries, f);
+  f->dump_string("next_marker", next_marker);
   f->dump_int("truncated", (int)truncated);
 }
 

diff --git a/src/cls/rgw/cls_rgw_ops.h b/src/cls/rgw/cls_rgw_ops.h
index 74035929387b443d54326c54cddb5d6dc519350f..5b3a17486d3e8a85cf5d3bb5f2f87e2b64ddfb99 100644 (file)
--- a/src/cls/rgw/cls_rgw_ops.h
+++ b/src/cls/rgw/cls_rgw_ops.h
@@ -841,20 +841,24 @@ WRITE_CLASS_ENCODER(cls_rgw_gc_list_op)
 
 struct cls_rgw_gc_list_ret {
   list<cls_rgw_gc_obj_info> entries;
+  string next_marker;
   bool truncated;
 
   cls_rgw_gc_list_ret() : truncated(false) {}
 
   void encode(bufferlist& bl) const {
-    ENCODE_START(1, 1, bl);
+    ENCODE_START(2, 1, bl);
     ::encode(entries, bl);
+    ::encode(next_marker, bl);
     ::encode(truncated, bl);
     ENCODE_FINISH(bl);
   }
 
   void decode(bufferlist::iterator& bl) {
-    DECODE_START(1, bl);
+    DECODE_START(2, bl);
     ::decode(entries, bl);
+    if (struct_v >= 2)
+      ::decode(next_marker, bl);
     ::decode(truncated, bl);
     DECODE_FINISH(bl);
   }

diff --git a/src/rgw/rgw_gc.cc b/src/rgw/rgw_gc.cc
index a5f5d395055b3878f1a5fb4ed7964ffd1ffef19b..f37a4c799b6164dd720ba509e1e7dfd083136aca 100644 (file)
--- a/src/rgw/rgw_gc.cc
+++ b/src/rgw/rgw_gc.cc
@@ -94,10 +94,11 @@ int RGWGC::remove(int index, const std::list<string>& tags)
 int RGWGC::list(int *index, string& marker, uint32_t max, bool expired_only, std::list<cls_rgw_gc_obj_info>& result, bool *truncated)
 {
   result.clear();
+  string next_marker;
 
   for (; *index < max_objs && result.size() < max; (*index)++, marker.clear()) {
     std::list<cls_rgw_gc_obj_info> entries;
-    int ret = cls_rgw_gc_list(store->gc_pool_ctx, obj_names[*index], marker, max - result.size(), expired_only, entries, truncated);
+    int ret = cls_rgw_gc_list(store->gc_pool_ctx, obj_names[*index], marker, max - result.size(), expired_only, entries, truncated, next_marker);
     if (ret == -ENOENT)
       continue;
     if (ret < 0)
@@ -108,6 +109,8 @@ int RGWGC::list(int *index, string& marker, uint32_t max, bool expired_only, std
       result.push_back(*iter);
     }
 
+    marker = next_marker;
+
     if (*index == max_objs - 1) {
       /* we cut short here, truncated will hold the correct value */
       return 0;
@@ -154,12 +157,13 @@ int RGWGC::process(int index, int max_secs)
     return ret;
 
   string marker;
+  string next_marker;
   bool truncated;
   IoCtx *ctx = new IoCtx;
   do {
     int max = 100;
     std::list<cls_rgw_gc_obj_info> entries;
-    ret = cls_rgw_gc_list(store->gc_pool_ctx, obj_names[index], marker, max, true, entries, &truncated);
+    ret = cls_rgw_gc_list(store->gc_pool_ctx, obj_names[index], marker, max, true, entries, &truncated, next_marker);
     if (ret == -ENOENT) {
       ret = 0;
       goto done;

diff --git a/src/test/cls_rgw/test_cls_rgw.cc b/src/test/cls_rgw/test_cls_rgw.cc
index 22f137dd284f95de2c7328aca86335beb73e5d06..a15a033db0e6324b29016bec4b4c41685d464843 100644 (file)
--- a/src/test/cls_rgw/test_cls_rgw.cc
+++ b/src/test/cls_rgw/test_cls_rgw.cc
@@ -422,16 +422,18 @@ TEST(cls_rgw, gc_set)
   bool truncated;
   list<cls_rgw_gc_obj_info> entries;
   string marker;
+  string next_marker;
 
   /* list chains, verify truncated */
-  ASSERT_EQ(0, cls_rgw_gc_list(ioctx, oid, marker, 8, true, entries, &truncated));
+  ASSERT_EQ(0, cls_rgw_gc_list(ioctx, oid, marker, 8, true, entries, &truncated, next_marker));
   ASSERT_EQ(8, (int)entries.size());
   ASSERT_EQ(1, truncated);
 
   entries.clear();
+  next_marker.clear();
 
   /* list all chains, verify not truncated */
-  ASSERT_EQ(0, cls_rgw_gc_list(ioctx, oid, marker, 10, true, entries, &truncated));
+  ASSERT_EQ(0, cls_rgw_gc_list(ioctx, oid, marker, 10, true, entries, &truncated, next_marker));
   ASSERT_EQ(10, (int)entries.size());
   ASSERT_EQ(0, truncated);
  
@@ -468,6 +470,84 @@ TEST(cls_rgw, gc_set)
   }
 }
 
+TEST(cls_rgw, gc_list)
+{
+  /* add chains */
+  string oid = "obj";
+  for (int i = 0; i < 10; i++) {
+    char buf[32];
+    snprintf(buf, sizeof(buf), "chain-%d", i);
+    string tag = buf;
+    librados::ObjectWriteOperation op;
+    cls_rgw_gc_obj_info info;
+
+    cls_rgw_obj obj1, obj2;
+    create_obj(obj1, i, 1);
+    create_obj(obj2, i, 2);
+    info.chain.objs.push_back(obj1);
+    info.chain.objs.push_back(obj2);
+
+    op.create(false); // create object
+
+    info.tag = tag;
+    cls_rgw_gc_set_entry(op, 0, info);
+
+    ASSERT_EQ(0, ioctx.operate(oid, &op));
+  }
+
+  bool truncated;
+  list<cls_rgw_gc_obj_info> entries;
+  list<cls_rgw_gc_obj_info> entries2;
+  string marker;
+  string next_marker;
+
+  /* list chains, verify truncated */
+  ASSERT_EQ(0, cls_rgw_gc_list(ioctx, oid, marker, 8, true, entries, &truncated, next_marker));
+  ASSERT_EQ(8, (int)entries.size());
+  ASSERT_EQ(1, truncated);
+
+  marker = next_marker;
+  next_marker.clear();
+
+  ASSERT_EQ(0, cls_rgw_gc_list(ioctx, oid, marker, 8, true, entries2, &truncated, next_marker));
+  ASSERT_EQ(2, (int)entries2.size());
+  ASSERT_EQ(0, truncated);
+
+  entries.splice(entries.end(), entries2);
+
+  /* verify all chains are valid */
+  list<cls_rgw_gc_obj_info>::iterator iter = entries.begin();
+  for (int i = 0; i < 10; i++, ++iter) {
+    cls_rgw_gc_obj_info& entry = *iter;
+
+    /* create expected chain name */
+    char buf[32];
+    snprintf(buf, sizeof(buf), "chain-%d", i);
+    string tag = buf;
+
+    /* verify chain name as expected */
+    ASSERT_EQ(entry.tag, tag);
+
+    /* verify expected num of objects in chain */
+    ASSERT_EQ(2, (int)entry.chain.objs.size());
+
+    list<cls_rgw_obj>::iterator oiter = entry.chain.objs.begin();
+    cls_rgw_obj obj1, obj2;
+
+    /* create expected objects */
+    create_obj(obj1, i, 1);
+    create_obj(obj2, i, 2);
+
+    /* assign returned object names */
+    cls_rgw_obj& ret_obj1 = *oiter++;
+    cls_rgw_obj& ret_obj2 = *oiter;
+
+    /* verify objects are as expected */
+    ASSERT_EQ(1, (int)cmp_objs(obj1, ret_obj1));
+    ASSERT_EQ(1, (int)cmp_objs(obj2, ret_obj2));
+  }
+}
+
 TEST(cls_rgw, gc_defer)
 {
   librados::IoCtx ioctx;
@@ -496,9 +576,10 @@ TEST(cls_rgw, gc_defer)
   bool truncated;
   list<cls_rgw_gc_obj_info> entries;
   string marker;
+  string next_marker;
 
   /* list chains, verify num entries as expected */
-  ASSERT_EQ(0, cls_rgw_gc_list(ioctx, oid, marker, 1, true, entries, &truncated));
+  ASSERT_EQ(0, cls_rgw_gc_list(ioctx, oid, marker, 1, true, entries, &truncated, next_marker));
   ASSERT_EQ(1, (int)entries.size());
   ASSERT_EQ(0, truncated);
 
@@ -509,17 +590,19 @@ TEST(cls_rgw, gc_defer)
   ASSERT_EQ(0, ioctx.operate(oid, &op2));
 
   entries.clear();
+  next_marker.clear();
 
   /* verify list doesn't show deferred entry (this may fail if cluster is thrashing) */
-  ASSERT_EQ(0, cls_rgw_gc_list(ioctx, oid, marker, 1, true, entries, &truncated));
+  ASSERT_EQ(0, cls_rgw_gc_list(ioctx, oid, marker, 1, true, entries, &truncated, next_marker));
   ASSERT_EQ(0, (int)entries.size());
   ASSERT_EQ(0, truncated);
 
   /* wait enough */
   sleep(5);
+  next_marker.clear();
 
   /* verify list shows deferred entry */
-  ASSERT_EQ(0, cls_rgw_gc_list(ioctx, oid, marker, 1, true, entries, &truncated));
+  ASSERT_EQ(0, cls_rgw_gc_list(ioctx, oid, marker, 1, true, entries, &truncated, next_marker));
   ASSERT_EQ(1, (int)entries.size());
   ASSERT_EQ(0, truncated);
 
@@ -532,9 +615,10 @@ TEST(cls_rgw, gc_defer)
   ASSERT_EQ(0, ioctx.operate(oid, &op3));
 
   entries.clear();
+  next_marker.clear();
 
   /* verify entry was removed */
-  ASSERT_EQ(0, cls_rgw_gc_list(ioctx, oid, marker, 1, true, entries, &truncated));
+  ASSERT_EQ(0, cls_rgw_gc_list(ioctx, oid, marker, 1, true, entries, &truncated, next_marker));
   ASSERT_EQ(0, (int)entries.size());
   ASSERT_EQ(0, truncated);