rgw: dynamic resharding modifies existing bucket instance by smanjara · Pull Request #35175 · ceph/ceph

smanjara · 2020-05-21T14:09:24Z

With these changes dynamic resharding does not require a new bucket instance anymore, thus facilitating the upcoming multisite support use case.
In continuation with #34096 and #34352, a property called bucket layout is introduced in RGWBucketInfo. With every reshard process, the bucket index transitions from current_index to target_index layout and each can have its own properties like BucketIndexType and BucketHashType. To do this, it maintains a generation number that is incremented in every reshard cycle.

This PR is part of multisite work documented in:
https://github.com/ceph/ceph/blob/master/src/doc/rgw/multisite-reshard.md#bucket-index-resharding

src/cls/rgw/cls_rgw_types.h

src/rgw/rgw_common.h

src/rgw/rgw_bucket_layout.h

src/rgw/services/svc_bi_rados.cc

smanjara · 2020-07-07T13:03:10Z

@cbodley, pushed changes addressing most of the comments. Could you please review?

src/cls/rgw/cls_rgw_types.h

src/rgw/rgw_rados.h

src/rgw/services/svc_bi_rados.h

src/rgw/rgw_reshard.cc

src/rgw/services/svc_bi_rados.h

src/rgw/services/svc_bi_rados.cc

src/rgw/rgw_reshard.cc

smanjara · 2020-07-23T19:56:02Z

@cbodley addressed some comments in the last three commits. Can you please review them?

ivancich

I believe there are some left-over comments near line #1117 of rgw_common.h.

src/cls/rgw/cls_rgw_types.h

src/rgw/rgw_admin.cc

ivancich · 2020-08-25T19:55:33Z

src/rgw/services/svc_bi_rados.cc

    bucket_objects[0] = bucket_oid_base;
  } else {
-    char buf[bucket_oid_base.size() + 32];
+    char buf[bucket_oid_base.size() + 64];


Would a std::stringstream work better here? Array of chars + snprintf seems so clunky, especially given that the result will ultimately go into a std::string.

src/rgw/rgw_reshard.cc

src/rgw/rgw_rados.cc

rzarzynski · 2020-08-26T16:26:56Z

src/cls/rgw/cls_rgw_types.cc

  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);


Do we need to preserve a backward compatibility here?

rzarzynski · 2020-08-26T16:27:22Z

src/cls/rgw/cls_rgw_types.cc

 {
  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);


Same question about backward compatibility here.

src/cls/rgw/cls_rgw_types.h

rzarzynski · 2020-08-26T16:37:59Z

src/rgw/rgw_common.h

-  cls_rgw_reshard_status reshard_status{cls_rgw_reshard_status::NOT_RESHARDING};
-  string new_bucket_instance_id;
+  rgw::BucketReshardState reshard_status{rgw::BucketReshardState::NOT_RESHARDING};
+  string bucket_instance_id;


Suggested change

string bucket_instance_id;

std::string bucket_instance_id;

nit: let's not rely on namespace inclusion in headers.

rzarzynski · 2020-08-26T16:38:19Z

src/rgw/rgw_json_enc.cc

  encode_json("mdsearch_config", mdsearch_config, f);
  encode_json("reshard_status", (int)reshard_status, f);
-  encode_json("new_bucket_instance_id", new_bucket_instance_id, f);
+  encode_json("bucket_instance_id", bucket_instance_id, f);


Backward compatibility? Do we need to worry about that when working with JSONs?

src/rgw/rgw_rados.cc

src/rgw/rgw_reshard.cc

src/rgw/services/svc_bi_rados.cc

rzarzynski · 2020-08-27T15:38:48Z

src/rgw/rgw_admin.cc

      int shard_id = (bucket_info.layout.current_index.layout.normal.num_shards > 0  ? i : -1);
      const auto& current = bucket_info.layout.current_index;
-      int ret = bs.init(bucket, shard_id, current, nullptr /* no RGWBucketInfo */);
+      int ret = bs.init(bucket, shard_id, current, std::nullopt, nullptr /* no RGWBucketInfo */);


Is this commit a fixup?

smanjara · 2020-09-21T13:08:55Z

I believe there are some left-over comments near line #1117 of rgw_common.h.

Thanks, I have fixed it.

Signed-off-by: Shilpa Jagannath <smanjara@redhat.com>

…s a parameter. Signed-off-by: Shilpa Jagannath <smanjara@redhat.com>

…tions. - call init_index() on target layout during reshard process. Takes const rgw::bucket_index_layout_generation& Signed-off-by: Shilpa Jagannath <smanjara@redhat.com>

- fix bi_get() to get objects after being resharded Signed-off-by: Shilpa Jagannath <smanjara@redhat.com>

Signed-off-by: Shilpa Jagannath <smanjara@redhat.com>

…stance_id and new_instance_id fields back for proper decoding. Signed-off-by: Shilpa Jagannath <smanjara@redhat.com>

Signed-off-by: Shilpa Jagannath <smanjara@redhat.com>

- function update_bucket() handles updating bucket state Signed-off-by: Shilpa Jagannath <smanjara@redhat.com>

cbodley · 2020-10-06T12:59:23Z

@smanjara have you managed to integrate the FaultInjector stuff for testing? i'd really like to see test cases using that as part of this work

smanjara · 2020-10-06T13:10:24Z

@smanjara have you managed to integrate the FaultInjector stuff for testing? i'd really like to see test cases using that as part of this work

@cbodley I was under the impression that it was supposed to be used for local testing. Will write some test cases. Thanks!

cbodley · 2020-10-06T13:23:25Z

thanks @smanjara! i think this automated test coverage is important, first to give us confidence that we got things right, and second for regression testing to make sure we don't break things. to recap, i'd like to see two tests cases around each step of the reshard process (one injecting an error, and another injecting a crash), showing we can recover from each - meaning we can both write a new object, and reshard successfully

cc @ivancich, since you did a lot of work on the recovery and error handling here, i could really use your help with review!

ivancich · 2020-10-06T13:58:10Z

cc @ivancich, since you did a lot of work on the recovery and error handling here, i could really use your help with review!

Sure, @cbodley. I'll spend some time digging in.

smanjara · 2020-10-07T05:59:05Z

cc @ivancich, since you did a lot of work on the recovery and error handling here, i could really use your help with review!

Sure, @cbodley. I'll spend some time digging in.

That'd be great. Thanks @ivancich

Signed-off-by: Casey Bodley <cbodley@redhat.com>

src/rgw/rgw_reshard.cc

src/rgw/rgw_admin.cc

src/rgw/rgw_reshard.cc

Signed-off-by: Shilpa Jagannath <smanjara@redhat.com>

smanjara · 2020-10-28T14:39:23Z

@cbodley Initial results from a single run (search for --inject):
http://qa-proxy.ceph.com/teuthology/smanjara-2020-10-28_13:34:20-rgw:verify-wip-dynamic-reshard-distro-basic-smithi/5568240/teuthology.log

After a crash, the reshard lock is held on the bucket for a default time of 360 seconds. Is that intentional? @ivancich can you weigh in?

smanjara · 2020-11-02T06:01:58Z

Reduced the default reshard lock value and re-ran the test

http://qa-proxy.ceph.com/teuthology/smanjara-2020-10-30_12:52:04-rgw:verify-wip-dynamic-reshard-distro-basic-smithi/5575067/teuthology.log

smanjara · 2020-11-12T06:41:39Z

Updated test with more injection points:
http://qa-proxy.ceph.com/teuthology/smanjara-2020-11-11_13:03:06-rgw:verify-wip-dynamic-reshard-distro-basic-smithi/5613093/teuthology.log

github-actions · 2020-12-16T17:22:16Z

This pull request can no longer be automatically merged: a rebase is needed and changes have to be manually resolved

smanjara requested review from cbodley and mattbenjamin May 21, 2020 14:10

smanjara force-pushed the wip-dynamic-reshard branch from 5a7e69f to 12a8a6f Compare May 22, 2020 13:13

cbodley reviewed May 22, 2020

View reviewed changes

src/cls/rgw/cls_rgw_types.h Outdated Show resolved Hide resolved

src/cls/rgw/cls_rgw_types.h Outdated Show resolved Hide resolved

neha-ojha added the rgw label May 22, 2020

smanjara force-pushed the wip-dynamic-reshard branch from 12a8a6f to 4beeb92 Compare June 10, 2020 14:26

cbodley reviewed Jun 18, 2020

View reviewed changes

src/rgw/rgw_common.h Outdated Show resolved Hide resolved

src/rgw/rgw_bucket_layout.h Outdated Show resolved Hide resolved

src/rgw/services/svc_bi_rados.cc Outdated Show resolved Hide resolved

src/rgw/services/svc_bi_rados.cc Outdated Show resolved Hide resolved

smanjara force-pushed the wip-dynamic-reshard branch from 4beeb92 to bdf26a8 Compare June 29, 2020 19:20

smanjara force-pushed the wip-dynamic-reshard branch from 798216e to cd74662 Compare July 7, 2020 10:21

cbodley reviewed Jul 7, 2020

View reviewed changes

smanjara force-pushed the wip-dynamic-reshard branch from cd74662 to 9435822 Compare July 10, 2020 08:16

cbodley reviewed Jul 10, 2020

View reviewed changes

src/rgw/services/svc_bi_rados.cc Outdated Show resolved Hide resolved

smanjara force-pushed the wip-dynamic-reshard branch from e6b60da to a0b49ba Compare July 14, 2020 12:34

smanjara mentioned this pull request Jul 15, 2020

rgw: groundwork for supporting dynamic resharding in multisite environment #34096

Merged

cbodley reviewed Jul 15, 2020

View reviewed changes

smanjara force-pushed the wip-dynamic-reshard branch from a0b49ba to 63fef4f Compare July 21, 2020 13:16

smanjara requested a review from ivancich August 20, 2020 07:24

smanjara force-pushed the wip-dynamic-reshard branch from 63fef4f to 2b558bf Compare August 25, 2020 06:25

ivancich reviewed Aug 25, 2020

View reviewed changes

rzarzynski reviewed Aug 27, 2020

View reviewed changes

smanjara force-pushed the wip-dynamic-reshard branch 2 times, most recently from 7829051 to e58da6f Compare September 1, 2020 19:02

smanjara force-pushed the wip-dynamic-reshard branch 2 times, most recently from 3881d0a to b6c85f1 Compare September 11, 2020 16:38

Shilpa Jagannath added 3 commits September 28, 2020 08:13

rgw/dynamic-resharding: remove creation of new bucket instance

5688934

Signed-off-by: Shilpa Jagannath <smanjara@redhat.com>

rgw/dynamic-resharding: reshard bucket using the same bucket instance id

b8e3dc8

Signed-off-by: Shilpa Jagannath <smanjara@redhat.com>

rgw/dynamic-resharding: Allow clean_index to take generation number a…

9aed3ad

…s a parameter. Signed-off-by: Shilpa Jagannath <smanjara@redhat.com>

Shilpa Jagannath added 10 commits September 28, 2020 08:26

rgw: - take gen_id without optional as parameter in svc_bi_rados func…

6f589b5

…tions. - call init_index() on target layout during reshard process. Takes const rgw::bucket_index_layout_generation& Signed-off-by: Shilpa Jagannath <smanjara@redhat.com>

rgw: - allow clean_index() to take const bucket_index_layout_generation&

c9b6184

- fix bi_get() to get objects after being resharded Signed-off-by: Shilpa Jagannath <smanjara@redhat.com>

rgw: failing to reshard, restore old indexes

a0456b0

Signed-off-by: Shilpa Jagannath <smanjara@redhat.com>

rgw: minor fixes

34213c9

Signed-off-by: Shilpa Jagannath <smanjara@redhat.com>

rgw: helper function to set reshard status from rgw::BucketReshardState.

8d41c49

Signed-off-by: Shilpa Jagannath <smanjara@redhat.com>

rgw: clean up fixes for extra spaces and indentation.

0cb2552

Signed-off-by: Shilpa Jagannath <smanjara@redhat.com>

rgw: code organization fixes and error handling

d4af2c5

Signed-off-by: Shilpa Jagannath <smanjara@redhat.com>

rgw: for backward compatibility during upgrades, adding new_bucket_in…

941ba9b

…stance_id and new_instance_id fields back for proper decoding. Signed-off-by: Shilpa Jagannath <smanjara@redhat.com>

rgw: use a helper function to handle repetitive num_shards check

af093b3

Signed-off-by: Shilpa Jagannath <smanjara@redhat.com>

rgw: - Better error handling

5517551

- function update_bucket() handles updating bucket state Signed-off-by: Shilpa Jagannath <smanjara@redhat.com>

smanjara force-pushed the wip-dynamic-reshard branch from 5b4fdb0 to 5517551 Compare September 28, 2020 05:39

cbodley mentioned this pull request Oct 7, 2020

rgw: add bucket index log layout structure #37586

Merged

3 tasks

common: add generic FaultInjector

f297705

Signed-off-by: Casey Bodley <cbodley@redhat.com>

cbodley reviewed Oct 19, 2020

View reviewed changes

src/rgw/rgw_reshard.cc Outdated Show resolved Hide resolved

src/rgw/rgw_reshard.cc Show resolved Hide resolved

src/rgw/rgw_admin.cc Show resolved Hide resolved

src/rgw/rgw_reshard.cc Outdated Show resolved Hide resolved

smanjara force-pushed the wip-dynamic-reshard branch from 1cc6d45 to 4305ea7 Compare October 20, 2020 08:28

rgw: reshard tests with fault injection

ae18662

Signed-off-by: Shilpa Jagannath <smanjara@redhat.com>

smanjara force-pushed the wip-dynamic-reshard branch from 4305ea7 to ae18662 Compare October 20, 2020 08:39

github-actions bot added the needs-rebase label Dec 16, 2020

cbodley mentioned this pull request Dec 18, 2020

rgw: dynamic resharding modifies existing bucket instance #38657

Closed

3 tasks

cbodley mentioned this pull request Jan 21, 2021

rgw multisite: bucket reshard work in progress #39002

Merged

31 tasks

smanjara closed this May 20, 2021

Conversation

smanjara commented May 21, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

smanjara commented Jul 7, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

smanjara commented Jul 23, 2020

Uh oh!

ivancich left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ivancich Aug 25, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rzarzynski Aug 26, 2020

Choose a reason for hiding this comment

Uh oh!

rzarzynski Aug 26, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rzarzynski Aug 26, 2020

Choose a reason for hiding this comment

Uh oh!

rzarzynski Aug 26, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rzarzynski Aug 27, 2020

Choose a reason for hiding this comment

Uh oh!

smanjara commented Sep 21, 2020

Uh oh!

cbodley commented Oct 6, 2020

Uh oh!

smanjara commented Oct 6, 2020

Uh oh!

cbodley commented Oct 6, 2020

Uh oh!

ivancich commented Oct 6, 2020

Uh oh!

smanjara commented Oct 7, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!