From ccdd026be3aba4ef71cb7baeb4f3fd25b8f62835 Mon Sep 17 00:00:00 2001 From: "J. Eric Ivancich" Date: Fri, 7 Jan 2022 12:32:48 -0500 Subject: [PATCH] rgw: resharding causes bucket attributes to be lost With the new resharding code, some bucket metadata that is stored as xattrs (e.g., ACLs, life-cycle policies) were not sent with the updated bucket instance data when resharding completed. As a result, resharding has a regression where that metadata is lost after a successful reshard. This commit restores the variable in the RGWBucketReshard class that maintains the bucket attributes, so they can be saved when the bucket instance object is updated. Signed-off-by: J. Eric Ivancich --- src/rgw/rgw_admin.cc | 12 +++++++++--- src/rgw/rgw_reshard.cc | 28 +++++++++++++++++++--------- src/rgw/rgw_reshard.h | 6 ++++++ 3 files changed, 34 insertions(+), 12 deletions(-) diff --git a/src/rgw/rgw_admin.cc b/src/rgw/rgw_admin.cc index 9ba9979f628d9..fc9ad094a6b26 100644 --- a/src/rgw/rgw_admin.cc +++ b/src/rgw/rgw_admin.cc @@ -7685,7 +7685,9 @@ next: return -EINVAL; } - RGWBucketReshard br(static_cast(store), bucket->get_info(), nullptr /* no callback */); + RGWBucketReshard br(static_cast(store), + bucket->get_info(), bucket->get_attrs(), + nullptr /* no callback */); #define DEFAULT_RESHARD_MAX_ENTRIES 1000 if (max_entries < 1) { @@ -7785,7 +7787,9 @@ next: return -ret; } - RGWBucketReshard br(static_cast(store), bucket->get_info(), nullptr /* no callback */); + RGWBucketReshard br(static_cast(store), + bucket->get_info(), bucket->get_attrs(), + nullptr /* no callback */); list status; int r = br.get_status(dpp(), &status); if (r < 0) { @@ -7828,7 +7832,9 @@ next: if (bucket_initable) { // we did not encounter an error, so let's work with the bucket - RGWBucketReshard br(static_cast(store), bucket->get_info(), nullptr /* no callback */); + RGWBucketReshard br(static_cast(store), + bucket->get_info(), bucket->get_attrs(), + nullptr /* no callback */); int ret = br.cancel(dpp()); if (ret < 0) { if (ret == -EBUSY) { diff --git a/src/rgw/rgw_reshard.cc b/src/rgw/rgw_reshard.cc index 596421bbc99c0..55233b81b80b4 100644 --- a/src/rgw/rgw_reshard.cc +++ b/src/rgw/rgw_reshard.cc @@ -248,8 +248,9 @@ public: RGWBucketReshard::RGWBucketReshard(rgw::sal::RadosStore* _store, const RGWBucketInfo& _bucket_info, + const std::map& _bucket_attrs, RGWBucketReshardLock* _outer_reshard_lock) : - store(_store), bucket_info(_bucket_info), + store(_store), bucket_info(_bucket_info), bucket_attrs(_bucket_attrs), reshard_lock(store, bucket_info, true), outer_reshard_lock(_outer_reshard_lock) { } @@ -323,6 +324,7 @@ static int init_target_index(rgw::sal::RadosStore* store, // write the target layout to the bucket instance metadata static int init_target_layout(rgw::sal::RadosStore* store, RGWBucketInfo& bucket_info, + std::map& bucket_attrs, const ReshardFaultInjector& fault, uint32_t new_num_shards, const DoutPrefixProvider* dpp) @@ -371,7 +373,7 @@ static int init_target_layout(rgw::sal::RadosStore* store, if (ret = fault.check("set_target_layout"); ret == 0) { // no fault injected, write the bucket instance metadata ret = store->getRados()->put_bucket_instance_info(bucket_info, false, - real_time(), nullptr, dpp); + real_time(), &bucket_attrs, dpp); } if (ret < 0) { @@ -424,11 +426,12 @@ static int revert_target_layout(rgw::sal::RadosStore* store, static int init_reshard(rgw::sal::RadosStore* store, RGWBucketInfo& bucket_info, + std::map& bucket_attrs, const ReshardFaultInjector& fault, uint32_t new_num_shards, const DoutPrefixProvider *dpp) { - int ret = init_target_layout(store, bucket_info, fault, new_num_shards, dpp); + int ret = init_target_layout(store, bucket_info, bucket_attrs, fault, new_num_shards, dpp); if (ret < 0) { return ret; } @@ -472,6 +475,7 @@ static int cancel_reshard(rgw::sal::RadosStore* store, static int commit_reshard(rgw::sal::RadosStore* store, RGWBucketInfo& bucket_info, + std::map& bucket_attrs, const ReshardFaultInjector& fault, const DoutPrefixProvider *dpp) { @@ -506,7 +510,10 @@ static int commit_reshard(rgw::sal::RadosStore* store, int ret = fault.check("commit_target_layout"); if (ret == 0) { // no fault injected, write the bucket instance metadata - ret = store->getRados()->put_bucket_instance_info(bucket_info, false, real_time(), nullptr, dpp); + ret = + store->getRados()->put_bucket_instance_info(bucket_info, false, + real_time(), + &bucket_attrs, dpp); } if (ret < 0) { @@ -817,7 +824,7 @@ int RGWBucketReshard::execute(int num_shards, } // prepare the target index and add its layout the bucket info - ret = init_reshard(store, bucket_info, fault, num_shards, dpp); + ret = init_reshard(store, bucket_info, bucket_attrs, fault, num_shards, dpp); if (ret < 0) { return ret; } @@ -837,7 +844,7 @@ int RGWBucketReshard::execute(int num_shards, return ret; } - ret = commit_reshard(store, bucket_info, fault, dpp); + ret = commit_reshard(store, bucket_info, bucket_attrs, fault, dpp); if (ret < 0) { return ret; } @@ -1051,11 +1058,14 @@ int RGWReshard::process_entry(const cls_rgw_reshard_entry& entry, rgw_bucket bucket; RGWBucketInfo bucket_info; + std::map bucket_attrs; int ret = store->getRados()->get_bucket_info(store->svc(), - entry.tenant, entry.bucket_name, + entry.tenant, + entry.bucket_name, bucket_info, nullptr, - null_yield, dpp, nullptr); + null_yield, dpp, + &bucket_attrs); if (ret < 0 || bucket_info.bucket.bucket_id != entry.bucket_id) { if (ret < 0) { ldpp_dout(dpp, 0) << __func__ << @@ -1097,7 +1107,7 @@ int RGWReshard::process_entry(const cls_rgw_reshard_entry& entry, return remove(dpp, entry); } - RGWBucketReshard br(store, bucket_info, nullptr); + RGWBucketReshard br(store, bucket_info, bucket_attrs, nullptr); ReshardFaultInjector f; // no fault injected ret = br.execute(entry.new_num_shards, f, max_entries, dpp, diff --git a/src/rgw/rgw_reshard.h b/src/rgw/rgw_reshard.h index 09c89bb973006..cb9264dfd7bf0 100644 --- a/src/rgw/rgw_reshard.h +++ b/src/rgw/rgw_reshard.h @@ -74,6 +74,7 @@ class RGWBucketReshard { private: rgw::sal::RadosStore *store; RGWBucketInfo bucket_info; + std::map bucket_attrs; RGWBucketReshardLock reshard_lock; RGWBucketReshardLock* outer_reshard_lock; @@ -95,6 +96,7 @@ public: // manage RGWBucketReshard(rgw::sal::RadosStore* _store, const RGWBucketInfo& _bucket_info, + const std::map& _bucket_attrs, RGWBucketReshardLock* _outer_reshard_lock); int execute(int num_shards, const ReshardFaultInjector& f, int max_op_entries, const DoutPrefixProvider *dpp, @@ -164,6 +166,10 @@ public: return final_num_shards; } + const std::map& get_bucket_attrs() const { + return bucket_attrs; + } + // for multisite, the RGWBucketInfo keeps a history of old log generations // until all peers are done with them. prevent this log history from growing // too large by refusing to reshard the bucket until the old logs get trimmed -- 2.39.5