rgw: fix get_obj_state_impl wrongly set s->has_attrs = true by shashalu · Pull Request #60957 · ceph/ceph

shashalu · 2024-12-05T03:28:36Z

rgw: fix get_obj_state_impl wrongly set s->has_attrs = true

While RGWRados::raw_obj_stat return ENOENT,
s->has_attrs shouldnot set to true

Signed-off-by: [Shasha Lu] lu.shasha@aishu.cn

Contribution Guidelines

To sign and title your commits, please refer to Submitting Patches to Ceph.
If you are submitting a fix for a stable branch (e.g. "quincy"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.
When filling out the below checklist, you may click boxes directly in the GitHub web UI. When entering or editing the entire PR message in the GitHub web UI editor, you may also select a checklist item by adding an x between the brackets: [x]. Spaces and capitalization matter when checking off items this way.

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
jenkins test rook e2e

Signed-off-by: Shasha Lu <lu.shasha@aishu.cn>

cbodley · 2024-12-05T14:55:37Z

src/rgw/driver/rados/rgw_rados.cc

  if (r == -ENOENT) {
    s->exists = false;
-    s->has_attrs = true;
+    s->has_attrs = false;


can you please clarify what breaks as a result of this?

looking back through git history, i see that it's been this way since 2011 though i'm not sure why: b03930f

while upload a multipart,
1.the multipart obj data was written first
2.after all shadow obj has been written, will do write_meta for multipart obj, which only write some xatt
In RGWRados::Object::Write::write_meta, assume_noent is true, in RGWRados::get_obj_state_impl wouldnot actually do raw_obj_stat, only set s->exists = false, s->has_attrs = true
3.if write_meta write xattr failed, then ~RadosWriter() will remove those objs, when remove multipart obj. Because of s->has_attrs = true, s->exists = false, the delete_obj will return ENOENT, thus multipart obj leaked.

actually i have no idea why https://github.com/ceph/ceph/commit/b03930fa02295f8068195f30312ff2d6fac50991 set s->has_attrs = true either？

cbodley

thanks @shashalu. still not sure about this, but we'll run through qa to see whether anything breaks 🤞

ivancich · 2024-12-12T19:22:58Z

jenkins test make check arm64

ivancich · 2024-12-12T19:23:41Z

@shashalu Please fill out the checklist and add a tracker if need be. I've added this PR to a QA run.

ivancich · 2024-12-18T03:49:52Z

I believe this PR is causing this teuthology error, which can be found here:

https://qa-proxy.ceph.com/teuthology/ivancich-2024-12-18_02:37:18-rgw-wip-eric-testing-2-distro-default-smithi/8043007/teuthology.log

2024-12-18T03:34:26.157 INFO:teuthology.orchestra.run.smithi122.stdout:=================================== FAILURES ===================================
2024-12-18T03:34:26.157 INFO:teuthology.orchestra.run.smithi122.stdout:______________________ test_versioning_obj_suspended_copy ______________________

... elided...

2024-12-18T03:34:26.159 INFO:teuthology.orchestra.run.smithi122.stdout:> assert body == content
2024-12-18T03:34:26.159 INFO:teuthology.orchestra.run.smithi122.stdout:E AssertionError: assert 'content-0' == 'null content'
2024-12-18T03:34:26.159 INFO:teuthology.orchestra.run.smithi122.stdout:E
2024-12-18T03:34:26.159 INFO:teuthology.orchestra.run.smithi122.stdout:E - null content
2024-12-18T03:34:26.159 INFO:teuthology.orchestra.run.smithi122.stdout:E + content-0
2024-12-18T03:34:26.159 INFO:teuthology.orchestra.run.smithi122.stdout:
2024-12-18T03:34:26.159 INFO:teuthology.orchestra.run.smithi122.stdout:s3tests_boto3/functional/test_s3.py:7722: AssertionError

github-actions · 2025-02-17T19:01:33Z

This pull request has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs for another 30 days.
If you are a maintainer or core committer, please follow-up on this pull request to identify what steps should be taken by the author to move this proposed change forward.
If you are the author of this pull request, thank you for your proposed contribution. If you believe this change is still appropriate, please ensure that any feedback has been addressed and ask for a code review.

github-actions · 2025-03-19T20:01:48Z

This pull request has been automatically closed because there has been no activity for 90 days. Please feel free to reopen this pull request (or open a new one) if the proposed change is still appropriate. Thank you for your contribution!

cbodley · 2025-03-19T21:43:47Z

while upload a multipart,
1.the multipart obj data was written first
2.after all shadow obj has been written, will do write_meta for multipart obj, which only write some xatt
In RGWRados::Object::Write::write_meta, assume_noent is true, in RGWRados::get_obj_state_impl wouldnot actually do raw_obj_stat, only set s->exists = false, s->has_attrs = true
3.if write_meta write xattr failed, then ~RadosWriter() will remove those objs, when remove multipart obj. Because of s->has_attrs = true, s->exists = false, the delete_obj will return ENOENT, thus multipart obj leaked.

i think there's still something to fix here. we may just need to work through the unintended consequences of the change

ceph-jenkins · 2025-03-19T21:43:50Z

Can one of the admins verify this patch?

github-actions · 2025-05-18T23:02:01Z

This pull request has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs for another 30 days.
If you are a maintainer or core committer, please follow-up on this pull request to identify what steps should be taken by the author to move this proposed change forward.
If you are the author of this pull request, thank you for your proposed contribution. If you believe this change is still appropriate, please ensure that any feedback has been addressed and ask for a code review.

github-actions · 2025-06-18T00:06:52Z

This pull request has been automatically closed because there has been no activity for 90 days. Please feel free to reopen this pull request (or open a new one) if the proposed change is still appropriate. Thank you for your contribution!

rgw: fix get_obj_state_impl wrongly set s->has_attrs = true

885e321

Signed-off-by: Shasha Lu <lu.shasha@aishu.cn>

shashalu requested a review from a team as a code owner December 5, 2024 03:28

github-actions bot added the rgw label Dec 5, 2024

cbodley reviewed Dec 5, 2024

View reviewed changes

cbodley approved these changes Dec 6, 2024

View reviewed changes

cbodley added the needs-qa label Dec 6, 2024

ivancich added the wip-eric-testing-2 for ivancich testing label Dec 12, 2024

ivancich removed wip-eric-testing-2 for ivancich testing needs-qa labels Dec 18, 2024

github-actions bot added the stale label Feb 17, 2025

github-actions bot closed this Mar 19, 2025

cbodley reopened this Mar 19, 2025

github-actions bot removed the stale label Mar 19, 2025

github-actions bot added the stale label May 18, 2025

github-actions bot closed this Jun 18, 2025

cbodley mentioned this pull request Jun 18, 2025

RGW | fix conditional Delete, MultiDelete and Put #63348

Merged

14 tasks

Conversation

shashalu commented Dec 5, 2024

Contribution Guidelines

Checklist

Uh oh!

cbodley Dec 5, 2024

Choose a reason for hiding this comment

Uh oh!

shashalu Dec 6, 2024

Choose a reason for hiding this comment

Uh oh!

shashalu Dec 6, 2024

Choose a reason for hiding this comment

Uh oh!

cbodley left a comment

Choose a reason for hiding this comment

Uh oh!

ivancich commented Dec 12, 2024

Uh oh!

ivancich commented Dec 12, 2024

Uh oh!

ivancich commented Dec 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Feb 17, 2025

Uh oh!

github-actions bot commented Mar 19, 2025

Uh oh!

cbodley commented Mar 19, 2025

Uh oh!

ceph-jenkins commented Mar 19, 2025

Uh oh!

github-actions bot commented May 18, 2025

Uh oh!

github-actions bot commented Jun 18, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ivancich commented Dec 18, 2024 •

edited

Loading