Skip to content

rgw: remove entries from bucket index shards directly in limited cases#46030

Merged
ivancich merged 3 commits intoceph:masterfrom
ivancich:wip-fix-bucket-index-new
Apr 30, 2022
Merged

rgw: remove entries from bucket index shards directly in limited cases#46030
ivancich merged 3 commits intoceph:masterfrom
ivancich:wip-fix-bucket-index-new

Conversation

@ivancich
Copy link
Member

@ivancich ivancich commented Apr 25, 2022 •
edited
Loading

This fixes issues surrounding radosgw-admin bucket check --fix ... and radosgw-admin object unlink .... Previously to remove an entry from a bucket index shard, we employed the dir_suggest mechanism. That is very subtle though, and another, important bug fix broke this functionality.

So this removes entries from the bucket index using librados and bypassing CLS.

Signed-off-by: J. Eric Ivancich ivancich@redhat.com

Fixes: https://tracker.ceph.com/issues/55476

Checklist

  • Tracker (select at least one)
    • References tracker ticket
    • Very recent bug; references commit where it was introduced
    • New feature (ticket optional)
    • Doc update (no ticket needed)
    • Code cleanup (no ticket needed)
  • Component impact
    • Affects Dashboard, opened tracker ticket
    • Affects Orchestrator, opened tracker ticket
    • No impact that needs to be tracked
  • Documentation (select at least one)
    • Updates relevant documentation
    • No doc update is appropriate
  • Tests (select at least one)
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 dashboard cephadm
  • jenkins test api
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox
  • jenkins test windows

@ivancich
Copy link
Member Author

ivancich commented Apr 25, 2022

@mattbenjamin and @cbodley This still needs some clean-up, but I wanted to give you an early view of it. This brings down the previous fix from wip-rgw-multisite-reshard and then removes references to the new bucket info data structures that supported that, and then reworks the underlying codepath.

@ivancich
rgw: use dpp in bitx logging code
3793a29 
This updates the bitx logging code to use the logging provided by
DoutPrefixProvider.

Signed-off-by: J. Eric Ivancich <ivancich@redhat.com>
@ivancich ivancich force-pushed the wip-fix-bucket-index-new branch 4 times, most recently from 04b7c22 to 60bc536 Compare April 26, 2022 17:48
@ivancich
Copy link
Member Author

ivancich commented Apr 26, 2022

@cbodley and @mattbenjamin : I think I'm done with the clean-up. Given the limited codepaths that come here, and all known are through radosgw-admin, I switched from asynchronous to synchronous given the former would force me to choose between fire and forget or managing the completions.

cbodley
cbodley reviewed Apr 27, 2022
View reviewed changes
Copy link
Contributor

@cbodley cbodley left a comment

Choose a reason for hiding this comment

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

I switched from asynchronous to synchronous given the former would force me to choose between fire and forget or managing the completions.

yeah, synchronous is probably fine here. 👍 the fire-and-forget model is especially problematic for radosgw-admin commands, because the process may exit before all of the enqueued requests get sent

@ivancich ivancich force-pushed the wip-fix-bucket-index-new branch from 60bc536 to 5525122 Compare April 27, 2022 14:50
@ivancich
rgw: "bucket check --fix" should delete damaged multipart uploads fro…
0521c2a 
…m bi

As one of the steps in `radosgw-admin bucket check --fix ...` it looks
for bucket index entries for incomplete multipart uploads that do not
have a corresponding ".meta" entry in the same bucket index. It then
intends to delete those entries, however the function that it calls
to perform the bucket index deletions was flawed and did not direct
the removals to the appropriate shard(s), but instead a non-existant
oid.

This commit determines the appropriate shard for each of the entries
to be removed and asynchronously issues a librados call to
omap_rm_keys.

Signed-off-by: J. Eric Ivancich <ivancich@redhat.com>
@ivancich ivancich force-pushed the wip-fix-bucket-index-new branch from 5525122 to fa2fff8 Compare April 27, 2022 14:51
@ivancich
rgw: address technical debt in RGWBucket::check_bad_index_multipart
de36f46 
Signed-off-by: J. Eric Ivancich <ivancich@redhat.com>
@ivancich ivancich force-pushed the wip-fix-bucket-index-new branch from fa2fff8 to de36f46 Compare April 28, 2022 17:35
@ivancich ivancich added the wip-eric-testing-1 for ivancich testing label Apr 29, 2022
@ivancich
Copy link
Member Author

ivancich commented Apr 30, 2022

Teuthology Results:

@ivancich
Copy link
Member Author

ivancich commented Apr 30, 2022

jenkins test api

1 similar comment
@ivancich
Copy link
Member Author

ivancich commented Apr 30, 2022

jenkins test api

@ivancich ivancich merged commit 2c12dbb into ceph:master Apr 30, 2022
@ivancich ivancich removed needs-qa wip-eric-testing-1 for ivancich testing labels Apr 30, 2022
@ivancich ivancich mentioned this pull request May 3, 2024
Closed
14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cbodley cbodley cbodley approved these changes

@mattbenjamin mattbenjamin Awaiting requested review from mattbenjamin

Assignees

No one assigned

Labels

bug-fix rgw

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ivancich @cbodley