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 <cbodley@redhat.com>

diff --git a/src/cls/rgw/cls_rgw_client.cc b/src/cls/rgw/cls_rgw_client.cc
index e65dedf14e42d45c1ab1b0720051a29f0cbc19ac..2dc204d9057b8e4dd9fa71235bd2c3917fe79a5e 100644 (file)
--- 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<cls_rgw_gc_obj_info>& 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<cls_rgw_gc_obj_info>& 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<string>& tags)

diff --git a/src/cls/rgw/cls_rgw_client.h b/src/cls/rgw/cls_rgw_client.h
index 365a51fb5d5997c30bfdd2a3bd4d018bef032d1f..b708d5b51b3d70eebfccba61c0e1a4cea0431f59 100644 (file)
--- 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<std::string>& 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<cls_rgw_gc_obj_info>& 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<cls_rgw_gc_obj_info>& 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 dfa9e935dd9838433b179b33d9182eaea6cb19c2..7a4e22608bdd7457e7ce19664952b9426e046d01 100644 (file)
--- 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<cls_rgw_gc_obj_info>& 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<cls_rgw_gc_obj_info>& 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<cls_rgw_gc_obj_info> 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<cls_rgw_gc_obj_info> 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 a2f2fa66a7672435ede5b72c650da33cc59e19f0..1235918457f7a8ba9998bcaba492c45bae7975a4 100644 (file)
--- 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<cls_rgw_gc_obj_info>& 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);