Skip to content

rgw/configstore: don't reinitialize the rados client in RadosRealmWatcher#67287

Merged
cbodley merged 1 commit intoceph:mainfrom
nbalacha:wip-nbalacha-71390
Feb 17, 2026
Merged

rgw/configstore: don't reinitialize the rados client in RadosRealmWatcher#67287
cbodley merged 1 commit intoceph:mainfrom
nbalacha:wip-nbalacha-71390

Conversation

@nbalacha
Copy link
Contributor

@nbalacha nbalacha commented Feb 10, 2026

RadosRealmWatcher::watch_start() was reusing and re-initing a rados client which had been inited when creating the ConfigStore. This led to the original Objecter being lost and valgrind reporting a leak.

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

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

You must only issue one Jenkins command per-comment. Jenkins does not understand
comments with more than one command.

@nbalacha nbalacha requested a review from a team as a code owner February 10, 2026 10:57
@github-actions github-actions bot added the rgw label Feb 10, 2026
@nbalacha nbalacha requested review from cbodley and yuvalif February 10, 2026 10:57
@nbalacha
Copy link
Contributor Author

nbalacha commented Feb 10, 2026

@cbodley , I have changed the RadosRealmWatcher to have its own instance of librados::Rados so as to not overwrite the existing librados::Rados in ConfigImpl. I'm not sure if that is the best way . If the existing ConfigImpl::rados can be re-used, it should not be re-inited.

With these changes valgrind reports:
==00:00:02:08.799 1664370== LEAK SUMMARY:
==00:00:02:08.799 1664370== definitely lost: 0 bytes in 0 blocks
==00:00:02:08.799 1664370== indirectly lost: 0 bytes in 0 blocks
==00:00:02:08.799 1664370== possibly lost: 0 bytes in 0 blocks
==00:00:02:08.799 1664370== still reachable: 36,219 bytes in 1,463 blocks
==00:00:02:08.799 1664370== suppressed: 256,781 bytes in 3,295 blocks

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.

good catch, thanks

The RadosRealmWatcher::watch_start() function incorrectly
reinitialized a rados client that had already been initialized during
the creation of the ConfigStore. This caused the original Objecter
to be lost, which was subsequently reported as a memory leak by Valgrind.

Signed-off-by: Nithya Balachandran <nithya.balachandran@ibm.com>
@nbalacha nbalacha changed the title rgw/configstore: use a different rados client for the RadosRealmWatcher rgw/configstore: don't reinitialize the rados client in the RadosRealmWatcher Feb 10, 2026
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.

looks good, thanks 👍 the rgw/multisite qa suite should catch any regressions here

@nbalacha nbalacha changed the title rgw/configstore: don't reinitialize the rados client in the RadosRealmWatcher rgw/configstore: don't reinitialize the rados client in RadosRealmWatcher Feb 11, 2026
@nbalacha
Copy link
Contributor Author

jenkins test make check

@nbalacha
Copy link
Contributor Author

jenkins test windows

@nbalacha
Copy link
Contributor Author

jenkins test make check arm64

@adamemerson
Copy link
Contributor

jenkins test make check

@adamemerson
Copy link
Contributor

jenkins test make check arm64

@ivancich ivancich added the wip-eric-testing-1 for ivancich testing label Feb 12, 2026
@ivancich
Copy link
Member

1 similar comment
@ivancich
Copy link
Member

@kchheda3
Copy link
Contributor

just FYI,
i had created the tracker https://tracker.ceph.com/issues/74983, to tackle broader issue with our current approach of creating multiple librados client in rgw. and in that tracker i had pointed out this issue or memory leak due to multiple init of librados by realmwatcher.

planning to discuss that tomo on rgw call.

@ivancich
Copy link
Member

jenkins test make check

@cbodley cbodley merged commit f5ae31d into ceph:main Feb 17, 2026
19 checks passed
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.

5 participants