Skip to content

Comments

rgw: fix CompleteMultipart error handling regression#57257

Merged
cbodley merged 3 commits intoceph:mainfrom
cbodley:wip-65746
May 6, 2024
Merged

rgw: fix CompleteMultipart error handling regression#57257
cbodley merged 3 commits intoceph:mainfrom
cbodley:wip-65746

Conversation

@cbodley
Copy link
Contributor

@cbodley cbodley commented May 3, 2024

#54569 moved some CompleteMultipart code around, and introduced a regression in the error path that causes the 'multipart meta object' to be deleted prematurely

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

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

@cbodley
Copy link
Contributor Author

cbodley commented May 3, 2024

the test_ps_s3_multipart_on_master_http test case from #54569 is still passing

@mattbenjamin
Copy link
Contributor

mattbenjamin commented May 3, 2024

could be helpful to have brief comment in the commit messages explaining it's intended effect

cbodley added 2 commits May 3, 2024 15:48
most requests operate directly on s->object. there's no reason to
allocate a separate target_obj for the same purpose

Signed-off-by: Casey Bodley <[email protected]>
get_notification() should be associated with the target object
s->object. the meta_obj has the wrong object name, so required passing
s->object->get_name() as an extra argument

importantly, Notification no longer depends on the lifetime of meta_obj
to avoid a dangling pointer, while the lifetime of s->object is guaranteed

Signed-off-by: Casey Bodley <[email protected]>
move publish_complete() and meta_obj->delete_object() back to execute()
so they only run on success. this allows several member variables to
move back to execute()'s stack as well

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

Signed-off-by: Casey Bodley <[email protected]>
Copy link
Member

@AliMasarweh AliMasarweh left a comment

Choose a reason for hiding this comment

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

Looks good

Copy link
Contributor

@mattbenjamin mattbenjamin left a comment

Choose a reason for hiding this comment

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

lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants