From: lu.shasha Date: Thu, 5 Jan 2017 03:50:42 +0000 (+0800) Subject: rgw: fix 'gc list --include-all' command infinite loop the first 1000 items X-Git-Tag: v11.2.1~48^2 X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=91569f63852f12d958175bf970967fa9d25b2cfc;p=ceph.git 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 Signed-off-by: Shasha Lu (cherry picked from commit fc29f52ebca63104a05515484088ff136dfb0b15) --- diff --git a/src/cls/rgw/cls_rgw.cc b/src/cls/rgw/cls_rgw.cc index 9873590487bc2..b568a931e0f3e 100644 --- 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& entries, bool *truncated) + list& 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 437c888098c70..60b4ccf051bd1 100644 --- 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& entries, bool *truncated) + list& 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& tags) diff --git a/src/cls/rgw/cls_rgw_client.h b/src/cls/rgw/cls_rgw_client.h index be7106a6c34f9..7601fa52a90f7 100644 --- 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& entries, bool *truncated); + list& entries, bool *truncated, string& next_marker); void cls_rgw_gc_remove(librados::ObjectWriteOperation& op, const list& tags); diff --git a/src/cls/rgw/cls_rgw_ops.cc b/src/cls/rgw/cls_rgw_ops.cc index 0b3701bf9ff31..e500c62427a6a 100644 --- 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& 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 74035929387b4..5b3a17486d3e8 100644 --- 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 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 a5f5d395055b3..f37a4c799b616 100644 --- a/src/rgw/rgw_gc.cc +++ b/src/rgw/rgw_gc.cc @@ -94,10 +94,11 @@ int RGWGC::remove(int index, const std::list& tags) int RGWGC::list(int *index, string& marker, uint32_t max, bool expired_only, std::list& result, bool *truncated) { result.clear(); + string next_marker; for (; *index < max_objs && result.size() < max; (*index)++, marker.clear()) { std::list 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 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 22f137dd284f9..a15a033db0e63 100644 --- 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 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 entries; + list 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::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::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 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);