rgw: add radosgw-admin bucket check olh/unlinked commands by cfsnyder · Pull Request #52576 · ceph/ceph

cfsnyder · 2023-07-21T15:49:42Z

Adds commands to radosgw-admin for checking for and fixing leftover entries in the bucket index (and associated RADOS objects).

Fixes: https://tracker.ceph.com/issues/62075
Signed-off-by: Cory Snyder [email protected]

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)
- Includes unit test(s)
- Includes integration test(s)
- Includes bug reproducer
- No tests

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

cbodley

very nicely done with the coroutines!

can you please share some example output so i can review the formatting?

src/rgw/rgw_admin.cc

src/rgw/driver/rados/rgw_bucket.cc

cbodley · 2023-08-02T22:35:44Z

what guidance should we provide for users that might be affected? do you think a release note would suffice?

cfsnyder · 2023-09-01T09:18:12Z

very nicely done with the coroutines!

can you please share some example output so i can review the formatting?

radosgw-admin bucket check olh --bucket check-bucket --fix
2023-09-01T09:10:56.191+0000 7f77b1ab9f40 -1 WARNING: all dangerous and experimental features are enabled.
2023-09-01T09:10:56.199+0000 7f77b1ab9f40 -1 WARNING: all dangerous and experimental features are enabled.
2023-09-01T09:10:56.199+0000 7f77b1ab9f40 -1 WARNING: all dangerous and experimental features are enabled.
2023-09-01T09:10:56.399+0000 7f77b1ab9f40  1 NOTICE: finished shard 0 (0 entries removed)
2023-09-01T09:10:56.399+0000 7f77b1ab9f40  1 NOTICE: finished shard 1 (0 entries removed)
2023-09-01T09:10:56.399+0000 7f77b1ab9f40  1 NOTICE: finished shard 2 (0 entries removed)
2023-09-01T09:10:56.403+0000 7f77b1ab9f40  1 NOTICE: finished shard 3 (0 entries removed)
2023-09-01T09:10:56.403+0000 7f77b1ab9f40  1 NOTICE: finished shard 4 (0 entries removed)
2023-09-01T09:10:56.407+0000 7f77b1ab9f40  1 NOTICE: finished shard 6 (0 entries removed)
2023-09-01T09:10:56.411+0000 7f77b1ab9f40  1 NOTICE: finished shard 7 (0 entries removed)
2023-09-01T09:10:56.411+0000 7f77b1ab9f40  1 NOTICE: finished shard 9 (0 entries removed)
2023-09-01T09:10:56.415+0000 7f77b1ab9f40  1 NOTICE: finished shard 10 (0 entries removed)
2023-09-01T09:10:56.451+0000 7f77b1ab9f40  1 NOTICE: finished shard 5 (1 entries removed)
2023-09-01T09:10:56.455+0000 7f77b1ab9f40  1 NOTICE: finished shard 8 (1 entries removed)
2023-09-01T09:10:56.455+0000 7f77b1ab9f40  1 NOTICE: finished all shards (2 entries removed)

radosgw-admin bucket check olh --bucket check-bucket
2023-09-01T09:10:55.611+0000 7f05c8fa0f40 -1 WARNING: all dangerous and experimental features are enabled.
2023-09-01T09:10:55.619+0000 7f05c8fa0f40 -1 WARNING: all dangerous and experimental features are enabled.
2023-09-01T09:10:55.619+0000 7f05c8fa0f40 -1 WARNING: all dangerous and experimental features are enabled.
2023-09-01T09:10:55.811+0000 7f05c8fa0f40  1 NOTICE: finished shard 0 (0 entries found)
2023-09-01T09:10:55.815+0000 7f05c8fa0f40  1 NOTICE: finished shard 1 (0 entries found)
2023-09-01T09:10:55.815+0000 7f05c8fa0f40  1 NOTICE: finished shard 2 (0 entries found)
2023-09-01T09:10:55.815+0000 7f05c8fa0f40  1 NOTICE: finished shard 3 (0 entries found)
2023-09-01T09:10:55.819+0000 7f05c8fa0f40  1 NOTICE: finished shard 4 (0 entries found)
2023-09-01T09:10:55.819+0000 7f05c8fa0f40  1 NOTICE: finished shard 5 (1 entries found)
2023-09-01T09:10:55.819+0000 7f05c8fa0f40  1 NOTICE: finished shard 6 (0 entries found)
2023-09-01T09:10:55.823+0000 7f05c8fa0f40  1 NOTICE: finished shard 7 (0 entries found)
2023-09-01T09:10:55.823+0000 7f05c8fa0f40  1 NOTICE: finished shard 8 (1 entries found)
2023-09-01T09:10:55.827+0000 7f05c8fa0f40  1 NOTICE: finished shard 9 (0 entries found)
2023-09-01T09:10:55.827+0000 7f05c8fa0f40  1 NOTICE: finished shard 10 (0 entries found)
2023-09-01T09:10:55.827+0000 7f05c8fa0f40  1 NOTICE: finished all shards (2 entries found)

radosgw-admin bucket check olh --bucket check-bucket --dump-keys --hide-progress
2023-09-01T09:14:54.523+0000 7fcdc6e0af40 -1 WARNING: all dangerous and experimental features are enabled.
2023-09-01T09:14:54.531+0000 7fcdc6e0af40 -1 WARNING: all dangerous and experimental features are enabled.
2023-09-01T09:14:54.531+0000 7fcdc6e0af40 -1 WARNING: all dangerous and experimental features are enabled.
[
    "d",
    "c"
]

radosgw-admin bucket check unlinked --bucket check-bucket --min-age-hours 0
2023-09-01T09:16:29.506+0000 7f74b2919f40 -1 WARNING: all dangerous and experimental features are enabled.
2023-09-01T09:16:29.510+0000 7f74b2919f40 -1 WARNING: all dangerous and experimental features are enabled.
2023-09-01T09:16:29.514+0000 7f74b2919f40 -1 WARNING: all dangerous and experimental features are enabled.
2023-09-01T09:16:29.698+0000 7f74b2919f40  1 NOTICE: finished shard 0 (0 entries found)
2023-09-01T09:16:29.706+0000 7f74b2919f40  1 NOTICE: finished shard 2 (0 entries found)
2023-09-01T09:16:29.706+0000 7f74b2919f40  1 NOTICE: finished shard 3 (0 entries found)
2023-09-01T09:16:29.706+0000 7f74b2919f40  1 NOTICE: finished shard 4 (0 entries found)
2023-09-01T09:16:29.714+0000 7f74b2919f40  1 NOTICE: finished shard 6 (0 entries found)
2023-09-01T09:16:29.714+0000 7f74b2919f40  1 NOTICE: finished shard 7 (0 entries found)
2023-09-01T09:16:29.718+0000 7f74b2919f40  1 NOTICE: finished shard 9 (0 entries found)
2023-09-01T09:16:29.718+0000 7f74b2919f40  1 NOTICE: finished shard 1 (1 entries found)
2023-09-01T09:16:29.718+0000 7f74b2919f40  1 NOTICE: finished shard 5 (1 entries found)
2023-09-01T09:16:29.722+0000 7f74b2919f40  1 NOTICE: finished shard 8 (1 entries found)
2023-09-01T09:16:29.722+0000 7f74b2919f40  1 NOTICE: finished shard 10 (1 entries found)
2023-09-01T09:16:29.722+0000 7f74b2919f40  1 NOTICE: finished all shards (4 entries found)

radosgw-admin bucket check unlinked --bucket check-bucket --min-age-hours 0 --dump-keys --hide-progress
2023-09-01T09:17:15.511+0000 7f03e0f56f40 -1 WARNING: all dangerous and experimental features are enabled.
2023-09-01T09:17:15.519+0000 7f03e0f56f40 -1 WARNING: all dangerous and experimental features are enabled.
2023-09-01T09:17:15.519+0000 7f03e0f56f40 -1 WARNING: all dangerous and experimental features are enabled.
[
    {
        "name": "e",
        "instance": "ojyZwRsIalwzxD70bhPvXk8r7K4huNz"
    },
    {
        "name": "d",
        "instance": "Z5X.BaZV.ojkThuajQpB98mjzZpocCx"
    },
    {
        "name": "c",
        "instance": "KGw9c1D-FfFh2mLT-76wzx1qyqCz0fm"
    },
    {
        "name": "f",
        "instance": "lbJGb.boc7p2VfBq9gW7ISWg3NbCIzT"
    }
]

cfsnyder · 2023-09-01T09:26:48Z

what guidance should we provide for users that might be affected? do you think a release note would suffice?

I'm not sure, what have you guys done for this sort of thing in the past? I could write something up to provide an explanation but not sure where it should live?

mattbenjamin · 2023-09-11T19:44:34Z

@ivancich could you give these changes some review, as well?

ivancich

Looks really good. There's a lot of iterated I/O (e.g., check on listable), but there's really no obvious way around it.

I would love to see really nice comments above each of the functions added to rgw/driver/rados/rgw_bucket.cc explaining what they do and a little bit of how they do it.

Thank you!

src/cls/rgw/cls_rgw_types.cc

src/cls/rgw/cls_rgw.cc

src/rgw/driver/rados/rgw_bucket.cc

cbodley · 2023-09-12T20:59:17Z

what guidance should we provide for users that might be affected? do you think a release note would suffice?

I'm not sure, what have you guys done for this sort of thing in the past? I could write something up to provide an explanation but not sure where it should live?

@cfsnyder a recent PendingReleaseNotes example from #52248:

RGW: S3 multipart uploads using Server-Side Encryption now replicate correctly in
multi-site. Previously, the replicas of such objects were corrupted on decryption.
A new tool, radosgw-admin bucket resync encrypted multipart, can be used to
identify these original multipart uploads. The LastModified timestamp of any
identified object is incremented by 1ns to cause peer zones to replicate it again.
For multi-site deployments that make any use of Server-Side Encryption, we
recommended running this command against every bucket in every zone after all
zones have upgraded.

cfsnyder · 2023-09-14T20:37:14Z

Looks really good. There's a lot of iterated I/O (e.g., check on listable), but there's really no obvious way around it.

I would love to see really nice comments above each of the functions added to rgw/driver/rados/rgw_bucket.cc explaining what they do and a little bit of how they do it.

Thank you!

Thanks for the review, @ivancich! I've added some doc comments, let me know if you have any feedback.

cfsnyder · 2023-09-15T07:27:16Z

what guidance should we provide for users that might be affected? do you think a release note would suffice?

I'm not sure, what have you guys done for this sort of thing in the past? I could write something up to provide an explanation but not sure where it should live?

@cfsnyder a recent PendingReleaseNotes example from #52248:

RGW: S3 multipart uploads using Server-Side Encryption now replicate correctly in
multi-site. Previously, the replicas of such objects were corrupted on decryption.
A new tool, radosgw-admin bucket resync encrypted multipart, can be used to
identify these original multipart uploads. The LastModified timestamp of any
identified object is incremented by 1ns to cause peer zones to replicate it again.
For multi-site deployments that make any use of Server-Side Encryption, we
recommended running this command against every bucket in every zone after all
zones have upgraded.

@cbodley how about this:

RGW: New tools have been added to radosgw-admin for identifying and correcting
issues with versioned bucket indexes. Historical bugs with the versioned bucket
index transaction workflow made it possible for the index to accumulate
extraneous "book-keeping" olh entries and plain placeholder entries. In some
specific scenarios where clients made concurrent requests referencing the same
object key, it was likely that a lot of extra index entries would accumulate. When
a significant number of these entries are present in a single bucket index shard,
they can cause high bucket listing latencies and lifecycle processing failures. To
check whether a versioned bucket has unnecessary olh entries, users can
now run radosgw-admin bucket check olh. If the --fix flag is used, the
extra entries will be safely removed. A distinct issue from the one described thus
far, it is also possible that some versioned buckets are maintaining extra unlinked
objects that are not listable from the S3/Swift APIs. These extra objects are
typically a result of PUT requests that exited abnormally, in the middle of a
bucket index transaction - so the client would not have received a successful
response. Bugs in prior releases made these unlinked objects easy to
reproduce with any PUT request that was made on a bucket that was actively
resharding. Besides the extra space that these hidden, unlinked objects consume,
there can be another side effect in certain scenarios, caused by the nature of the
failure mode that produced them, where a client of a bucket that was a victim
of this bug may find the object associated with the key to be in an inconsistent
state. To check whether a versioned bucket has unlinked entries, users can
now run radosgw-admin bucket check unlinked. If the --fix flag is used, the
unlinked objects will be safely removed. Finally, a third issue made it possible
for versioned bucket index stats to be accounted inaccurately. The tooling
for recalculating versioned bucket stats also had a bug, and was not previously
capable of fixing these inaccuracies. This release resolves those issues and
users can now expect that the existing radosgw-admin bucket check command
will produce correct results. We recommend that users with versioned buckets,
especially those that existed on prior releases, use these new tools to check
whether their buckets are affected and to clean them up accordingly.

cbodley · 2023-09-15T15:18:34Z

@cbodley how about this:

@cfsnyder that looks great. can you please add that to the PendingReleaseNotes file?

Adds commands to radosgw-admin for checking for and fixing leftover entries in the bucket index (and associated RADOS objects). Fixes: https://tracker.ceph.com/issues/62075 Signed-off-by: Cory Snyder <[email protected]>

… bugs Signed-off-by: Cory Snyder <[email protected]>

…k index Fixes: https://tracker.ceph.com/issues/62760 Signed-off-by: Cory Snyder <[email protected]>

If a call to bucket_index_link_olh or bucket_index_unlink_instance fails, its associated pending xattr may have prevented the olh object from being removed by another thread. We should do a best effort cleanup attempt for this case by calling update_olh before returning an error to the caller. Signed-off-by: Cory Snyder <[email protected]>

cfsnyder · 2023-09-21T19:37:02Z

FYI added another small commit to handle another possible scenario where olh objects get left behind when concurrent requests are interleaved in a particular way

cfsnyder · 2023-09-22T07:11:23Z

Test results here: http://pulpito.front.sepia.ceph.com/cfsnyder-2023-09-21_21:27:43-rgw-wip-62075-distro-default-smithi/

cfsnyder · 2023-09-22T07:38:07Z

Looks like failures are unrelated to me, but I'll let you double check the results @cbodley .

…--check-objects flag Printing all index entries can be very time consuming for large buckets and the inability to switch this behavior off makes it cumbersome to use the command for fixing bucket stats. This was also preventing the command from outputting recalculated bucket stats when the --fix flag wasn't specified. Signed-off-by: Cory Snyder <[email protected]>

cfsnyder · 2023-09-22T08:43:41Z

Actually added one more tiny change in a new commit. I don't think it should require a new QA run.

radosgw-admin bucket check should only print index entries with --check-objects flag

Printing all index entries can be very time consuming for large
buckets and the inability to switch this behavior off makes it
cumbersome to use the command for fixing bucket stats. This was
also preventing the command from outputting recalculated bucket
stats when the --fix flag wasn't specified.

Based upon the error message here, it looks like this was the intended use of the --check-objects flag anyhow, so this may have been a regression

cbodley · 2023-09-22T14:32:01Z

src/rgw/driver/rados/rgw_rados.cc

+      // it's possible that the pending xattr from this op prevented the olh
+      // object from being cleaned by another thread that was deleting the last
+      // existing version. We invoke a best-effort update_olh here to handle this case.
+      int r = update_olh(dpp, obj_ctx, state, bucket_info, olh_obj, y);


cbodley · 2023-09-23T14:58:35Z

Actually added one more tiny change in a new commit. I don't think it should require a new QA run.

@cfsnyder unfortunately, that commit broke the ragweed tests which use RGWOp_Check_Bucket_Index (the admin API version of 'bucket check'): https://tracker.ceph.com/issues/62956

cfsnyder · 2023-09-25T10:14:04Z

@cbodley , I noticed that at the EOD on Friday while testing a Pacific backport of these changes. What I found was that this admin API doesn't return valid JSON. It coincidentally did return valid JSON previously, with the right combination of query params and due to the bug that was fixed here. I just added a commit here #53607 to fix the output format of the API. I'll look at updating the Ragweed tests accordingly this morning, but wondering if you know of any other clients that may need to be updated to account for this fix?

cfsnyder requested a review from a team as a code owner July 21, 2023 15:49

github-actions bot added rgw tests labels Jul 21, 2023

cfsnyder force-pushed the wip-62075 branch from 939cf6f to 08c482f Compare July 21, 2023 15:57

cfsnyder requested a review from cbodley July 21, 2023 15:58

cbodley reviewed Aug 2, 2023

View reviewed changes

cfsnyder force-pushed the wip-62075 branch 3 times, most recently from f612d4c to 5e4ddc5 Compare September 1, 2023 09:23

cfsnyder requested a review from cbodley September 1, 2023 09:27

cfsnyder force-pushed the wip-62075 branch from 5e4ddc5 to e77b86a Compare September 1, 2023 15:00

cfsnyder force-pushed the wip-62075 branch 2 times, most recently from d197ecb to bcf9046 Compare September 11, 2023 19:41

mattbenjamin requested a review from ivancich September 11, 2023 19:44

ivancich approved these changes Sep 12, 2023

View reviewed changes

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

src/cls/rgw/cls_rgw.cc Outdated Show resolved Hide resolved

src/cls/rgw/cls_rgw.cc Outdated Show resolved Hide resolved

src/rgw/driver/rados/rgw_bucket.cc Show resolved Hide resolved

ivancich added feature needs-qa labels Sep 12, 2023

cfsnyder force-pushed the wip-62075 branch from bcf9046 to 57caa23 Compare September 14, 2023 20:35

cfsnyder added 3 commits September 15, 2023 18:43

qa/workunits/rgw: add tests that reproduce bucket stats inconsistency…

b79dcf6

… bugs Signed-off-by: Cory Snyder <[email protected]>

rgw: fix rgw versioned bucket stat accounting during reshard and chec…

152aadb

…k index Fixes: https://tracker.ceph.com/issues/62760 Signed-off-by: Cory Snyder <[email protected]>

cfsnyder force-pushed the wip-62075 branch from 57caa23 to 152aadb Compare September 15, 2023 18:47

github-actions bot added the documentation label Sep 15, 2023

cfsnyder added the wip-cfsnyder-testing label Sep 21, 2023

cfsnyder removed the wip-cfsnyder-testing label Sep 22, 2023

cbodley approved these changes Sep 22, 2023

View reviewed changes

cbodley merged commit 9b7cd7f into ceph:main Sep 22, 2023

cfsnyder mentioned this pull request Sep 22, 2023

rgw: fix radosgw-admin bucket check stat calculation bug #53607

Merged

14 tasks

This was referenced Oct 4, 2023

pacific: rgw: add radosgw-admin bucket check olh/unlinked commands #53808

Merged

quincy: rgw: add radosgw-admin bucket check olh/unlinked commands #53821

Merged

reef: rgw: add radosgw-admin bucket check olh/unlinked commands #53823

Merged

Comments

Conversation

cfsnyder commented Jul 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Uh oh!

cbodley left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cbodley commented Aug 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cfsnyder commented Sep 1, 2023

Uh oh!

cfsnyder commented Sep 1, 2023

Uh oh!

mattbenjamin commented Sep 11, 2023

Uh oh!

ivancich left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cbodley commented Sep 12, 2023

Uh oh!

cfsnyder commented Sep 14, 2023

Uh oh!

cfsnyder commented Sep 15, 2023

Uh oh!

cbodley commented Sep 15, 2023

Uh oh!

cfsnyder commented Sep 21, 2023

Uh oh!

cfsnyder commented Sep 22, 2023

Uh oh!

cfsnyder commented Sep 22, 2023

Uh oh!

cfsnyder commented Sep 22, 2023

Uh oh!

cbodley Sep 22, 2023

Choose a reason for hiding this comment

Uh oh!

cbodley commented Sep 23, 2023

Uh oh!

cfsnyder commented Sep 25, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

cfsnyder commented Jul 21, 2023 •

edited

Loading

cbodley commented Aug 2, 2023 •

edited

Loading