From 3338946fac6f3eca3530624cc587efa82cf5549e Mon Sep 17 00:00:00 2001 From: Shilpa Jagannath Date: Thu, 21 May 2020 18:03:01 +0530 Subject: [PATCH] rgw/dynamic-resharding: remove creation of new bucket instance Signed-off-by: Shilpa Jagannath --- src/cls/rgw/cls_rgw.cc | 2 +- src/cls/rgw/cls_rgw_types.cc | 7 +- src/cls/rgw/cls_rgw_types.h | 22 +++--- src/rgw/rgw_admin.cc | 4 +- src/rgw/rgw_common.cc | 2 +- src/rgw/rgw_reshard.cc | 137 ++++++++++++----------------------- src/rgw/rgw_reshard.h | 3 +- 7 files changed, 64 insertions(+), 113 deletions(-) diff --git a/src/cls/rgw/cls_rgw.cc b/src/cls/rgw/cls_rgw.cc index 57ecdc6929ebf..b5ab261a1afe0 100644 --- a/src/cls/rgw/cls_rgw.cc +++ b/src/cls/rgw/cls_rgw.cc @@ -3981,7 +3981,7 @@ static int rgw_set_bucket_resharding(cls_method_context_t hctx, bufferlist *in, return rc; } - header.new_instance.set_status(op.entry.new_bucket_instance_id, op.entry.num_shards, op.entry.reshard_status); + header.new_instance.set_status(op.entry.bucket_instance_id, op.entry.num_shards, op.entry.reshard_status); return write_bucket_header(hctx, &header); } diff --git a/src/cls/rgw/cls_rgw_types.cc b/src/cls/rgw/cls_rgw_types.cc index 2e306fe7e8471..fa4c6f9dc0cc7 100644 --- a/src/cls/rgw/cls_rgw_types.cc +++ b/src/cls/rgw/cls_rgw_types.cc @@ -720,10 +720,8 @@ void cls_rgw_reshard_entry::dump(Formatter *f) const encode_json("tenant", tenant, f); encode_json("bucket_name", bucket_name, f); encode_json("bucket_id", bucket_id, f); - encode_json("new_instance_id", new_instance_id, f); encode_json("old_num_shards", old_num_shards, f); encode_json("new_num_shards", new_num_shards, f); - } void cls_rgw_reshard_entry::generate_test_instances(list& ls) @@ -734,7 +732,6 @@ void cls_rgw_reshard_entry::generate_test_instances(list ls.back()->tenant = "tenant"; ls.back()->bucket_name = "bucket1"""; ls.back()->bucket_id = "bucket_id"; - ls.back()->new_instance_id = "new_instance_id"; ls.back()->old_num_shards = 8; ls.back()->new_num_shards = 64; } @@ -742,7 +739,7 @@ void cls_rgw_reshard_entry::generate_test_instances(list void cls_rgw_bucket_instance_entry::dump(Formatter *f) const { encode_json("reshard_status", to_string(reshard_status), f); - encode_json("new_bucket_instance_id", new_bucket_instance_id, f); + encode_json("bucket_instance_id", bucket_instance_id, f); encode_json("num_shards", num_shards, f); } @@ -753,7 +750,7 @@ void cls_rgw_bucket_instance_entry::generate_test_instances( ls.push_back(new cls_rgw_bucket_instance_entry); ls.push_back(new cls_rgw_bucket_instance_entry); ls.back()->reshard_status = RESHARD_STATUS::IN_PROGRESS; - ls.back()->new_bucket_instance_id = "new_instance_id"; + ls.back()->bucket_instance_id = "instance_id"; } void cls_rgw_lc_obj_head::dump(Formatter *f) const diff --git a/src/cls/rgw/cls_rgw_types.h b/src/cls/rgw/cls_rgw_types.h index 434feceeed370..e604c6555ada1 100644 --- a/src/cls/rgw/cls_rgw_types.h +++ b/src/cls/rgw/cls_rgw_types.h @@ -741,13 +741,13 @@ struct cls_rgw_bucket_instance_entry { using RESHARD_STATUS = cls_rgw_reshard_status; cls_rgw_reshard_status reshard_status{RESHARD_STATUS::NOT_RESHARDING}; - std::string new_bucket_instance_id; + std::string bucket_instance_id; int32_t num_shards{-1}; void encode(ceph::buffer::list& bl) const { ENCODE_START(1, 1, bl); encode((uint8_t)reshard_status, bl); - encode(new_bucket_instance_id, bl); + encode(bucket_instance_id, bl); encode(num_shards, bl); ENCODE_FINISH(bl); } @@ -757,7 +757,7 @@ struct cls_rgw_bucket_instance_entry { uint8_t s; decode(s, bl); reshard_status = (cls_rgw_reshard_status)s; - decode(new_bucket_instance_id, bl); + decode(bucket_instance_id, bl); decode(num_shards, bl); DECODE_FINISH(bl); } @@ -767,14 +767,13 @@ struct cls_rgw_bucket_instance_entry { void clear() { reshard_status = RESHARD_STATUS::NOT_RESHARDING; - new_bucket_instance_id.clear(); } - void set_status(const std::string& new_instance_id, + void set_status(const std::string& instance_id, int32_t new_num_shards, cls_rgw_reshard_status s) { reshard_status = s; - new_bucket_instance_id = new_instance_id; + bucket_instance_id = instance_id; num_shards = new_num_shards; } @@ -1246,31 +1245,32 @@ struct cls_rgw_reshard_entry std::string tenant; std::string bucket_name; std::string bucket_id; - std::string new_instance_id; uint32_t old_num_shards{0}; uint32_t new_num_shards{0}; cls_rgw_reshard_entry() {} void encode(ceph::buffer::list& bl) const { - ENCODE_START(1, 1, bl); + ENCODE_START(2, 1, bl); encode(time, bl); encode(tenant, bl); encode(bucket_name, bl); encode(bucket_id, bl); - encode(new_instance_id, bl); encode(old_num_shards, bl); encode(new_num_shards, bl); ENCODE_FINISH(bl); } void decode(ceph::buffer::list::const_iterator& bl) { - DECODE_START(1, bl); + DECODE_START(2, bl); decode(time, bl); decode(tenant, bl); decode(bucket_name, bl); decode(bucket_id, bl); - decode(new_instance_id, bl); + if (struct_v < 2) { + std::string new_instance_id; + decode(new_instance_id, bl); + } decode(old_num_shards, bl); decode(new_num_shards, bl); DECODE_FINISH(bl); diff --git a/src/rgw/rgw_admin.cc b/src/rgw/rgw_admin.cc index b78925dcbe900..a8dacfd2d2a1b 100644 --- a/src/rgw/rgw_admin.cc +++ b/src/rgw/rgw_admin.cc @@ -1101,8 +1101,8 @@ static void show_reshard_status( for (const auto& entry : status) { formatter->open_object_section("entry"); formatter->dump_string("reshard_status", to_string(entry.reshard_status)); - formatter->dump_string("new_bucket_instance_id", - entry.new_bucket_instance_id); + formatter->dump_string("bucket_instance_id", + entry.bucket_instance_id); formatter->dump_int("num_shards", entry.num_shards); formatter->close_section(); } diff --git a/src/rgw/rgw_common.cc b/src/rgw/rgw_common.cc index 57765bbfe922c..883bc31aaff31 100644 --- a/src/rgw/rgw_common.cc +++ b/src/rgw/rgw_common.cc @@ -2163,7 +2163,7 @@ void RGWBucketInfo::decode(bufferlist::const_iterator& bl) { decode(swift_versioning, bl); if (swift_versioning) { decode(swift_ver_location, bl); - } + } } if (struct_v >= 17) { decode(creation_time, bl); diff --git a/src/rgw/rgw_reshard.cc b/src/rgw/rgw_reshard.cc index c2c915ce193fe..a0ac79bffe66b 100644 --- a/src/rgw/rgw_reshard.cc +++ b/src/rgw/rgw_reshard.cc @@ -261,17 +261,12 @@ RGWBucketReshard::RGWBucketReshard(rgw::sal::RGWRadosStore *_store, int RGWBucketReshard::set_resharding_status(rgw::sal::RGWRadosStore* store, const RGWBucketInfo& bucket_info, - const string& new_instance_id, + const string& instance_id, int32_t num_shards, - cls_rgw_reshard_status status) + cls_rgw_reshard_status status) { - if (new_instance_id.empty()) { - ldout(store->ctx(), 0) << __func__ << " missing new bucket instance id" << dendl; - return -EINVAL; - } - cls_rgw_bucket_instance_entry instance_entry; - instance_entry.set_status(new_instance_id, num_shards, status); + instance_entry.set_status(instance_id, num_shards, status); int ret = store->getRados()->bucket_set_reshard(bucket_info, instance_entry); if (ret < 0) { @@ -329,7 +324,7 @@ int RGWBucketReshard::clear_index_shard_reshard_status(rgw::sal::RGWRadosStore* static int create_new_bucket_instance(rgw::sal::RGWRadosStore *store, int new_num_shards, - const RGWBucketInfo& bucket_info, + RGWBucketInfo& bucket_info, map& attrs, RGWBucketInfo& new_bucket_info, const DoutPrefixProvider *dpp) @@ -338,8 +333,7 @@ static int create_new_bucket_instance(rgw::sal::RGWRadosStore *store, store->getRados()->create_bucket_id(&new_bucket_info.bucket.bucket_id); - new_bucket_info.layout.current_index.layout.normal.num_shards = new_num_shards; - new_bucket_info.objv_tracker.clear(); + bucket_info.layout.resharding = rgw::BucketReshardState::None; new_bucket_info.new_bucket_instance_id.clear(); new_bucket_info.reshard_status = cls_rgw_reshard_status::NOT_RESHARDING; @@ -389,8 +383,8 @@ class BucketInfoReshardUpdate bool in_progress{false}; - int set_status(cls_rgw_reshard_status s, const DoutPrefixProvider *dpp) { - bucket_info.reshard_status = s; + int set_status(rgw::BucketReshardState s, const DoutPrefixProvider *dpp) { + bucket_info.layout.resharding = s; int ret = store->getRados()->put_bucket_instance_info(bucket_info, false, real_time(), &bucket_attrs, dpp); if (ret < 0) { ldout(store->ctx(), 0) << "ERROR: failed to write bucket info, ret=" << ret << dendl; @@ -403,15 +397,12 @@ public: BucketInfoReshardUpdate(const DoutPrefixProvider *_dpp, rgw::sal::RGWRadosStore *_store, RGWBucketInfo& _bucket_info, - map& _bucket_attrs, - const string& new_bucket_id) : + map& _bucket_attrs) : dpp(_dpp), store(_store), bucket_info(_bucket_info), bucket_attrs(_bucket_attrs) - { - bucket_info.new_bucket_instance_id = new_bucket_id; - } + {} ~BucketInfoReshardUpdate() { if (in_progress) { @@ -425,12 +416,12 @@ public: bucket_info.new_bucket_instance_id.clear(); // clears new_bucket_instance as well - set_status(cls_rgw_reshard_status::NOT_RESHARDING, dpp); + set_status(rgw::BucketReshardState::None, dpp); } } int start() { - int ret = set_status(cls_rgw_reshard_status::IN_PROGRESS, dpp); + int ret = set_status(rgw::BucketReshardState::InProgress, dpp); if (ret < 0) { return ret; } @@ -439,7 +430,7 @@ public: } int complete() { - int ret = set_status(cls_rgw_reshard_status::DONE, dpp); + int ret = set_status(rgw::BucketReshardState::None, dpp); if (ret < 0) { return ret; } @@ -528,7 +519,6 @@ int RGWBucketReshardLock::renew(const Clock::time_point& now) { int RGWBucketReshard::do_reshard(int num_shards, - RGWBucketInfo& new_bucket_info, int max_entries, bool verbose, ostream *out, @@ -536,13 +526,8 @@ int RGWBucketReshard::do_reshard(int num_shards, const DoutPrefixProvider *dpp) { if (out) { - const rgw_bucket& bucket = bucket_info.bucket; - (*out) << "tenant: " << bucket.tenant << std::endl; - (*out) << "bucket name: " << bucket.name << std::endl; - (*out) << "old bucket instance id: " << bucket.bucket_id << - std::endl; - (*out) << "new bucket instance id: " << new_bucket_info.bucket.bucket_id << - std::endl; + (*out) << "tenant: " << bucket_info.bucket.tenant << std::endl; + (*out) << "bucket name: " << bucket_info.bucket.name << std::endl; } /* update bucket info -- in progress*/ @@ -556,7 +541,7 @@ int RGWBucketReshard::do_reshard(int num_shards, // NB: destructor cleans up sharding state if reshard does not // complete successfully - BucketInfoReshardUpdate bucket_info_updater(dpp, store, bucket_info, bucket_attrs, new_bucket_info.bucket.bucket_id); + BucketInfoReshardUpdate bucket_info_updater(dpp, store, bucket_info, bucket_attrs); int ret = bucket_info_updater.start(); if (ret < 0) { @@ -564,9 +549,9 @@ int RGWBucketReshard::do_reshard(int num_shards, return ret; } - int num_target_shards = (new_bucket_info.layout.current_index.layout.normal.num_shards > 0 ? new_bucket_info.layout.current_index.layout.normal.num_shards : 1); + int num_target_shards = bucket_info.layout.target_index->layout.normal.num_shards; - BucketReshardManager target_shards_mgr(dpp, store, new_bucket_info, num_target_shards); + BucketReshardManager target_shards_mgr(dpp, store, bucket_info, num_target_shards); bool verbose_json_out = verbose && (formatter != nullptr) && (out != nullptr); @@ -613,13 +598,13 @@ int RGWBucketReshard::do_reshard(int num_shards, rgw_bucket_category_stats stats; bool account = entry.get_info(&cls_key, &category, &stats); rgw_obj_key key(cls_key); - rgw_obj obj(new_bucket_info.bucket, key); + rgw_obj obj(bucket_info.bucket, key); RGWMPObj mp; if (key.ns == RGW_OBJ_NS_MULTIPART && mp.from_meta(key.name)) { // place the multipart .meta object on the same shard as its head object obj.index_hash_source = mp.get_key(); } - int ret = store->getRados()->get_target_shard_id(new_bucket_info.layout.current_index.layout.normal, obj.get_hash_object(), &target_shard_id); + int ret = store->getRados()->get_target_shard_id(bucket_info.layout.target_index->layout.normal, obj.get_hash_object(), &target_shard_id); if (ret < 0) { lderr(store->ctx()) << "ERROR: get_target_shard_id() returned ret=" << ret << dendl; return ret; @@ -672,16 +657,13 @@ int RGWBucketReshard::do_reshard(int num_shards, return -EIO; } - ret = store->ctl()->bucket->link_bucket(new_bucket_info.owner, new_bucket_info.bucket, bucket_info.creation_time, null_yield, dpp); - if (ret < 0) { - lderr(store->ctx()) << "failed to link new bucket instance (bucket_id=" << new_bucket_info.bucket.bucket_id << ": " << cpp_strerror(-ret) << ")" << dendl; - return ret; - } - - ret = bucket_info_updater.complete(); + //overwrite current_index for the next reshard process + bucket_info.layout.current_index = *bucket_info.layout.target_index; + bucket_info.layout.resharding = rgw::BucketReshardState::None; + ret = store->getRados()->put_bucket_instance_info(bucket_info, false, real_time(), &bucket_attrs, dpp); if (ret < 0) { - ldout(store->ctx(), 0) << __func__ << ": failed to update bucket info ret=" << ret << dendl; - /* don't error out, reshard process succeeded */ + lderr(store->ctx()) << "ERROR: failed writing bucket instance info: " << dendl; + return ret; } return 0; @@ -707,27 +689,28 @@ int RGWBucketReshard::execute(int num_shards, int max_op_entries, RGWBucketInfo new_bucket_info; ret = create_new_bucket_instance(num_shards, new_bucket_info, dpp); if (ret < 0) { - // shard state is uncertain, but this will attempt to remove them anyway + return ret; goto error_out; } if (reshard_log) { - ret = reshard_log->update(bucket_info, new_bucket_info); + ret = reshard_log->update(bucket_info); if (ret < 0) { + return ret; goto error_out; } } // set resharding status of current bucket_info & shards with // information about planned resharding - ret = set_resharding_status(new_bucket_info.bucket.bucket_id, + ret = set_resharding_status(bucket_info.bucket.bucket_id, num_shards, cls_rgw_reshard_status::IN_PROGRESS); if (ret < 0) { + return ret; goto error_out; } ret = do_reshard(num_shards, - new_bucket_info, max_op_entries, verbose, out, formatter, dpp); if (ret < 0) { @@ -739,30 +722,20 @@ int RGWBucketReshard::execute(int num_shards, int max_op_entries, reshard_lock.unlock(); - // resharding successful, so remove old bucket index shards; use - // best effort and don't report out an error; the lock isn't needed - // at this point since all we're using a best effor to to remove old - // shard objects - ret = store->svc()->bi->clean_index(bucket_info); - if (ret < 0) { - lderr(store->ctx()) << "Error: " << __func__ << - " failed to clean up old shards; " << - "RGWRados::clean_bucket_index returned " << ret << dendl; - } + // resharding successful, so remove old bucket index shards; use + // best effort and don't report out an error; the lock isn't needed + // at this point since all we're using a best effor to to remove old + // shard objects - ret = store->ctl()->bucket->remove_bucket_instance_info(bucket_info.bucket, - bucket_info, null_yield, dpp); - if (ret < 0) { - lderr(store->ctx()) << "Error: " << __func__ << - " failed to clean old bucket info object \"" << - bucket_info.bucket.get_key() << - "\"created after successful resharding with error " << ret << dendl; + ret = store->svc()->bi->clean_index(bucket_info); + if (ret < 0) { + lderr(store->ctx()) << "Error: " << __func__ << + " failed to clean up old shards; " << + "RGWRados::clean_bucket_index returned " << ret << dendl; } ldout(store->ctx(), 1) << __func__ << - " INFO: reshard of bucket \"" << bucket_info.bucket.name << "\" from \"" << - bucket_info.bucket.get_key() << "\" to \"" << - new_bucket_info.bucket.get_key() << "\" completed successfully" << dendl; + " INFO: reshard of bucket \"" << bucket_info.bucket.name << "\" completed successfully" << dendl; return 0; @@ -770,26 +743,18 @@ error_out: reshard_lock.unlock(); + + //TODO: Cleanup failed incomplete resharding // since the real problem is the issue that led to this error code // path, we won't touch ret and instead use another variable to // temporarily error codes - int ret2 = store->svc()->bi->clean_index(new_bucket_info); + int ret2 = store->svc()->bi->clean_index(bucket_info); if (ret2 < 0) { lderr(store->ctx()) << "Error: " << __func__ << " failed to clean up shards from failed incomplete resharding; " << "RGWRados::clean_bucket_index returned " << ret2 << dendl; } - ret2 = store->ctl()->bucket->remove_bucket_instance_info(new_bucket_info.bucket, - new_bucket_info, - null_yield, dpp); - if (ret2 < 0) { - lderr(store->ctx()) << "Error: " << __func__ << - " failed to clean bucket info object \"" << - new_bucket_info.bucket.get_key() << - "\"created during incomplete resharding with error " << ret2 << dendl; - } - return ret; } // execute @@ -823,10 +788,6 @@ void RGWReshard::get_bucket_logshard_oid(const string& tenant, const string& buc int RGWReshard::add(cls_rgw_reshard_entry& entry) { - if (!store->svc()->zone->can_reshard()) { - ldout(store->ctx(), 20) << __func__ << " Resharding is disabled" << dendl; - return 0; - } string logshard_oid; @@ -843,7 +804,7 @@ int RGWReshard::add(cls_rgw_reshard_entry& entry) return 0; } -int RGWReshard::update(const RGWBucketInfo& bucket_info, const RGWBucketInfo& new_bucket_info) +int RGWReshard::update(const RGWBucketInfo& bucket_info) { cls_rgw_reshard_entry entry; entry.bucket_name = bucket_info.bucket.name; @@ -855,8 +816,6 @@ int RGWReshard::update(const RGWBucketInfo& bucket_info, const RGWBucketInfo& ne return ret; } - entry.new_instance_id = new_bucket_info.bucket.name + ":" + new_bucket_info.bucket.bucket_id; - ret = add(entry); if (ret < 0) { ldout(store->ctx(), 0) << __func__ << ":Error in updating entry bucket " << entry.bucket_name << ": " << @@ -1015,7 +974,6 @@ int RGWReshard::process_single_logshard(int logshard_num, const DoutPrefixProvid } for(auto& entry: entries) { // logshard entries - if(entry.new_instance_id.empty()) { ldout(store->ctx(), 20) << __func__ << " resharding " << entry.bucket_name << dendl; @@ -1062,6 +1020,7 @@ int RGWReshard::process_single_logshard(int logshard_num, const DoutPrefixProvid goto finished_entry; } + { RGWBucketReshard br(store, bucket_info, attrs, nullptr); ret = br.execute(entry.new_num_shards, max_entries, dpp, false, nullptr, nullptr, this); @@ -1082,8 +1041,8 @@ int RGWReshard::process_single_logshard(int logshard_num, const DoutPrefixProvid entry.bucket_name << " from resharding queue: " << cpp_strerror(-ret) << dendl; return ret; + } } - } // if new instance id is empty finished_entry: @@ -1115,10 +1074,6 @@ void RGWReshard::get_logshard_oid(int shard_num, string *logshard) int RGWReshard::process_all_logshards(const DoutPrefixProvider *dpp) { - if (!store->svc()->zone->can_reshard()) { - ldout(store->ctx(), 20) << __func__ << " Resharding is disabled" << dendl; - return 0; - } int ret = 0; for (int i = 0; i < num_logshards; i++) { diff --git a/src/rgw/rgw_reshard.h b/src/rgw/rgw_reshard.h index cbdff34b1c200..6a0ebced0d337 100644 --- a/src/rgw/rgw_reshard.h +++ b/src/rgw/rgw_reshard.h @@ -88,7 +88,6 @@ private: RGWBucketInfo& new_bucket_info, const DoutPrefixProvider *dpp); int do_reshard(int num_shards, - RGWBucketInfo& new_bucket_info, int max_entries, bool verbose, ostream *os, @@ -234,7 +233,7 @@ protected: public: RGWReshard(rgw::sal::RGWRadosStore* _store, bool _verbose = false, ostream *_out = nullptr, Formatter *_formatter = nullptr); int add(cls_rgw_reshard_entry& entry); - int update(const RGWBucketInfo& bucket_info, const RGWBucketInfo& new_bucket_info); + int update(const RGWBucketInfo& bucket_info); int get(cls_rgw_reshard_entry& entry); int remove(cls_rgw_reshard_entry& entry); int list(int logshard_num, string& marker, uint32_t max, std::list& entries, bool *is_truncated); -- 2.39.5