Skip to content

rgw: dynamic resharding modifies existing bucket instance#38657

Closed
cbodley wants to merge 38 commits intoceph:masterfrom
cbodley:wip-rgw-reshard-instance
Closed

rgw: dynamic resharding modifies existing bucket instance#38657
cbodley wants to merge 38 commits intoceph:masterfrom
cbodley:wip-rgw-reshard-instance

Conversation

@cbodley
Copy link
Contributor

@cbodley cbodley commented Dec 18, 2020

builds on #35175

  • refactors the reshard logic into separate functions to better compose the fault injection and error handling
  • adds cleanup of old-style reshards that were in progress
  • refactors the test coverage into a test_bucket_reshard() function that's parameterized on the fault to inject

Checklist

  • References tracker ticket
  • Updates documentation if necessary
  • Includes tests for new functionality or reproducer for bug
Show available Jenkins commands
  • jenkins retest this please
  • jenkins test classic perf
  • jenkins test crimson perf
  • jenkins test signed
  • jenkins test make check
  • jenkins test make check arm64
  • jenkins test submodules
  • jenkins test dashboard
  • jenkins test api
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox

@cbodley cbodley added the rgw label Dec 18, 2020
@cbodley cbodley requested a review from smanjara December 18, 2020 18:29
@cbodley
Copy link
Contributor Author

cbodley commented Dec 18, 2020

passed qa in https://pulpito.ceph.com/cbodley-2020-12-18_18:48:51-rgw-wip-rgw-reshard-instance-distro-basic-smithi/

@cbodley cbodley force-pushed the wip-rgw-reshard-instance branch from e2890fb to b31e054 Compare December 18, 2020 21:09
@cbodley
Copy link
Contributor Author

cbodley commented Jan 13, 2021

jenkins test make check

1 similar comment
@cbodley
Copy link
Contributor Author

cbodley commented Jan 15, 2021

jenkins test make check

@cbodley
Copy link
Contributor Author

cbodley commented Jan 18, 2021

jenkins test make check

Shilpa Jagannath and others added 18 commits January 18, 2021 14:44
@cbodley
rgw/dynamic-resharding: remove creation of new bucket instance
7deebfa 
Signed-off-by: Shilpa Jagannath <smanjara@redhat.com>
@cbodley
rgw/dynamic-resharding: reshard bucket using the same bucket instance id
1290f10 
Signed-off-by: Shilpa Jagannath <smanjara@redhat.com>
@cbodley
rgw/dynamic-resharding: Allow clean_index to take generation number a…
6b4ff14 
…s a parameter.

Signed-off-by: Shilpa Jagannath <smanjara@redhat.com>
@cbodley
rgw: use BucketReshardState states in appropriate places replacing cl…
c68fd1d 
…s_rgw_reshard_status.

Remove unused parameters in cls_rgw_bucket_instance_entry.
Other minor cleanup fixes.

Signed-off-by: Shilpa Jagannath <smanjara@redhat.com>
@cbodley
rgw: fetch updated bucketinfo rather than bucket_id in guard_reshard(…
1d397ec 
…) to be

able to read the updated layout.

Signed-off-by: Shilpa Jagannath <smanjara@redhat.com>
@cbodley
rgw: use rgw::BucketReshardState for reshard state change
f8ef0df 
Signed-off-by: Shilpa Jagannath <smanjara@redhat.com>
@cbodley
rgw: target_layout to be optional
25d0f68 
Signed-off-by: Shilpa Manjarabad Jagannath <smanjara@redhat.com>
@cbodley
rgw: - make RGWBucketInfo a non-const reference in functions using gu…
0d1ccbb 
…ard_reshard()

     - remove unused 'instance_id' from cls_rgw_reshard_entry
     - other minor fixes

Signed-off-by: Shilpa Manjarabad Jagannath <smanjara@redhat.com>
@cbodley
rgw: - take gen_id without optional as parameter in svc_bi_rados func…
58eb82c 
…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>
@cbodley
rgw: - allow clean_index() to take const bucket_index_layout_generation&
4d08de8 
     - fix bi_get() to get objects after being resharded

Signed-off-by: Shilpa Jagannath <smanjara@redhat.com>
@cbodley
rgw: failing to reshard, restore old indexes
f949f00 
Signed-off-by: Shilpa Jagannath <smanjara@redhat.com>
@cbodley
rgw: minor fixes
b9ab0e5 
Signed-off-by: Shilpa Jagannath <smanjara@redhat.com>
@cbodley
rgw: helper function to set reshard status from rgw::BucketReshardState.
b91153a 
Signed-off-by: Shilpa Jagannath <smanjara@redhat.com>
@cbodley
rgw: clean up fixes for extra spaces and indentation.
c21c75b 
Signed-off-by: Shilpa Jagannath <smanjara@redhat.com>
@cbodley
rgw: code organization fixes and error handling
c7516d0 
Signed-off-by: Shilpa Jagannath <smanjara@redhat.com>
@cbodley
rgw: for backward compatibility during upgrades, adding new_bucket_in…
3d2ed44 
…stance_id and

new_instance_id fields back for proper decoding.

Signed-off-by: Shilpa Jagannath <smanjara@redhat.com>
@cbodley
rgw: use a helper function to handle repetitive num_shards check
5f73841 
Signed-off-by: Shilpa Jagannath <smanjara@redhat.com>
@cbodley
rgw: - Better error handling
eac7d1f 
     - function update_bucket() handles updating bucket state

Signed-off-by: Shilpa Jagannath <smanjara@redhat.com>
@cbodley
rgw: removed unused BucketInfoReshardUpdate
adcdd06 
Signed-off-by: Casey Bodley <cbodley@redhat.com>
@cbodley
radosgw-admin: 'bucket reshard' returns positive error codes
fa2291a 
Signed-off-by: Casey Bodley <cbodley@redhat.com>
@cbodley
test/rgw: test_rgw_reshard.py exec_cmd() can return error code
b7ee3a2 
Signed-off-by: Casey Bodley <cbodley@redhat.com>
@cbodley
test/rgw: add test_bucket_reshard() for fault injection testing
e599116 
Signed-off-by: Casey Bodley <cbodley@redhat.com>
@cbodley cbodley force-pushed the wip-rgw-reshard-instance branch from b31e054 to c282264 Compare January 18, 2021 19:45
@cbodley
Copy link
Contributor Author

cbodley commented Jan 18, 2021

rebased. hoping the unrelated make check failures go away

@cbodley
test/rgw: ssl support for test_rgw_reshard.py
4cebcde 
Signed-off-by: Casey Bodley <cbodley@redhat.com>
@cbodley
qa/rgw: disable coredumps for reshard fault injection
baa2dd4 
Signed-off-by: Casey Bodley <cbodley@redhat.com>
@cbodley
rgw: clean up uses of BucketShard::init() without info
bab2750 
the rgw_bucket overload of BucketShard::init() has to look up the bucket
info. use the RGWBucketInfo overload when we have one

Signed-off-by: Casey Bodley <cbodley@redhat.com>
@cbodley
rgw: BucketReshardManager takes layouts
679c74c 
Signed-off-by: Casey Bodley <cbodley@redhat.com>
@cbodley
rgw: fix decode of cls_rgw reshard types
7821a4f 
Signed-off-by: Casey Bodley <cbodley@redhat.com>
@cbodley
rgw: RGWBucketReshard doesn't need a friend
21288e4 
Signed-off-by: Casey Bodley <cbodley@redhat.com>
@cbodley
radosgw-admin: remove fault injection options from usage
ffaa749 
these are only used for testing, not administration

Signed-off-by: Casey Bodley <cbodley@redhat.com>
@cbodley cbodley force-pushed the wip-rgw-reshard-instance branch from c282264 to ffaa749 Compare January 18, 2021 22:09
@cbodley
Copy link
Contributor Author

cbodley commented Jan 18, 2021

oh wow, that's an obscure failure. i guess something in 'make check' was linting test_rgw_reshard.py?

flake8 run-test: commands[0] | flake8 --select=F,E9 --exclude=venv,.tox
./workunits/rgw/test_rgw_reshard.py:9:1: F401 'pprint.pprint' imported but unused
./workunits/rgw/test_rgw_reshard.py:10:1: F401 're' imported but unused

pushed a fix

@cbodley
test: fix threading for FaultInjector death tests
25a5b20 
addresses test timeout and warning message:

[WARNING] /home/jenkins-build/build/workspace/ceph-pull-requests/src/googletest/googletest/src/gtest-death-test.cc:1121:: Death tests use fork(), which is unsafe particularly in a threaded context. For this test, Google Test detected 3 threads. See https://github.com/google/googletest/blob/master/googletest/docs/advanced.md#death-tests-and-threads for more explanation and suggested solutions, especially if this is the last message you see before your test times out.

Signed-off-by: Casey Bodley <cbodley@redhat.com>
@cbodley
Copy link
Contributor Author

cbodley commented Jan 19, 2021

215/215 Test #110: unittest_fault_injector ...................***Timeout 3600.01 sec

pushed a fix for the death tests

@cbodley
Copy link
Contributor Author

cbodley commented Jan 19, 2021

jenkins test api

This was referenced Jan 19, 2021
Closed
Merged
smanjara
smanjara requested changes Jan 25, 2021
View reviewed changes
src/rgw/rgw_reshard.cc
ldout(store->ctx(), 0) << "ERROR: " << __func__ << " failed to clear "
"target index layout in bucket info: " << cpp_strerror(ret) << dendl;

bucket_info.layout = std::move(prev); // restore in-memory layout
Copy link
Contributor

@smanjara smanjara Jan 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

after restoring the in memory layout, I think we have to commit it.

Copy link
Contributor Author

@cbodley cbodley Jan 27, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this block is handling the error from put_bucket_instance_info() above, where we failed to commit the updated bucket_info to rados. so it's just reverting to what we had before

@smanjara
Copy link
Contributor

smanjara commented Jan 25, 2021

everything else looks great!

@github-actions
Copy link

github-actions bot commented Jan 27, 2021

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

@cbodley
Copy link
Contributor Author

cbodley commented Feb 5, 2021

this merged into #39002

@cbodley cbodley closed this Feb 5, 2021
@mattbenjamin
Copy link
Contributor

mattbenjamin commented Feb 5, 2021

yay!

@cbodley cbodley mentioned this pull request Nov 10, 2021
Merged
15 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@smanjara smanjara smanjara requested changes

Assignees

No one assigned

Labels

build/ops common needs-rebase rgw tests

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@cbodley @smanjara @mattbenjamin