From 3a5647f748185e3a3fc2602a132874c0dfb24368 Mon Sep 17 00:00:00 2001 From: Abhishek Lekshmanan Date: Tue, 5 Mar 2019 09:30:57 +0100 Subject: [PATCH 01/11] rgw: drop entries only if the markers do not match. Signed-off-by: Abhishek Lekshmanan --- src/rgw/rgw_lc.cc | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/rgw/rgw_lc.cc b/src/rgw/rgw_lc.cc index 8abb73a27a9..268faf053ec 100644 --- a/src/rgw/rgw_lc.cc +++ b/src/rgw/rgw_lc.cc @@ -952,16 +952,17 @@ int RGWLC::bucket_lc_process(string& shard_id) boost::split(result, shard_id, boost::is_any_of(":")); string bucket_tenant = result[0]; string bucket_name = result[1]; - string bucket_id = result[2]; + string bucket_marker = result[2]; int ret = store->get_bucket_info(obj_ctx, bucket_tenant, bucket_name, bucket_info, NULL, &bucket_attrs); if (ret < 0) { ldpp_dout(this, 0) << "LC:get_bucket_info for " << bucket_name << " failed" << dendl; return ret; } - ret = bucket_info.bucket.bucket_id.compare(bucket_id) ; - if (ret != 0) { - ldpp_dout(this, 0) << "LC:old bucket id found. " << bucket_name << " should be deleted" << dendl; + if (bucket_info.bucket.marker != bucket_marker) { + ldpp_dout(this, 1) << "LC: deleting stale entry found for bucket=" << bucket_tenant + << ":" << bucket_name << " cur_marker=" << bucket_info.bucket.marker + << " orig_marker=" << bucket_marker << dendl; return -ENOENT; } From bf64aa843febf2f09aab54f9e0a24aa44f5938ea Mon Sep 17 00:00:00 2001 From: Abhishek Lekshmanan Date: Tue, 5 Mar 2019 10:46:23 +0100 Subject: [PATCH 02/11] rgw lc: use marker for the shard id Since buckets can undergo resharding which changes the bucket id, using the bucket marker in the shard id can help prevent the need to rewrite the entry as the buckets get resharded. This also helps detect the exit criteria when the bucket gets deleted. Signed-off-by: Abhishek Lekshmanan --- src/rgw/rgw_lc.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rgw/rgw_lc.cc b/src/rgw/rgw_lc.cc index 268faf053ec..2a779be0e46 100644 --- a/src/rgw/rgw_lc.cc +++ b/src/rgw/rgw_lc.cc @@ -1305,7 +1305,7 @@ template static int guard_lc_modify(RGWRados* store, const rgw_bucket& bucket, const string& cookie, const F& f) { CephContext *cct = store->ctx(); - string shard_id = bucket.tenant + ':' + bucket.name + ':' + bucket.bucket_id; + string shard_id = bucket.tenant + ':' + bucket.name + ':' + bucket.marker; string oid; get_lc_oid(cct, shard_id, &oid); From 197bc9b124e917c985863f45848c19beae2bd744 Mon Sep 17 00:00:00 2001 From: Abhishek Lekshmanan Date: Tue, 5 Mar 2019 13:34:05 +0100 Subject: [PATCH 03/11] cls_rgw: implement a read_omap_entry method Also refactor other methods that just read a single omap entry to use this method instead Signed-off-by: Abhishek Lekshmanan --- src/cls/rgw/cls_rgw.cc | 45 +++++++++++++++--------------------------- 1 file changed, 16 insertions(+), 29 deletions(-) diff --git a/src/cls/rgw/cls_rgw.cc b/src/cls/rgw/cls_rgw.cc index 21535eae61a..1f17f828689 100644 --- a/src/cls/rgw/cls_rgw.cc +++ b/src/cls/rgw/cls_rgw.cc @@ -756,7 +756,8 @@ static void log_entry(const char *func, const char *str, rgw_bucket_olh_entry *e } template -static int read_index_entry(cls_method_context_t hctx, string& name, T *entry) +static int read_omap_entry(cls_method_context_t hctx, const std::string& name, + T *entry) { bufferlist current_entry; int rc = cls_cxx_map_get_val(hctx, name, ¤t_entry); @@ -768,9 +769,19 @@ static int read_index_entry(cls_method_context_t hctx, string& name, T *entry) try { decode(*entry, cur_iter); } catch (buffer::error& err) { - CLS_LOG(1, "ERROR: read_index_entry(): failed to decode entry\n"); + CLS_LOG(1, "ERROR: %s(): failed to decode entry\n", __func__); return -EIO; } + return 0; +} + +template +static int read_index_entry(cls_method_context_t hctx, string& name, T *entry) +{ + int ret = read_omap_entry(hctx, name, entry); + if (ret < 0) { + return ret; + } log_entry(__func__, "existing entry", entry); return 0; @@ -3168,18 +3179,10 @@ static int gc_omap_get(cls_method_context_t hctx, int type, const string& key, c string index; prepend_index_prefix(key, type, &index); - bufferlist bl; - int ret = cls_cxx_map_get_val(hctx, index, &bl); + int ret = read_omap_entry(hctx, key, info); if (ret < 0) return ret; - try { - auto iter = bl.cbegin(); - decode(*info, iter); - } catch (buffer::error& err) { - CLS_LOG(0, "ERROR: rgw_cls_gc_omap_get(): failed to decode index=%s\n", index.c_str()); - } - return 0; } @@ -3677,22 +3680,6 @@ static int rgw_reshard_list(cls_method_context_t hctx, bufferlist *in, bufferlis return 0; } -static int get_reshard_entry(cls_method_context_t hctx, const string& key, cls_rgw_reshard_entry *entry) -{ - bufferlist bl; - int ret = cls_cxx_map_get_val(hctx, key, &bl); - if (ret < 0) - return ret; - auto iter = bl.cbegin(); - try { - decode(*entry, iter); - } catch (buffer::error& err) { - CLS_LOG(0, "ERROR: %s : failed to decode entry %s\n", __func__, err.what()); - return -EIO; - } - return 0; -} - static int rgw_reshard_get(cls_method_context_t hctx, bufferlist *in, bufferlist *out) { auto in_iter = in->cbegin(); @@ -3708,7 +3695,7 @@ static int rgw_reshard_get(cls_method_context_t hctx, bufferlist *in, bufferlis string key; cls_rgw_reshard_entry entry; op.entry.get_key(&key); - int ret = get_reshard_entry(hctx, key, &entry); + int ret = read_omap_entry(hctx, key, &entry); if (ret < 0) { return ret; } @@ -3734,7 +3721,7 @@ static int rgw_reshard_remove(cls_method_context_t hctx, bufferlist *in, bufferl string key; cls_rgw_reshard_entry entry; cls_rgw_reshard_entry::generate_key(op.tenant, op.bucket_name, &key); - int ret = get_reshard_entry(hctx, key, &entry); + int ret = read_omap_entry(hctx, key, &entry); if (ret < 0) { return ret; } From 9e1d2a28807fc709b5c9f86132609841e453ae45 Mon Sep 17 00:00:00 2001 From: Abhishek Lekshmanan Date: Tue, 5 Mar 2019 13:35:50 +0100 Subject: [PATCH 04/11] cls_rgw: alias the LC entries as rgw_lc_entry_t instead of a naked std::pair everywhere Signed-off-by: Abhishek Lekshmanan --- src/cls/rgw/cls_rgw_ops.h | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/cls/rgw/cls_rgw_ops.h b/src/cls/rgw/cls_rgw_ops.h index 78f8c692f32..30e82a9e20f 100644 --- a/src/cls/rgw/cls_rgw_ops.h +++ b/src/cls/rgw/cls_rgw_ops.h @@ -1010,9 +1010,10 @@ struct cls_rgw_lc_get_next_entry_op { }; WRITE_CLASS_ENCODER(cls_rgw_lc_get_next_entry_op) -struct cls_rgw_lc_get_next_entry_ret { - pair entry; +using rgw_lc_entry_t = std::pair; +struct cls_rgw_lc_get_next_entry_ret { + rgw_lc_entry_t entry; cls_rgw_lc_get_next_entry_ret() {} void encode(bufferlist& bl) const { @@ -1031,7 +1032,7 @@ struct cls_rgw_lc_get_next_entry_ret { WRITE_CLASS_ENCODER(cls_rgw_lc_get_next_entry_ret) struct cls_rgw_lc_rm_entry_op { - pair entry; + rgw_lc_entry_t entry; cls_rgw_lc_rm_entry_op() {} void encode(bufferlist& bl) const { @@ -1049,7 +1050,7 @@ struct cls_rgw_lc_rm_entry_op { WRITE_CLASS_ENCODER(cls_rgw_lc_rm_entry_op) struct cls_rgw_lc_set_entry_op { - pair entry; + rgw_lc_entry_t entry; cls_rgw_lc_set_entry_op() {} void encode(bufferlist& bl) const { From 64c5d6f880cf6d4d7a93cf4961f14f067d275736 Mon Sep 17 00:00:00 2001 From: Abhishek Lekshmanan Date: Tue, 5 Mar 2019 13:37:00 +0100 Subject: [PATCH 05/11] cls rgw: implement a method to get a single LC entry Signed-off-by: Abhishek Lekshmanan --- src/cls/rgw/cls_rgw.cc | 25 ++++++++++++++++++++++ src/cls/rgw/cls_rgw_client.cc | 23 ++++++++++++++++++++ src/cls/rgw/cls_rgw_client.h | 1 + src/cls/rgw/cls_rgw_const.h | 1 + src/cls/rgw/cls_rgw_ops.h | 40 +++++++++++++++++++++++++++++++++++ 5 files changed, 90 insertions(+) diff --git a/src/cls/rgw/cls_rgw.cc b/src/cls/rgw/cls_rgw.cc index 1f17f828689..1f57cceeb35 100644 --- a/src/cls/rgw/cls_rgw.cc +++ b/src/cls/rgw/cls_rgw.cc @@ -3469,6 +3469,29 @@ static int rgw_cls_gc_remove(cls_method_context_t hctx, bufferlist *in, bufferli return gc_remove(hctx, op.tags); } +static int rgw_cls_lc_get_entry(cls_method_context_t hctx, bufferlist *in, bufferlist *out) +{ + auto in_iter = in->cbegin(); + + cls_rgw_lc_get_entry_op op; + try { + decode(op, in_iter); + } catch (buffer::error& err) { + CLS_LOG(1, "ERROR: rgw_cls_lc_set_entry(): failed to decode entry\n"); + return -EINVAL; + } + + rgw_lc_entry_t lc_entry; + int ret = read_omap_entry(hctx, op.marker, &lc_entry); + if (ret < 0) + return ret; + + cls_rgw_lc_get_entry_ret op_ret(std::move(lc_entry)); + encode(op_ret, *out); + return 0; +} + + static int rgw_cls_lc_set_entry(cls_method_context_t hctx, bufferlist *in, bufferlist *out) { auto in_iter = in->cbegin(); @@ -3876,6 +3899,7 @@ CLS_INIT(rgw) cls_method_handle_t h_rgw_gc_set_entry; cls_method_handle_t h_rgw_gc_list; cls_method_handle_t h_rgw_gc_remove; + cls_method_handle_t h_rgw_lc_get_entry; cls_method_handle_t h_rgw_lc_set_entry; cls_method_handle_t h_rgw_lc_rm_entry; cls_method_handle_t h_rgw_lc_get_next_entry; @@ -3938,6 +3962,7 @@ CLS_INIT(rgw) cls_register_cxx_method(h_class, RGW_GC_REMOVE, CLS_METHOD_RD | CLS_METHOD_WR, rgw_cls_gc_remove, &h_rgw_gc_remove); /* lifecycle bucket list */ + cls_register_cxx_method(h_class, RGW_LC_GET_ENTRY, CLS_METHOD_RD, rgw_cls_lc_get_entry, &h_rgw_lc_get_entry); cls_register_cxx_method(h_class, RGW_LC_SET_ENTRY, CLS_METHOD_RD | CLS_METHOD_WR, rgw_cls_lc_set_entry, &h_rgw_lc_set_entry); cls_register_cxx_method(h_class, RGW_LC_RM_ENTRY, CLS_METHOD_RD | CLS_METHOD_WR, rgw_cls_lc_rm_entry, &h_rgw_lc_rm_entry); cls_register_cxx_method(h_class, RGW_LC_GET_NEXT_ENTRY, CLS_METHOD_RD, rgw_cls_lc_get_next_entry, &h_rgw_lc_get_next_entry); diff --git a/src/cls/rgw/cls_rgw_client.cc b/src/cls/rgw/cls_rgw_client.cc index c5fde8cf298..1f25068f9f3 100644 --- a/src/cls/rgw/cls_rgw_client.cc +++ b/src/cls/rgw/cls_rgw_client.cc @@ -816,6 +816,29 @@ int cls_rgw_lc_set_entry(IoCtx& io_ctx, const string& oid, const pair& entry); int cls_rgw_lc_rm_entry(librados::IoCtx& io_ctx, const string& oid, const pair& entry); int cls_rgw_lc_set_entry(librados::IoCtx& io_ctx, const string& oid, const pair& entry); +int cls_rgw_lc_get_entry(librados::IoCtx& io_ctx, const string& oid, const std::string& marker, rgw_lc_entry_t& entry); int cls_rgw_lc_list(librados::IoCtx& io_ctx, const string& oid, const string& marker, uint32_t max_entries, diff --git a/src/cls/rgw/cls_rgw_const.h b/src/cls/rgw/cls_rgw_const.h index 1d920abff8d..fc3537ea880 100644 --- a/src/cls/rgw/cls_rgw_const.h +++ b/src/cls/rgw/cls_rgw_const.h @@ -52,6 +52,7 @@ #define RGW_GC_REMOVE "gc_remove" /* lifecycle bucket list */ +#define RGW_LC_GET_ENTRY "lc_get_entry" #define RGW_LC_SET_ENTRY "lc_set_entry" #define RGW_LC_RM_ENTRY "lc_rm_entry" #define RGW_LC_GET_NEXT_ENTRY "lc_get_next_entry" diff --git a/src/cls/rgw/cls_rgw_ops.h b/src/cls/rgw/cls_rgw_ops.h index 30e82a9e20f..86f08bcf677 100644 --- a/src/cls/rgw/cls_rgw_ops.h +++ b/src/cls/rgw/cls_rgw_ops.h @@ -1031,6 +1031,46 @@ struct cls_rgw_lc_get_next_entry_ret { }; WRITE_CLASS_ENCODER(cls_rgw_lc_get_next_entry_ret) +struct cls_rgw_lc_get_entry_op { + string marker; + cls_rgw_lc_get_entry_op() {} + cls_rgw_lc_get_entry_op(const std::string& _marker) : marker(_marker) {} + + void encode(bufferlist& bl) const { + ENCODE_START(1, 1, bl); + encode(marker, bl); + ENCODE_FINISH(bl); + } + + void decode(bufferlist::const_iterator& bl) { + DECODE_START(1, bl); + decode(marker, bl); + DECODE_FINISH(bl); + } +}; +WRITE_CLASS_ENCODER(cls_rgw_lc_get_entry_op) + +struct cls_rgw_lc_get_entry_ret { + rgw_lc_entry_t entry; + cls_rgw_lc_get_entry_ret() {} + cls_rgw_lc_get_entry_ret(rgw_lc_entry_t&& _entry) : entry(std::move(_entry)) {} + + void encode(bufferlist& bl) const { + ENCODE_START(1, 1, bl); + encode(entry, bl); + ENCODE_FINISH(bl); + } + + void decode(bufferlist::const_iterator& bl) { + DECODE_START(1, bl); + decode(entry, bl); + DECODE_FINISH(bl); + } + +}; +WRITE_CLASS_ENCODER(cls_rgw_lc_get_entry_ret) + + struct cls_rgw_lc_rm_entry_op { rgw_lc_entry_t entry; cls_rgw_lc_rm_entry_op() {} From 4528db4b642c37f67b3338ce04979b1ba7772e3e Mon Sep 17 00:00:00 2001 From: Abhishek Lekshmanan Date: Mon, 4 Mar 2019 19:10:45 +0100 Subject: [PATCH 06/11] rgw admin: implement a lc fix option An radosgw-admin lc fix --bucket <> option is added which checks if the bucket entry exists in the corresponding lc shard and creates it if not. In case of resharded buckets not running a fixed rgw that writes/compares the marker this would write a new entry with the marker as the old entry would've already been deleted by a LC process. We currently don't cleanup the stale entry as it is assumed this would be picked up by the LC processor already or would be picked up in the next cycle. Signed-off-by: Abhishek Lekshmanan --- src/rgw/rgw_admin.cc | 21 +++++++++++ src/rgw/rgw_lc.cc | 60 +++++++++++++++++++++++++++++-- src/rgw/rgw_lc.h | 5 +++ src/test/cli/radosgw-admin/help.t | 1 + 4 files changed, 85 insertions(+), 2 deletions(-) diff --git a/src/rgw/rgw_admin.cc b/src/rgw/rgw_admin.cc index 4b457741c31..0c711c74fe9 100644 --- a/src/rgw/rgw_admin.cc +++ b/src/rgw/rgw_admin.cc @@ -193,6 +193,7 @@ void usage() cout << " lc list list all bucket lifecycle progress\n"; cout << " lc get get a lifecycle bucket configuration\n"; cout << " lc process manually process lifecycle\n"; + cout << " lc fix fix LC for a resharded bucket\n"; cout << " metadata get get metadata info\n"; cout << " metadata put put metadata info\n"; cout << " metadata rm remove metadata info\n"; @@ -435,6 +436,7 @@ enum { OPT_LC_LIST, OPT_LC_GET, OPT_LC_PROCESS, + OPT_LC_FIX, OPT_ORPHANS_FIND, OPT_ORPHANS_FINISH, OPT_ORPHANS_LIST_JOBS, @@ -888,6 +890,8 @@ static int get_cmd(const char *cmd, const char *prev_cmd, const char *prev_prev_ return OPT_LC_GET; if (strcmp(cmd, "process") == 0) return OPT_LC_PROCESS; + if (strcmp(cmd, "fix") == 0) + return OPT_LC_FIX; } else if (strcmp(prev_cmd, "orphans") == 0) { if (strcmp(cmd, "find") == 0) return OPT_ORPHANS_FIND; @@ -6578,6 +6582,23 @@ next: } } + + if (opt_cmd == OPT_LC_FIX) { + rgw_bucket bucket; + RGWBucketInfo bucket_info; + map attrs; + ret = init_bucket(tenant, bucket_name, bucket_id, bucket_info, bucket, &attrs); + if (ret < 0) { + cerr << "ERROR: could not init bucket: " << cpp_strerror(-ret) << std::endl; + return -ret; + } + + ret = rgw::lc::fix_lc_shard_entry(store, bucket_info, attrs); + if (ret < 0) { + cerr << "ERROR: fixing lc shard entry failed with" << cpp_strerror(-ret) << std::endl; + } + } + if (opt_cmd == OPT_ORPHANS_FIND) { if (!yes_i_really_mean_it) { cerr << "accidental removal of active objects can not be reversed; " diff --git a/src/rgw/rgw_lc.cc b/src/rgw/rgw_lc.cc index 2a779be0e46..a66f2a3f3f0 100644 --- a/src/rgw/rgw_lc.cc +++ b/src/rgw/rgw_lc.cc @@ -16,6 +16,7 @@ #include "rgw_common.h" #include "rgw_bucket.h" #include "rgw_lc.h" +#include "rgw_string.h" #include "services/svc_sys_obj.h" @@ -1290,7 +1291,7 @@ void RGWLifecycleConfiguration::generate_test_instances(list_conf->rgw_lc_max_objs > HASH_PRIME ? HASH_PRIME : cct->_conf->rgw_lc_max_objs); int index = ceph_str_hash_linux(shard_id.c_str(), shard_id.size()) % HASH_PRIME % max_objs; @@ -1301,11 +1302,17 @@ static void get_lc_oid(CephContext *cct, const string& shard_id, string *oid) return; } + + +static std::string get_lc_shard_name(const rgw_bucket& bucket){ + return string_join_reserve(':', bucket.tenant, bucket.name, bucket.marker); +} + template static int guard_lc_modify(RGWRados* store, const rgw_bucket& bucket, const string& cookie, const F& f) { CephContext *cct = store->ctx(); - string shard_id = bucket.tenant + ':' + bucket.name + ':' + bucket.marker; + string shard_id = get_lc_shard_name(bucket); string oid; get_lc_oid(cct, shard_id, &oid); @@ -1391,3 +1398,52 @@ int RGWLC::remove_bucket_config(RGWBucketInfo& bucket_info, return ret; } +namespace rgw::lc { + +int fix_lc_shard_entry(RGWRados* store, const RGWBucketInfo& bucket_info, + const map& battrs) +{ + if (auto aiter = battrs.find(RGW_ATTR_LC); + aiter == battrs.end()) { + return 0; // No entry, nothing to fix + } + + auto shard_name = get_lc_shard_name(bucket_info.bucket); + std::string lc_oid; + get_lc_oid(store->ctx(), shard_name, &lc_oid); + + rgw_lc_entry_t entry; + // There are multiple cases we need to encounter here + // 1. entry exists and is already set to marker, happens in plain buckets & newly resharded buckets + // 2. entry doesn't exist, which usually happens when reshard has happened prior to update and next LC process has already dropped the update + // 3. entry exists matching the current bucket id which was after a reshard (needs to be updated to the marker) + // We are not dropping the old marker here as that would be caught by the next LC process update + auto lc_pool_ctx = store->get_lc_pool_ctx(); + int ret = cls_rgw_lc_get_entry(*lc_pool_ctx, + lc_oid, shard_name, entry); + if (ret == 0) { + ldout(store->ctx(), 5) << "Entry already exists, nothing to do" << dendl; + return ret; // entry is already existing correctly set to marker + } + ldout(store->ctx(), 5) << "cls_rgw_lc_get_entry errored ret code=" << ret << dendl; + if (ret == -ENOENT) { + ldout(store->ctx(), 1) << "No entry for bucket=" << bucket_info.bucket.name + << " creating " << dendl; + // TODO: we have too many ppl making cookies like this! + char cookie_buf[COOKIE_LEN + 1]; + gen_rand_alphanumeric(store->ctx(), cookie_buf, sizeof(cookie_buf) - 1); + std::string cookie = cookie_buf; + + ret = guard_lc_modify(store, bucket_info.bucket, cookie, + [&lc_pool_ctx, &lc_oid](librados::IoCtx *ctx, const string& oid, + const pair& entry) { + return cls_rgw_lc_set_entry(*lc_pool_ctx, + lc_oid, entry); + }); + + } + + return ret; +} + +} diff --git a/src/rgw/rgw_lc.h b/src/rgw/rgw_lc.h index 01fc01c7005..3566f7c1ce9 100644 --- a/src/rgw/rgw_lc.h +++ b/src/rgw/rgw_lc.h @@ -502,6 +502,11 @@ class RGWLC : public DoutPrefixProvider { int handle_multipart_expiration(RGWRados::Bucket *target, const map& prefix_map); }; +namespace rgw::lc { +int fix_lc_shard_entry(RGWRados *store, const RGWBucketInfo& bucket_info, + const map& battrs); + +} // namespace rgw::lc #endif diff --git a/src/test/cli/radosgw-admin/help.t b/src/test/cli/radosgw-admin/help.t index e0079a19cb9..8fdacf9ebe2 100644 --- a/src/test/cli/radosgw-admin/help.t +++ b/src/test/cli/radosgw-admin/help.t @@ -115,6 +115,7 @@ lc list list all bucket lifecycle progress lc get get a lifecycle bucket configuration lc process manually process lifecycle + lc fix fix LC for a resharded bucket metadata get get metadata info metadata put put metadata info metadata rm remove metadata info From 767a16815b85e351eafb17fa369f580761e2f22a Mon Sep 17 00:00:00 2001 From: Abhishek Lekshmanan Date: Thu, 7 Mar 2019 14:12:49 +0100 Subject: [PATCH 07/11] rgw: add a fix_lc_shards AdminOp that can fix lc shards for all the buckets The output would be a list of bucket names and their fix status, currently buckets which do not need a fix will also be logged with a status of 0. Signed-off-by: Abhishek Lekshmanan --- src/rgw/rgw_bucket.cc | 82 +++++++++++++++++++++++++++++++++++++++++++ src/rgw/rgw_bucket.h | 2 ++ 2 files changed, 84 insertions(+) diff --git a/src/rgw/rgw_bucket.cc b/src/rgw/rgw_bucket.cc index d08494afc27..1285a5dde3e 100644 --- a/src/rgw/rgw_bucket.cc +++ b/src/rgw/rgw_bucket.cc @@ -31,6 +31,7 @@ // until everything is moved from rgw_common #include "rgw_common.h" #include "rgw_reshard.h" +#include "rgw_lc.h" #include "cls/user/cls_user_types.h" #define dout_context g_ceph_context @@ -1877,6 +1878,87 @@ int RGWBucketAdminOp::clear_stale_instances(RGWRados *store, return process_stale_instances(store, op_state, flusher, process_f); } +static int fix_single_bucket_lc(RGWRados *store, + const std::string& tenant_name, + const std::string& bucket_name) +{ + auto obj_ctx = store->svc.sysobj->init_obj_ctx(); + RGWBucketInfo bucket_info; + map bucket_attrs; + int ret = store->get_bucket_info(obj_ctx, tenant_name, bucket_name, + bucket_info, nullptr, &bucket_attrs); + if (ret < 0) { + // TODO: Should we handle the case where the bucket could've been removed between + // listing and fetching? + return ret; + } + + return rgw::lc::fix_lc_shard_entry(store, bucket_info, bucket_attrs); +} + +static void format_lc_status(Formatter* formatter, + const std::string& tenant_name, + const std::string& bucket_name, + int status) +{ + formatter->open_object_section("bucket_entry"); + std::string entry = tenant_name.empty() ? bucket_name : tenant_name + "/" + bucket_name; + formatter->dump_string("bucket", entry); + formatter->dump_int("status", status); + formatter->close_section(); // bucket_entry +} + +static void process_single_lc_entry(RGWRados *store, Formatter *formatter, + const std::string& tenant_name, + const std::string& bucket_name) +{ + int ret = fix_single_bucket_lc(store, tenant_name, bucket_name); + format_lc_status(formatter, tenant_name, bucket_name, -ret); +} + +int RGWBucketAdminOp::fix_lc_shards(RGWRados *store, + RGWBucketAdminOpState& op_state, + RGWFormatterFlusher& flusher) +{ + std::string marker; + void *handle; + Formatter *formatter = flusher.get_formatter(); + static constexpr auto default_max_keys = 1000; + + int ret = store->meta_mgr->list_keys_init("bucket", marker, &handle); + if (ret < 0) { + std::cerr << "ERROR: can't get key: " << cpp_strerror(-ret) << std::endl; + return ret; + } + + bool truncated; + if (const std::string& bucket_name = op_state.get_bucket_name(); + ! bucket_name.empty()) { + const rgw_user user_id = op_state.get_user_id(); + process_single_lc_entry(store, formatter, user_id.tenant, bucket_name); + } else { + formatter->open_array_section("lc_fix_status"); + do { + list keys; + ret = store->meta_mgr->list_keys_next(handle, default_max_keys, keys, &truncated); + if (ret < 0 && ret != -ENOENT) { + std::cerr << "ERROR: lists_keys_next(): " << cpp_strerror(-ret) << std::endl; + return ret; + } if (ret != -ENOENT) { + for (const auto &key:keys) { + auto [tenant_name, bucket_name] = split_tenant(key); + process_single_lc_entry(store, formatter, tenant_name, bucket_name); + } + } + formatter->flush(cout); // regularly flush every 1k entries + } while (truncated); + formatter->close_section(); // lc_fix_status + } + formatter->flush(cout); + return 0; + +} + void rgw_data_change::dump(Formatter *f) const { string type; diff --git a/src/rgw/rgw_bucket.h b/src/rgw/rgw_bucket.h index 2834fbf78d7..c0a94230b1a 100644 --- a/src/rgw/rgw_bucket.h +++ b/src/rgw/rgw_bucket.h @@ -365,6 +365,8 @@ public: static int clear_stale_instances(RGWRados *store, RGWBucketAdminOpState& op_state, RGWFormatterFlusher& flusher); + static int fix_lc_shards(RGWRados *store, RGWBucketAdminOpState& op_state, + RGWFormatterFlusher& flusher); }; From 582c1e7afb5f091d1cde0f1c33ee9aa31f3539e9 Mon Sep 17 00:00:00 2001 From: Abhishek Lekshmanan Date: Thu, 7 Mar 2019 14:44:27 +0100 Subject: [PATCH 08/11] rgw admin: use the new AdminOp to fix lc shards Since it can iterate over a list of buckets, also rename the command to lc reshard fix Signed-off-by: Abhishek Lekshmanan --- src/rgw/rgw_admin.cc | 22 ++++++++-------------- src/test/cli/radosgw-admin/help.t | 2 +- 2 files changed, 9 insertions(+), 15 deletions(-) diff --git a/src/rgw/rgw_admin.cc b/src/rgw/rgw_admin.cc index 0c711c74fe9..5833501bf8d 100644 --- a/src/rgw/rgw_admin.cc +++ b/src/rgw/rgw_admin.cc @@ -193,7 +193,7 @@ void usage() cout << " lc list list all bucket lifecycle progress\n"; cout << " lc get get a lifecycle bucket configuration\n"; cout << " lc process manually process lifecycle\n"; - cout << " lc fix fix LC for a resharded bucket\n"; + cout << " lc reshard fix fix LC for a resharded bucket\n"; cout << " metadata get get metadata info\n"; cout << " metadata put put metadata info\n"; cout << " metadata rm remove metadata info\n"; @@ -436,7 +436,7 @@ enum { OPT_LC_LIST, OPT_LC_GET, OPT_LC_PROCESS, - OPT_LC_FIX, + OPT_LC_RESHARD_FIX, OPT_ORPHANS_FIND, OPT_ORPHANS_FINISH, OPT_ORPHANS_LIST_JOBS, @@ -890,8 +890,10 @@ static int get_cmd(const char *cmd, const char *prev_cmd, const char *prev_prev_ return OPT_LC_GET; if (strcmp(cmd, "process") == 0) return OPT_LC_PROCESS; + } else if ((prev_prev_cmd && strcmp(prev_prev_cmd, "lc") == 0) && + strcmp(prev_cmd, "reshard") == 0) { if (strcmp(cmd, "fix") == 0) - return OPT_LC_FIX; + return OPT_LC_RESHARD_FIX; } else if (strcmp(prev_cmd, "orphans") == 0) { if (strcmp(cmd, "find") == 0) return OPT_ORPHANS_FIND; @@ -6583,20 +6585,12 @@ next: } - if (opt_cmd == OPT_LC_FIX) { - rgw_bucket bucket; - RGWBucketInfo bucket_info; - map attrs; - ret = init_bucket(tenant, bucket_name, bucket_id, bucket_info, bucket, &attrs); + if (opt_cmd == OPT_LC_RESHARD_FIX) { + ret = RGWBucketAdminOp::fix_lc_shards(store, bucket_op,f); if (ret < 0) { - cerr << "ERROR: could not init bucket: " << cpp_strerror(-ret) << std::endl; - return -ret; + cerr << "ERROR: listing stale instances" << cpp_strerror(-ret) << std::endl; } - ret = rgw::lc::fix_lc_shard_entry(store, bucket_info, attrs); - if (ret < 0) { - cerr << "ERROR: fixing lc shard entry failed with" << cpp_strerror(-ret) << std::endl; - } } if (opt_cmd == OPT_ORPHANS_FIND) { diff --git a/src/test/cli/radosgw-admin/help.t b/src/test/cli/radosgw-admin/help.t index 8fdacf9ebe2..e70c8588e46 100644 --- a/src/test/cli/radosgw-admin/help.t +++ b/src/test/cli/radosgw-admin/help.t @@ -115,7 +115,7 @@ lc list list all bucket lifecycle progress lc get get a lifecycle bucket configuration lc process manually process lifecycle - lc fix fix LC for a resharded bucket + lc reshard fix fix LC for a resharded bucket metadata get get metadata info metadata put put metadata info metadata rm remove metadata info From a7c42a81dcb3ca92d65afd027d4dfb0fa3c50d4e Mon Sep 17 00:00:00 2001 From: Abhishek Lekshmanan Date: Fri, 8 Mar 2019 12:10:30 +0100 Subject: [PATCH 09/11] rgw: lc fix: protect list_keys and formatter with a scope_guard raii for fun and profit! Signed-off-by: Abhishek Lekshmanan --- src/rgw/rgw_bucket.cc | 51 ++++++++++++++++++++++++------------------- 1 file changed, 29 insertions(+), 22 deletions(-) diff --git a/src/rgw/rgw_bucket.cc b/src/rgw/rgw_bucket.cc index 1285a5dde3e..7af7aaf86b9 100644 --- a/src/rgw/rgw_bucket.cc +++ b/src/rgw/rgw_bucket.cc @@ -1925,36 +1925,43 @@ int RGWBucketAdminOp::fix_lc_shards(RGWRados *store, Formatter *formatter = flusher.get_formatter(); static constexpr auto default_max_keys = 1000; - int ret = store->meta_mgr->list_keys_init("bucket", marker, &handle); - if (ret < 0) { - std::cerr << "ERROR: can't get key: " << cpp_strerror(-ret) << std::endl; - return ret; - } - bool truncated; if (const std::string& bucket_name = op_state.get_bucket_name(); ! bucket_name.empty()) { const rgw_user user_id = op_state.get_user_id(); process_single_lc_entry(store, formatter, user_id.tenant, bucket_name); + formatter->flush(cout); } else { - formatter->open_array_section("lc_fix_status"); - do { - list keys; - ret = store->meta_mgr->list_keys_next(handle, default_max_keys, keys, &truncated); - if (ret < 0 && ret != -ENOENT) { - std::cerr << "ERROR: lists_keys_next(): " << cpp_strerror(-ret) << std::endl; - return ret; - } if (ret != -ENOENT) { - for (const auto &key:keys) { - auto [tenant_name, bucket_name] = split_tenant(key); - process_single_lc_entry(store, formatter, tenant_name, bucket_name); + int ret = store->meta_mgr->list_keys_init("bucket", marker, &handle); + if (ret < 0) { + std::cerr << "ERROR: can't get key: " << cpp_strerror(-ret) << std::endl; + return ret; + } + + { + formatter->open_array_section("lc_fix_status"); + auto sg = make_scope_guard([&store, &handle, &formatter](){ + store->meta_mgr->list_keys_complete(handle); + formatter->close_section(); // lc_fix_status + formatter->flush(cout); + }); + do { + list keys; + ret = store->meta_mgr->list_keys_next(handle, default_max_keys, keys, &truncated); + if (ret < 0 && ret != -ENOENT) { + std::cerr << "ERROR: lists_keys_next(): " << cpp_strerror(-ret) << std::endl; + return ret; + } if (ret != -ENOENT) { + for (const auto &key:keys) { + auto [tenant_name, bucket_name] = split_tenant(key); + process_single_lc_entry(store, formatter, tenant_name, bucket_name); + } } - } - formatter->flush(cout); // regularly flush every 1k entries - } while (truncated); - formatter->close_section(); // lc_fix_status + formatter->flush(cout); // regularly flush every 1k entries + } while (truncated); + } + } - formatter->flush(cout); return 0; } From dee9ac22f19360ba6436e674c93baea9d97ca5da Mon Sep 17 00:00:00 2001 From: Abhishek Lekshmanan Date: Fri, 8 Mar 2019 16:57:28 +0100 Subject: [PATCH 10/11] doc: add troubleshooting notes on reshard admin clis Adding a note on LC fixes and reshard stale instance fixes Signed-off-by: Abhishek Lekshmanan --- doc/radosgw/dynamicresharding.rst | 42 +++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/doc/radosgw/dynamicresharding.rst b/doc/radosgw/dynamicresharding.rst index 4d51cd76ebc..cd1ebdce808 100644 --- a/doc/radosgw/dynamicresharding.rst +++ b/doc/radosgw/dynamicresharding.rst @@ -95,3 +95,45 @@ Manual bucket resharding :: # radosgw-admin bucket reshard --bucket --num-shards + + +Troubleshooting +=============== + +Clusters prior to Luminous 12.2.11 and Mimic 13.2.5 left behind stale bucket +instance entries that weren't automatically cleaned up. The issue also affected +LifeCycle policies which weren't applied to resharded buckets anymore. Both of +these issues can be worked around using a couple of radosgw-admin commands. + +Stale Instance Management +------------------------- + +:: + + # radosgw-admin reshard stale-instances list + +This lists the stale instances in a cluster that are ready to be cleaned up. +Please note that the cleanup of these instances should be done only on a single +site cluster. The cleanup can be done by the following command: + +:: + + # radosgw-admin reshard stale-instances rm + + +Lifecycle fixes +--------------- + +For clusters which had resharded instances, it is highly likely that the old +lifecycle processes would've flagged and deleted lifecycle processing as the +bucket instance changed during a reshard. While this is fixed for newer clusters +(from 13.2.6 and 12.2.12), older buckets which had lifecycle policies and +would've undergone reshard will have to be manually fixed by issuing the following command + +:: + + # radosgw-admin lc reshard fix --bucket {bucketname} + + +As a convenience wrapper, if the ``--bucket`` argument is dropped then this +command will try and fix LC policies for all the buckets in the cluster. From 35baf931327c4306a72562d41e2b8f7feca0dd24 Mon Sep 17 00:00:00 2001 From: Abhishek Lekshmanan Date: Thu, 14 Mar 2019 22:26:32 +0100 Subject: [PATCH 11/11] cls_rgw: fix issue with gc code using the wrong name Also use the correct ptr notation Signed-off-by: Abhishek Lekshmanan --- src/cls/rgw/cls_rgw.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/cls/rgw/cls_rgw.cc b/src/cls/rgw/cls_rgw.cc index 1f57cceeb35..43f74178f2b 100644 --- a/src/cls/rgw/cls_rgw.cc +++ b/src/cls/rgw/cls_rgw.cc @@ -757,7 +757,7 @@ static void log_entry(const char *func, const char *str, rgw_bucket_olh_entry *e template static int read_omap_entry(cls_method_context_t hctx, const std::string& name, - T *entry) + T* entry) { bufferlist current_entry; int rc = cls_cxx_map_get_val(hctx, name, ¤t_entry); @@ -776,7 +776,7 @@ static int read_omap_entry(cls_method_context_t hctx, const std::string& name, } template -static int read_index_entry(cls_method_context_t hctx, string& name, T *entry) +static int read_index_entry(cls_method_context_t hctx, string& name, T* entry) { int ret = read_omap_entry(hctx, name, entry); if (ret < 0) { @@ -3179,7 +3179,7 @@ static int gc_omap_get(cls_method_context_t hctx, int type, const string& key, c string index; prepend_index_prefix(key, type, &index); - int ret = read_omap_entry(hctx, key, info); + int ret = read_omap_entry(hctx, index, info); if (ret < 0) return ret;