Skip to content

Commit 32f58e7

Browse files
cbodleycbodley
committed
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 <[email protected]>
1 parent a23254d commit 32f58e7

File tree

4 files changed

+53
-26
lines changed

4 files changed

+53
-26
lines changed

‎src/cls/rgw/cls_rgw_client.cc‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -902,19 +902,22 @@ void cls_rgw_gc_defer_entry(ObjectWriteOperation& op, uint32_t expiration_secs,
902902
op.exec(RGW_CLASS, RGW_GC_DEFER_ENTRY, in);
903903
}
904904

905-
int cls_rgw_gc_list(IoCtx& io_ctx, string& oid, string& marker, uint32_t max, bool expired_only,
906-
list<cls_rgw_gc_obj_info>& entries, bool *truncated, string& next_marker)
905+
void cls_rgw_gc_list(ObjectReadOperation& op, const string& marker,
906+
uint32_t max, bool expired_only, bufferlist& out)
907907
{
908-
bufferlist in, out;
908+
bufferlist in;
909909
cls_rgw_gc_list_op call;
910910
call.marker = marker;
911911
call.max = max;
912912
call.expired_only = expired_only;
913913
encode(call, in);
914-
int r = io_ctx.exec(oid, RGW_CLASS, RGW_GC_LIST, in, out);
915-
if (r < 0)
916-
return r;
914+
op.exec(RGW_CLASS, RGW_GC_LIST, in, &out, nullptr);
915+
}
917916

917+
int cls_rgw_gc_list_decode(const bufferlist& out,
918+
std::list<cls_rgw_gc_obj_info>& entries,
919+
bool *truncated, std::string& next_marker)
920+
{
918921
cls_rgw_gc_list_ret ret;
919922
try {
920923
auto iter = out.cbegin();
@@ -928,7 +931,7 @@ int cls_rgw_gc_list(IoCtx& io_ctx, string& oid, string& marker, uint32_t max, bo
928931
if (truncated)
929932
*truncated = ret.truncated;
930933
next_marker = std::move(ret.next_marker);
931-
return r;
934+
return 0;
932935
}
933936

934937
void cls_rgw_gc_remove(librados::ObjectWriteOperation& op, const vector<string>& tags)

‎src/cls/rgw/cls_rgw_client.h‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -600,13 +600,11 @@ void cls_rgw_usage_log_add(librados::ObjectWriteOperation& op, rgw_usage_log_inf
600600
void cls_rgw_gc_set_entry(librados::ObjectWriteOperation& op, uint32_t expiration_secs, cls_rgw_gc_obj_info& info);
601601
void cls_rgw_gc_defer_entry(librados::ObjectWriteOperation& op, uint32_t expiration_secs, const std::string& tag);
602602
void cls_rgw_gc_remove(librados::ObjectWriteOperation& op, const std::vector<std::string>& tags);
603-
604-
// these overloads which call io_ctx.operate() should not be called in the rgw.
605-
// rgw_rados_operate() should be called after the overloads w/o calls to io_ctx.operate()
606-
#ifndef CLS_CLIENT_HIDE_IOCTX
607-
int cls_rgw_gc_list(librados::IoCtx& io_ctx, std::string& oid, std::string& marker, uint32_t max, bool expired_only,
608-
std::list<cls_rgw_gc_obj_info>& entries, bool *truncated, std::string& next_marker);
609-
#endif
603+
void cls_rgw_gc_list(librados::ObjectReadOperation& op, const std::string& marker,
604+
uint32_t max, bool expired_only, bufferlist& bl);
605+
int cls_rgw_gc_list_decode(const bufferlist& bl,
606+
std::list<cls_rgw_gc_obj_info>& entries,
607+
bool *truncated, std::string& next_marker);
610608

611609
/* lifecycle */
612610
// these overloads which call io_ctx.operate() should not be called in the rgw.

‎src/rgw/driver/rados/rgw_gc.cc‎

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -244,6 +244,20 @@ int RGWGC::remove(int index, int num_entries, optional_yield y)
244244
return store->gc_operate(this, obj_names[index], &op, y);
245245
}
246246

247+
static int gc_list(const DoutPrefixProvider* dpp, optional_yield y, librados::IoCtx& io_ctx,
248+
std::string& oid, std::string& marker, uint32_t max, bool expired_only,
249+
std::list<cls_rgw_gc_obj_info>& entries, bool *truncated, std::string& next_marker)
250+
{
251+
librados::ObjectReadOperation op;
252+
bufferlist bl;
253+
cls_rgw_gc_list(op, marker, max, expired_only, bl);
254+
int ret = rgw_rados_operate(dpp, io_ctx, oid, &op, nullptr, y);
255+
if (ret < 0) {
256+
return ret;
257+
}
258+
return cls_rgw_gc_list_decode(bl, entries, truncated, next_marker);
259+
}
260+
247261
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)
248262
{
249263
result.clear();
@@ -256,7 +270,7 @@ int RGWGC::list(int *index, string& marker, uint32_t max, bool expired_only, std
256270

257271
//processing_queue is set to true from previous iteration if the queue was under process and probably has more elements in it.
258272
if (! transitioned_objects_cache[*index] && ! check_queue && ! processing_queue) {
259-
ret = cls_rgw_gc_list(store->gc_pool_ctx, obj_names[*index], marker, max - result.size(), expired_only, entries, truncated, next_marker);
273+
ret = gc_list(this, null_yield, store->gc_pool_ctx, obj_names[*index], marker, max - result.size(), expired_only, entries, truncated, next_marker);
260274
if (ret != -ENOENT && ret < 0) {
261275
return ret;
262276
}
@@ -271,7 +285,7 @@ int RGWGC::list(int *index, string& marker, uint32_t max, bool expired_only, std
271285
marker.clear();
272286
} else {
273287
std::list<cls_rgw_gc_obj_info> non_expired_entries;
274-
ret = cls_rgw_gc_list(store->gc_pool_ctx, obj_names[*index], marker, 1, false, non_expired_entries, truncated, next_marker);
288+
ret = gc_list(this, null_yield, store->gc_pool_ctx, obj_names[*index], marker, 1, false, non_expired_entries, truncated, next_marker);
275289
if (non_expired_entries.size() == 0) {
276290
transitioned_objects_cache[*index] = true;
277291
marker.clear();
@@ -588,7 +602,7 @@ int RGWGC::process(int index, int max_secs, bool expired_only,
588602
int ret = 0;
589603

590604
if (! transitioned_objects_cache[index]) {
591-
ret = cls_rgw_gc_list(store->gc_pool_ctx, obj_names[index], marker, max, expired_only, entries, &truncated, next_marker);
605+
ret = gc_list(this, y, store->gc_pool_ctx, obj_names[index], marker, max, expired_only, entries, &truncated, next_marker);
592606
ldpp_dout(this, 20) <<
593607
"RGWGC::process cls_rgw_gc_list returned with returned:" << ret <<
594608
", entries.size=" << entries.size() << ", truncated=" << truncated <<
@@ -597,7 +611,7 @@ int RGWGC::process(int index, int max_secs, bool expired_only,
597611
cls_version_read(store->gc_pool_ctx, obj_names[index], &objv);
598612
if ((objv.ver == 1) && entries.size() == 0) {
599613
std::list<cls_rgw_gc_obj_info> non_expired_entries;
600-
ret = cls_rgw_gc_list(store->gc_pool_ctx, obj_names[index], marker, 1, false, non_expired_entries, &truncated, next_marker);
614+
ret = gc_list(this, y, store->gc_pool_ctx, obj_names[index], marker, 1, false, non_expired_entries, &truncated, next_marker);
601615
if (non_expired_entries.size() == 0) {
602616
transitioned_objects_cache[index] = true;
603617
marker.clear();

‎src/test/cls_rgw/test_cls_rgw.cc‎

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -782,6 +782,18 @@ static bool cmp_objs(cls_rgw_obj& obj1, cls_rgw_obj& obj2)
782782
(obj1.loc == obj2.loc);
783783
}
784784

785+
static int gc_list(librados::IoCtx& io_ctx, std::string& oid, std::string& marker, uint32_t max, bool expired_only,
786+
std::list<cls_rgw_gc_obj_info>& entries, bool *truncated, std::string& next_marker)
787+
{
788+
librados::ObjectReadOperation op;
789+
bufferlist bl;
790+
cls_rgw_gc_list(op, marker, max, expired_only, bl);
791+
int ret = io_ctx.operate(oid, &op, nullptr);
792+
if (ret < 0) {
793+
return ret;
794+
}
795+
return cls_rgw_gc_list_decode(bl, entries, truncated, next_marker);
796+
}
785797

786798
TEST_F(cls_rgw, gc_set)
787799
{
@@ -814,15 +826,15 @@ TEST_F(cls_rgw, gc_set)
814826
string next_marker;
815827

816828
/* list chains, verify truncated */
817-
ASSERT_EQ(0, cls_rgw_gc_list(ioctx, oid, marker, 8, true, entries, &truncated, next_marker));
829+
ASSERT_EQ(0, gc_list(ioctx, oid, marker, 8, true, entries, &truncated, next_marker));
818830
ASSERT_EQ(8, (int)entries.size());
819831
ASSERT_EQ(1, truncated);
820832

821833
entries.clear();
822834
next_marker.clear();
823835

824836
/* list all chains, verify not truncated */
825-
ASSERT_EQ(0, cls_rgw_gc_list(ioctx, oid, marker, 10, true, entries, &truncated, next_marker));
837+
ASSERT_EQ(0, gc_list(ioctx, oid, marker, 10, true, entries, &truncated, next_marker));
826838
ASSERT_EQ(10, (int)entries.size());
827839
ASSERT_EQ(0, truncated);
828840

@@ -891,14 +903,14 @@ TEST_F(cls_rgw, gc_list)
891903
string next_marker;
892904

893905
/* list chains, verify truncated */
894-
ASSERT_EQ(0, cls_rgw_gc_list(ioctx, oid, marker, 8, true, entries, &truncated, next_marker));
906+
ASSERT_EQ(0, gc_list(ioctx, oid, marker, 8, true, entries, &truncated, next_marker));
895907
ASSERT_EQ(8, (int)entries.size());
896908
ASSERT_EQ(1, truncated);
897909

898910
marker = next_marker;
899911
next_marker.clear();
900912

901-
ASSERT_EQ(0, cls_rgw_gc_list(ioctx, oid, marker, 8, true, entries2, &truncated, next_marker));
913+
ASSERT_EQ(0, gc_list(ioctx, oid, marker, 8, true, entries2, &truncated, next_marker));
902914
ASSERT_EQ(2, (int)entries2.size());
903915
ASSERT_EQ(0, truncated);
904916

@@ -968,7 +980,7 @@ TEST_F(cls_rgw, gc_defer)
968980
string next_marker;
969981

970982
/* list chains, verify num entries as expected */
971-
ASSERT_EQ(0, cls_rgw_gc_list(ioctx, oid, marker, 1, true, entries, &truncated, next_marker));
983+
ASSERT_EQ(0, gc_list(ioctx, oid, marker, 1, true, entries, &truncated, next_marker));
972984
ASSERT_EQ(1, (int)entries.size());
973985
ASSERT_EQ(0, truncated);
974986

@@ -982,7 +994,7 @@ TEST_F(cls_rgw, gc_defer)
982994
next_marker.clear();
983995

984996
/* verify list doesn't show deferred entry (this may fail if cluster is thrashing) */
985-
ASSERT_EQ(0, cls_rgw_gc_list(ioctx, oid, marker, 1, true, entries, &truncated, next_marker));
997+
ASSERT_EQ(0, gc_list(ioctx, oid, marker, 1, true, entries, &truncated, next_marker));
986998
ASSERT_EQ(0, (int)entries.size());
987999
ASSERT_EQ(0, truncated);
9881000

@@ -991,7 +1003,7 @@ TEST_F(cls_rgw, gc_defer)
9911003
next_marker.clear();
9921004

9931005
/* verify list shows deferred entry */
994-
ASSERT_EQ(0, cls_rgw_gc_list(ioctx, oid, marker, 1, true, entries, &truncated, next_marker));
1006+
ASSERT_EQ(0, gc_list(ioctx, oid, marker, 1, true, entries, &truncated, next_marker));
9951007
ASSERT_EQ(1, (int)entries.size());
9961008
ASSERT_EQ(0, truncated);
9971009

@@ -1007,7 +1019,7 @@ TEST_F(cls_rgw, gc_defer)
10071019
next_marker.clear();
10081020

10091021
/* verify entry was removed */
1010-
ASSERT_EQ(0, cls_rgw_gc_list(ioctx, oid, marker, 1, true, entries, &truncated, next_marker));
1022+
ASSERT_EQ(0, gc_list(ioctx, oid, marker, 1, true, entries, &truncated, next_marker));
10111023
ASSERT_EQ(0, (int)entries.size());
10121024
ASSERT_EQ(0, truncated);
10131025

0 commit comments

Comments
 (0)