rgw: fix radosgw-admin bucket rm with --purge-objects and --bypass-gc #18922
Conversation
src/rgw/rgw_bucket.cc
Outdated
| if (ret < 0) { | ||
| return ret; | ||
| } | ||
|
|
There was a problem hiding this comment.
why did this call need to move up? i think we want it to come after the list_objects/rgw_remove_object loop in case we pass delete_children=false, so we can return -ENOTEMPTY without mutating the bucket
src/rgw/rgw_bucket.cc
Outdated
| } | ||
|
|
||
| ret = rgw_bucket_sync_user_stats(store, bucket.tenant, info); | ||
| ret = rgw_bucket_sync_user_stats(store, bucket.tenant, bucket.name); |
There was a problem hiding this comment.
okay, i see that bucket.tenant was getting converted to rgw_user for the (user_id, bucket_info) overload. but the (tenant_name, bucket_name) overload has to call get_bucket_info() again to look up the info that we already have here. it looks like we can take a shortcut:
ret = rgw_bucket_sync_user_stats(store, info.owner, info);
src/vstart.sh
Outdated
| @@ -459,6 +459,8 @@ $extra_conf | |||
| rgw crypt s3 kms encryption keys = testkey-1=YmluCmJvb3N0CmJvb3N0LWJ1aWxkCmNlcGguY29uZgo= testkey-2=aWIKTWFrZWZpbGUKbWFuCm91dApzcmMKVGVzdGluZwo= | |||
| rgw crypt require ssl = false | |||
| rgw lc debug interval = 10 | |||
| rgw gc processor period = 10 | |||
| rgw gc obj min wait = 10 | |||
There was a problem hiding this comment.
i'd prefer not to include these changes
cbodley
changed the title
agutikov
force-pushed
the
fix-22122
branch
2 times, most recently
from
d268188
to
835ad93
Compare
|
Please, do not merge. |
|
Now seems working as expected. Btw, it's not very obvious that --rgw-gc-obj-min-wait should be passed to radosgw-admin bucket rm when not using option --bypass-gc In other case default value is 2 hours and seems that gc not working or seems for example that async senging chain to gc not finished.
|
src/rgw/rgw_bucket.cc
Outdated
| ret = abort_bucket_multiparts(store, cct, info, prefix, delimiter); | ||
| if (ret < 0) { | ||
| return ret; | ||
| } |
There was a problem hiding this comment.
cool, this covers the ENOTEMPTY case. but i'm still not sure why abort_bucket_multiparts() needs to happen before object removal now - some clarifying comments would help here
There was a problem hiding this comment.
Agree, It will not help with concurrent delete bucket and uploads either multipart or simple.
store->delete_bucket calls check_bucket_empty
but even this will not guarantee that no garbage (from running uploads) will left after bucket been deleted.
There was a problem hiding this comment.
right. this reminds me of some earlier discussions in #15630 (review)
that's a good point. we don't even know whether the rados client has sent that osd op before shutdown. it looks like |
Call RGWRados::delete_bucket() from rgw_remove_bucket_bypass_gc() instead of partial copy of code of RGWRados::delete_bucket(). Fix updating user stats after radosgw-admin bucket rm --purge-objects and --bypass-gc Due to rgw_user(const std::string& s) was called incorrect version of rgw_bucket_sync_user_stats(). Fixes: http://tracker.ceph.com/issues/22122 Fixes: http://tracker.ceph.com/issues/19959 Signed-off-by: Aleksei Gutikov <aleksey.gutikov@synesis.ru>
|
Please, remove DNM label |
1 similar comment
Call RGWRados::delete_bucket() from rgw_remove_bucket_bypass_gc()
instead of partial copy of code of RGWRados::delete_bucket().
Fixes: http://tracker.ceph.com/issues/22122
Due to rgw_user(const std::string& s) was called incorrect version of rgw_bucket_sync_user_stats().
Fixes: http://tracker.ceph.com/issues/19959