From 32f58e70d4c310f56ab30e136866d0ac172780ac Mon Sep 17 00:00:00 2001 From: Casey Bodley Date: Fri, 5 Jul 2024 18:01:11 -0400 Subject: [PATCH] cls/rgw: gc_list uses ObjectOperation instead of IoCtx clean up the only gc function that was hidden with CLS_CLIENT_HIDE_IOCTX this allows rgw to use it asynchonously with rgw_rados_operate() and optional_yield, and warn about blocking calls that should be async Signed-off-by: Casey Bodley --- src/cls/rgw/cls_rgw_client.cc | 17 ++++++++++------- src/cls/rgw/cls_rgw_client.h | 12 +++++------- src/rgw/driver/rados/rgw_gc.cc | 22 ++++++++++++++++++---- src/test/cls_rgw/test_cls_rgw.cc | 28 ++++++++++++++++++++-------- 4 files changed, 53 insertions(+), 26 deletions(-) diff --git a/src/cls/rgw/cls_rgw_client.cc b/src/cls/rgw/cls_rgw_client.cc index e65dedf14e42d..2dc204d9057b8 100644 --- a/src/cls/rgw/cls_rgw_client.cc +++ b/src/cls/rgw/cls_rgw_client.cc @@ -902,19 +902,22 @@ void cls_rgw_gc_defer_entry(ObjectWriteOperation& op, uint32_t expiration_secs, op.exec(RGW_CLASS, RGW_GC_DEFER_ENTRY, in); } -int cls_rgw_gc_list(IoCtx& io_ctx, string& oid, string& marker, uint32_t max, bool expired_only, - list& entries, bool *truncated, string& next_marker) +void cls_rgw_gc_list(ObjectReadOperation& op, const string& marker, + uint32_t max, bool expired_only, bufferlist& out) { - bufferlist in, out; + bufferlist in; cls_rgw_gc_list_op call; call.marker = marker; call.max = max; call.expired_only = expired_only; encode(call, in); - int r = io_ctx.exec(oid, RGW_CLASS, RGW_GC_LIST, in, out); - if (r < 0) - return r; + op.exec(RGW_CLASS, RGW_GC_LIST, in, &out, nullptr); +} +int cls_rgw_gc_list_decode(const bufferlist& out, + std::list& entries, + bool *truncated, std::string& next_marker) +{ cls_rgw_gc_list_ret ret; try { auto iter = out.cbegin(); @@ -928,7 +931,7 @@ int cls_rgw_gc_list(IoCtx& io_ctx, string& oid, string& marker, uint32_t max, bo if (truncated) *truncated = ret.truncated; next_marker = std::move(ret.next_marker); - return r; + return 0; } void cls_rgw_gc_remove(librados::ObjectWriteOperation& op, const vector& tags) diff --git a/src/cls/rgw/cls_rgw_client.h b/src/cls/rgw/cls_rgw_client.h index 365a51fb5d599..b708d5b51b3d7 100644 --- a/src/cls/rgw/cls_rgw_client.h +++ b/src/cls/rgw/cls_rgw_client.h @@ -600,13 +600,11 @@ void cls_rgw_usage_log_add(librados::ObjectWriteOperation& op, rgw_usage_log_inf void cls_rgw_gc_set_entry(librados::ObjectWriteOperation& op, uint32_t expiration_secs, cls_rgw_gc_obj_info& info); void cls_rgw_gc_defer_entry(librados::ObjectWriteOperation& op, uint32_t expiration_secs, const std::string& tag); void cls_rgw_gc_remove(librados::ObjectWriteOperation& op, const std::vector& tags); - -// these overloads which call io_ctx.operate() should not be called in the rgw. -// rgw_rados_operate() should be called after the overloads w/o calls to io_ctx.operate() -#ifndef CLS_CLIENT_HIDE_IOCTX -int cls_rgw_gc_list(librados::IoCtx& io_ctx, std::string& oid, std::string& marker, uint32_t max, bool expired_only, - std::list& entries, bool *truncated, std::string& next_marker); -#endif +void cls_rgw_gc_list(librados::ObjectReadOperation& op, const std::string& marker, + uint32_t max, bool expired_only, bufferlist& bl); +int cls_rgw_gc_list_decode(const bufferlist& bl, + std::list& entries, + bool *truncated, std::string& next_marker); /* lifecycle */ // these overloads which call io_ctx.operate() should not be called in the rgw. diff --git a/src/rgw/driver/rados/rgw_gc.cc b/src/rgw/driver/rados/rgw_gc.cc index dfa9e935dd983..7a4e22608bdd7 100644 --- a/src/rgw/driver/rados/rgw_gc.cc +++ b/src/rgw/driver/rados/rgw_gc.cc @@ -244,6 +244,20 @@ int RGWGC::remove(int index, int num_entries, optional_yield y) return store->gc_operate(this, obj_names[index], &op, y); } +static int gc_list(const DoutPrefixProvider* dpp, optional_yield y, librados::IoCtx& io_ctx, + std::string& oid, std::string& marker, uint32_t max, bool expired_only, + std::list& entries, bool *truncated, std::string& next_marker) +{ + librados::ObjectReadOperation op; + bufferlist bl; + cls_rgw_gc_list(op, marker, max, expired_only, bl); + int ret = rgw_rados_operate(dpp, io_ctx, oid, &op, nullptr, y); + if (ret < 0) { + return ret; + } + return cls_rgw_gc_list_decode(bl, entries, truncated, next_marker); +} + int RGWGC::list(int *index, string& marker, uint32_t max, bool expired_only, std::list& result, bool *truncated, bool& processing_queue) { result.clear(); @@ -256,7 +270,7 @@ int RGWGC::list(int *index, string& marker, uint32_t max, bool expired_only, std //processing_queue is set to true from previous iteration if the queue was under process and probably has more elements in it. if (! transitioned_objects_cache[*index] && ! check_queue && ! processing_queue) { - ret = cls_rgw_gc_list(store->gc_pool_ctx, obj_names[*index], marker, max - result.size(), expired_only, entries, truncated, next_marker); + ret = gc_list(this, null_yield, store->gc_pool_ctx, obj_names[*index], marker, max - result.size(), expired_only, entries, truncated, next_marker); if (ret != -ENOENT && ret < 0) { return ret; } @@ -271,7 +285,7 @@ int RGWGC::list(int *index, string& marker, uint32_t max, bool expired_only, std marker.clear(); } else { std::list non_expired_entries; - ret = cls_rgw_gc_list(store->gc_pool_ctx, obj_names[*index], marker, 1, false, non_expired_entries, truncated, next_marker); + ret = gc_list(this, null_yield, store->gc_pool_ctx, obj_names[*index], marker, 1, false, non_expired_entries, truncated, next_marker); if (non_expired_entries.size() == 0) { transitioned_objects_cache[*index] = true; marker.clear(); @@ -588,7 +602,7 @@ int RGWGC::process(int index, int max_secs, bool expired_only, int ret = 0; if (! transitioned_objects_cache[index]) { - ret = cls_rgw_gc_list(store->gc_pool_ctx, obj_names[index], marker, max, expired_only, entries, &truncated, next_marker); + ret = gc_list(this, y, store->gc_pool_ctx, obj_names[index], marker, max, expired_only, entries, &truncated, next_marker); ldpp_dout(this, 20) << "RGWGC::process cls_rgw_gc_list returned with returned:" << ret << ", entries.size=" << entries.size() << ", truncated=" << truncated << @@ -597,7 +611,7 @@ int RGWGC::process(int index, int max_secs, bool expired_only, cls_version_read(store->gc_pool_ctx, obj_names[index], &objv); if ((objv.ver == 1) && entries.size() == 0) { std::list non_expired_entries; - ret = cls_rgw_gc_list(store->gc_pool_ctx, obj_names[index], marker, 1, false, non_expired_entries, &truncated, next_marker); + ret = gc_list(this, y, store->gc_pool_ctx, obj_names[index], marker, 1, false, non_expired_entries, &truncated, next_marker); if (non_expired_entries.size() == 0) { transitioned_objects_cache[index] = true; marker.clear(); diff --git a/src/test/cls_rgw/test_cls_rgw.cc b/src/test/cls_rgw/test_cls_rgw.cc index a2f2fa66a7672..1235918457f7a 100644 --- a/src/test/cls_rgw/test_cls_rgw.cc +++ b/src/test/cls_rgw/test_cls_rgw.cc @@ -782,6 +782,18 @@ static bool cmp_objs(cls_rgw_obj& obj1, cls_rgw_obj& obj2) (obj1.loc == obj2.loc); } +static int gc_list(librados::IoCtx& io_ctx, std::string& oid, std::string& marker, uint32_t max, bool expired_only, + std::list& entries, bool *truncated, std::string& next_marker) +{ + librados::ObjectReadOperation op; + bufferlist bl; + cls_rgw_gc_list(op, marker, max, expired_only, bl); + int ret = io_ctx.operate(oid, &op, nullptr); + if (ret < 0) { + return ret; + } + return cls_rgw_gc_list_decode(bl, entries, truncated, next_marker); +} TEST_F(cls_rgw, gc_set) { @@ -814,7 +826,7 @@ TEST_F(cls_rgw, gc_set) 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(0, gc_list(ioctx, oid, marker, 8, true, entries, &truncated, next_marker)); ASSERT_EQ(8, (int)entries.size()); ASSERT_EQ(1, truncated); @@ -822,7 +834,7 @@ TEST_F(cls_rgw, gc_set) next_marker.clear(); /* list all chains, verify not truncated */ - ASSERT_EQ(0, cls_rgw_gc_list(ioctx, oid, marker, 10, true, entries, &truncated, next_marker)); + ASSERT_EQ(0, gc_list(ioctx, oid, marker, 10, true, entries, &truncated, next_marker)); ASSERT_EQ(10, (int)entries.size()); ASSERT_EQ(0, truncated); @@ -891,14 +903,14 @@ TEST_F(cls_rgw, gc_list) 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(0, 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(0, gc_list(ioctx, oid, marker, 8, true, entries2, &truncated, next_marker)); ASSERT_EQ(2, (int)entries2.size()); ASSERT_EQ(0, truncated); @@ -968,7 +980,7 @@ TEST_F(cls_rgw, gc_defer) string next_marker; /* list chains, verify num entries as expected */ - ASSERT_EQ(0, cls_rgw_gc_list(ioctx, oid, marker, 1, true, entries, &truncated, next_marker)); + ASSERT_EQ(0, gc_list(ioctx, oid, marker, 1, true, entries, &truncated, next_marker)); ASSERT_EQ(1, (int)entries.size()); ASSERT_EQ(0, truncated); @@ -982,7 +994,7 @@ TEST_F(cls_rgw, gc_defer) 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, next_marker)); + ASSERT_EQ(0, gc_list(ioctx, oid, marker, 1, true, entries, &truncated, next_marker)); ASSERT_EQ(0, (int)entries.size()); ASSERT_EQ(0, truncated); @@ -991,7 +1003,7 @@ TEST_F(cls_rgw, gc_defer) next_marker.clear(); /* verify list shows deferred entry */ - ASSERT_EQ(0, cls_rgw_gc_list(ioctx, oid, marker, 1, true, entries, &truncated, next_marker)); + ASSERT_EQ(0, gc_list(ioctx, oid, marker, 1, true, entries, &truncated, next_marker)); ASSERT_EQ(1, (int)entries.size()); ASSERT_EQ(0, truncated); @@ -1007,7 +1019,7 @@ TEST_F(cls_rgw, gc_defer) next_marker.clear(); /* verify entry was removed */ - ASSERT_EQ(0, cls_rgw_gc_list(ioctx, oid, marker, 1, true, entries, &truncated, next_marker)); + ASSERT_EQ(0, gc_list(ioctx, oid, marker, 1, true, entries, &truncated, next_marker)); ASSERT_EQ(0, (int)entries.size()); ASSERT_EQ(0, truncated); -- 2.39.5