From f70932f0eb41aab74058f727e1f96f5328d18f78 Mon Sep 17 00:00:00 2001 From: Shilpa Jagannath Date: Tue, 14 Jul 2020 13:03:31 +0530 Subject: [PATCH] rgw: allow clean_index to take const bucket_index_layout_generation& fix bi_get() to get objects after being resharded Signed-off-by: Shilpa Jagannath --- src/rgw/rgw_rados.cc | 4 ++-- src/rgw/rgw_reshard.cc | 27 ++++++++++++++------------- src/rgw/services/svc_bi.h | 2 +- src/rgw/services/svc_bi_rados.cc | 17 +++++++++++------ src/rgw/services/svc_bi_rados.h | 8 +++----- 5 files changed, 31 insertions(+), 27 deletions(-) diff --git a/src/rgw/rgw_rados.cc b/src/rgw/rgw_rados.cc index ee6481fa7f3cb..0a80cded1056a 100644 --- a/src/rgw/rgw_rados.cc +++ b/src/rgw/rgw_rados.cc @@ -2399,8 +2399,8 @@ int RGWRados::create_bucket(const RGWUserInfo& owner, rgw_bucket& bucket, /* only remove it if it's a different bucket instance */ if (orig_info.bucket.bucket_id != bucket.bucket_id) { - int r = svc.bi->clean_index(dpp, info, info.layout.current_index.gen); - if (r < 0) { + int r = svc.bi->clean_index(dpp, info, info.layout.current_index); + if (r < 0) { ldpp_dout(dpp, 0) << "WARNING: could not remove bucket index (r=" << r << ")" << dendl; } r = ctl.bucket->remove_bucket_instance_info(info.bucket, info, null_yield, dpp); diff --git a/src/rgw/rgw_reshard.cc b/src/rgw/rgw_reshard.cc index 1bad304ecfd3a..5c1e28018a02d 100644 --- a/src/rgw/rgw_reshard.cc +++ b/src/rgw/rgw_reshard.cc @@ -659,6 +659,7 @@ int RGWBucketReshard::do_reshard(int num_shards, } //overwrite current_index for the next reshard process + const auto prev_index = bucket_info.layout.current_index; bucket_info.layout.current_index = *bucket_info.layout.target_index; bucket_info.layout.target_index = std::nullopt; // target_layout doesn't need to exist after reshard bucket_info.layout.resharding = rgw::BucketReshardState::None; @@ -673,6 +674,18 @@ int RGWBucketReshard::do_reshard(int num_shards, return ret; } + // 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(dpp, bucket_info, prev_index); + if (ret < 0) { + ldpp_dout(dpp, -1) << "Error: " << __func__ << + " failed to clean up old shards; " << + "RGWRados::clean_bucket_index returned " << ret << dendl; + } + return 0; // NB: some error clean-up is done by ~BucketInfoReshardUpdate } // RGWBucketReshard::do_reshard @@ -718,18 +731,6 @@ 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 effort to remove old - // shard objects - - ret = store->svc()->bi->clean_index(dpp, bucket_info, bucket_info.layout.current_index.gen); - if (ret < 0) { - lderr(store->ctx()) << "Error: " << __func__ << - " failed to clean up old shards; " << - "RGWRados::clean_bucket_index returned " << ret << dendl; - } - ldpp_dout(dpp, 1) << __func__ << " INFO: reshard of bucket \"" << bucket_info.bucket.name << "\" completed successfully" << dendl; @@ -744,7 +745,7 @@ error_out: // 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(dpp, bucket_info, bucket_info.layout.target_index->gen); + int ret2 = store->svc()->bi->clean_index(dpp, bucket_info, bucket_info.layout.current_index); if (ret2 < 0) { ldpp_dout(dpp, -1) << "Error: " << __func__ << " failed to clean up shards from failed incomplete resharding; " << diff --git a/src/rgw/services/svc_bi.h b/src/rgw/services/svc_bi.h index 9005c9dc595c0..0458901f4ba3a 100644 --- a/src/rgw/services/svc_bi.h +++ b/src/rgw/services/svc_bi.h @@ -30,7 +30,7 @@ public: virtual ~RGWSI_BucketIndex() {} virtual int init_index(const DoutPrefixProvider *dpp, RGWBucketInfo& bucket_info, const rgw::bucket_index_layout_generation& idx_layout) = 0; - virtual int clean_index(const DoutPrefixProvider *dpp, RGWBucketInfo& bucket_info, uint64_t gen) = 0; + virtual int clean_index(const DoutPrefixProvider *dpp, RGWBucketInfo& bucket_info, const rgw::bucket_index_layout_generation& idx_layout) = 0; virtual int read_stats(const DoutPrefixProvider *dpp, const RGWBucketInfo& bucket_info, diff --git a/src/rgw/services/svc_bi_rados.cc b/src/rgw/services/svc_bi_rados.cc index 98bcc7d318d92..b5c2c10c264c7 100644 --- a/src/rgw/services/svc_bi_rados.cc +++ b/src/rgw/services/svc_bi_rados.cc @@ -232,7 +232,7 @@ void RGWSI_BucketIndex_RADOS::get_bucket_index_object(const string& bucket_oid_b int RGWSI_BucketIndex_RADOS::get_bucket_index_object(const string& bucket_oid_base, const string& obj_key, uint32_t num_shards, rgw::BucketHashType hash_type, - string *bucket_obj, int *shard_id) + uint64_t gen_id, string *bucket_obj, int *shard_id) { int r = 0; switch (hash_type) { @@ -245,8 +245,12 @@ int RGWSI_BucketIndex_RADOS::get_bucket_index_object(const string& bucket_oid_ba } } else { uint32_t sid = bucket_shard_index(obj_key, num_shards); - char buf[bucket_oid_base.size() + 32]; - snprintf(buf, sizeof(buf), "%s.%d", bucket_oid_base.c_str(), sid); + char buf[bucket_oid_base.size() + 64]; + if (gen_id) { + snprintf(buf, sizeof(buf), "%s.%" PRIu64 ".%d", bucket_oid_base.c_str(), gen_id, sid); + } else { + snprintf(buf, sizeof(buf), "%s.%d", bucket_oid_base.c_str(), sid); + } (*bucket_obj) = buf; if (shard_id) { *shard_id = (int)sid; @@ -279,7 +283,7 @@ int RGWSI_BucketIndex_RADOS::open_bucket_index_shard(const DoutPrefixProvider *d string oid; ret = get_bucket_index_object(bucket_oid_base, obj_key, bucket_info.layout.current_index.layout.normal.num_shards, - bucket_info.layout.current_index.layout.normal.hash_type, &oid, shard_id); + bucket_info.layout.current_index.layout.normal.hash_type, bucket_info.layout.current_index.gen, &oid, shard_id); if (ret < 0) { ldpp_dout(dpp, 10) << "get_bucket_index_object() returned ret=" << ret << dendl; return ret; @@ -366,7 +370,7 @@ int RGWSI_BucketIndex_RADOS::init_index(const DoutPrefixProvider *dpp, RGWBucket cct->_conf->rgw_bucket_index_max_aio)(); } -int RGWSI_BucketIndex_RADOS::clean_index(const DoutPrefixProvider *dpp, RGWBucketInfo& bucket_info, uint64_t gen) +int RGWSI_BucketIndex_RADOS::clean_index(const DoutPrefixProvider *dpp, RGWBucketInfo& bucket_info, const rgw::bucket_index_layout_generation& idx_layout) { RGWSI_RADOS::Pool index_pool; @@ -379,7 +383,8 @@ int RGWSI_BucketIndex_RADOS::clean_index(const DoutPrefixProvider *dpp, RGWBucke dir_oid.append(bucket_info.bucket.bucket_id); std::map bucket_objs; - get_bucket_index_objects(dir_oid, bucket_info.layout.current_index.layout.normal.num_shards, gen, &bucket_objs); + get_bucket_index_objects(dir_oid, idx_layout.layout.normal.num_shards, + idx_layout.gen, &bucket_objs); return CLSRGWIssueBucketIndexClean(index_pool.ioctx(), bucket_objs, diff --git a/src/rgw/services/svc_bi_rados.h b/src/rgw/services/svc_bi_rados.h index 17fc86b695cac..6f1e0b83c5af1 100644 --- a/src/rgw/services/svc_bi_rados.h +++ b/src/rgw/services/svc_bi_rados.h @@ -57,7 +57,7 @@ class RGWSI_BucketIndex_RADOS : public RGWSI_BucketIndex int get_bucket_index_object(const std::string& bucket_oid_base, const std::string& obj_key, uint32_t num_shards, rgw::BucketHashType hash_type, - std::string* bucket_obj, int* shard_id); + uint64_t gen_id, std::string *bucket_obj, int *shard_id); int cls_bucket_head(const DoutPrefixProvider *dpp, const RGWBucketInfo& bucket_info, @@ -98,10 +98,8 @@ public: return rgw_shards_mod(sid2, num_shards); } - int init_index(const DoutPrefixProvider *dpp, RGWBucketInfo& bucket_info, - const rgw::bucket_index_layout_generation& idx_layout) override; - int clean_index(const DoutPrefixProvider *dpp, RGWBucketInfo& bucket_info, - uint64_t gen) override; + int init_index(const DoutPrefixProvider *dpp, RGWBucketInfo& bucket_info,const rgw::bucket_index_layout_generation& idx_layout); + int clean_index(const DoutPrefixProvider *dpp, RGWBucketInfo& bucket_info, const rgw::bucket_index_layout_generation& idx_layout); /* RADOS specific */ -- 2.39.5