Skip to content

rgw/multisite: Fix lifetime issues#63698

Merged
adamemerson merged 5 commits intoceph:mainfrom
adamemerson:wip-71066
Aug 6, 2025
Merged

rgw/multisite: Fix lifetime issues#63698
adamemerson merged 5 commits intoceph:mainfrom
adamemerson:wip-71066

Conversation

@adamemerson
Copy link
Contributor

@adamemerson adamemerson commented Jun 3, 2025

Fixes: https://tracker.ceph.com/issues/71066
Fixes: https://tracker.ceph.com/issues/72211

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)
Show available Jenkins commands

@adamemerson adamemerson force-pushed the wip-71066 branch 2 times, most recently from a1dab81 to c95d6b0 Compare June 5, 2025 02:06
@adamemerson
Copy link
Contributor Author

jenkins test make check arm64

@adamemerson adamemerson marked this pull request as ready for review June 5, 2025 11:35
@adamemerson adamemerson requested a review from a team as a code owner June 5, 2025 11:35
@adamemerson adamemerson requested a review from cbodley June 5, 2025 11:36
@adamemerson
Copy link
Contributor Author

@cbodley I think this fixes it, at least I can't reproduce it any more.

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.

shared_ptr changes look good 👍

@github-actions
Copy link

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

@adamemerson adamemerson force-pushed the wip-71066 branch 2 times, most recently from 99705ff to a465666 Compare July 2, 2025 21:47
@github-actions
Copy link

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

@adamemerson
Copy link
Contributor Author

Rebase broke something, investigating.

@github-actions
Copy link

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

@adamemerson
Copy link
Contributor Author

jenkins test make check

@adamemerson
Copy link
Contributor Author

jenkins test make check arm64

Asynchrony combined with cancellations keeps leading to occasional
lifetime issues, so follow the best-practices of Asio I/O objects by
having completions keep a reference live.

The original NeoRados backing implements Asio's two-phase shutdown
properly.

The RadosClient backing does not, because it shares an Objecter with
completions that do not belong to it. In practice I don't think this
will matter since librados and neorados get shut down around the same
time.

Signed-off-by: Adam C. Emerson <aemerson@redhat.com>
Also run them in strands. Also `datalog_rados` is a `shared_ptr`,
now. Probably make it intrusive later.

Signed-off-by: Adam C. Emerson <aemerson@redhat.com>
Tag operations with a subsystem so we can cancel them all in one go.

Signed-off-by: Adam C. Emerson <aemerson@redhat.com>
Split nominal handle object and reference-counted
implementation. While we're at it, add lazy-open functionality.

Signed-off-by: Adam C. Emerson <aemerson@redhat.com>
This is slightly ugly but good enough for now. Make sure we can block
when shutting down background tasks.

Remove a few `driver` parameters that are unused. This lets us
simplify the IAM Policy and Lua tests and not construct stores we
never use. (Which is good since we aren't running them under a cluster.)

Signed-off-by: Adam C. Emerson <aemerson@redhat.com>
@adamemerson adamemerson removed the cephfs Ceph File System label Aug 6, 2025
@adamemerson adamemerson merged commit a674144 into ceph:main Aug 6, 2025
13 checks passed
@cbodley
Copy link
Contributor

cbodley commented Aug 6, 2025

yay!

auto num = s.substr(0, pos);
auto ofs = s.substr(pos + 1);

auto n = ceph::parse<decltype(m.num)>(num);
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed here: #64952

Sorry about that.

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