Skip to content

mgr: block registration of a module's client#51169

Closed
ajarr wants to merge 3 commits intoceph:mainfrom
ajarr:ajarr-fix-58294
Closed

mgr: block registration of a module's client#51169
ajarr wants to merge 3 commits intoceph:mainfrom
ajarr:ajarr-fix-58294

Conversation

@ajarr
Copy link
Contributor

@ajarr ajarr commented Apr 21, 2023

... until the registration is reflected in the new MgrMap received
from the monitor. This guarantees that the monitor can blocklist all
the registered clients of the mgr being dropped by the monitor.
And this also prevents the possible racing of the clients of the mgr
being failed that reconnect on being blocklisted against the clients
of the standby mgr being promoted during mgr failover.

When a module is registering a client, the client is first registered
in the mgr's py_module_registry. Next time the mgr sends a beacon to
the monitor, it requests the monitor to register the client in the
its py_module_registry onto the monitor's MgrMap. The monitor on
receiving the beacon registers the client in the MgrMap, and sends an
updated MgrMap to the mgr. It's possible that a mgr module registers
its client in the py_module_registy, and when waiting for the client
to be registered in the monitor's MgrMap another mgr thread deregisters
the client from the py_module_registry. Here, the mgr will never send
a beacon message requesting for the client to be registered in the
MgrMap as the client is no longer in the py_module_registry. So the
module will be stuck waiting for the MgrMap with the client registered
in it. To prevent such a scenario, error out of the module's client
registration after receiving the new MgrMap from the monitor and
noticing that the client is not in the mgr's py_module_registry.

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

Contribution Guidelines

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
  • 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

@ajarr ajarr requested a review from a team as a code owner April 21, 2023 12:56
@ajarr ajarr requested review from batrick and idryomov April 21, 2023 12:58
@ajarr ajarr changed the title mgr: block registration of a module's client mgr: block registration of a module's client until the client's address shows up in the MgrMap. Apr 21, 2023
@ajarr ajarr changed the title mgr: block registration of a module's client until the client's address shows up in the MgrMap. mgr: block registration of a module's client until the client's address shows up in the MgrMap Apr 21, 2023
@ajarr ajarr marked this pull request as draft April 21, 2023 13:01
@ajarr ajarr force-pushed the ajarr-fix-58294 branch from 2ed02a4 to a0c6502 Compare April 25, 2023 16:25
@ajarr ajarr force-pushed the ajarr-fix-58294 branch from a0c6502 to 0d8a225 Compare May 4, 2023 18:19
@ajarr ajarr changed the title mgr: block registration of a module's client until the client's address shows up in the MgrMap mgr: block registration of a module's client May 4, 2023
@ajarr
Copy link
Contributor Author

ajarr commented May 4, 2023

Based on Patrick's suggestion here #51169 (comment) , I've update the draft.
I think the only pending item is to propagate error to the mgr module during client registration.

Copy link
Member

@batrick batrick left a comment

Choose a reason for hiding this comment

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

almost there!

@ajarr ajarr force-pushed the ajarr-fix-58294 branch from 0d8a225 to 82b3031 Compare May 5, 2023 01:52
@ajarr ajarr force-pushed the ajarr-fix-58294 branch from 82b3031 to 7fad005 Compare May 5, 2023 22:48
@ajarr ajarr marked this pull request as ready for review May 5, 2023 22:50
@ajarr ajarr force-pushed the ajarr-fix-58294 branch from 7fad005 to b86cc0b Compare May 5, 2023 22:57
@ajarr ajarr force-pushed the ajarr-fix-58294 branch from b86cc0b to be919d4 Compare May 9, 2023 17:28
@ajarr ajarr requested a review from batrick May 9, 2023 17:49
Copy link
Member

@batrick batrick left a comment

Choose a reason for hiding this comment

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

Good work!

@ajarr ajarr requested a review from idryomov May 10, 2023 14:17
@github-actions github-actions bot added the stale label Oct 7, 2024
@github-actions
Copy link

github-actions bot commented Nov 6, 2024

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!

@github-actions github-actions bot closed this Nov 6, 2024
@rzarzynski rzarzynski reopened this Nov 8, 2024
@rzarzynski
Copy link
Contributor

Needs rebease.

@github-actions github-actions bot removed the stale label Nov 8, 2024
@github-actions
Copy link

github-actions bot commented Jan 7, 2025

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 github-actions bot added the stale label Jan 7, 2025
@github-actions
Copy link

github-actions bot commented Feb 6, 2025

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!

@github-actions github-actions bot closed this Feb 6, 2025
@batrick batrick removed the stale label Feb 6, 2025
@batrick batrick reopened this Feb 6, 2025
@batrick
Copy link
Member

batrick commented Feb 6, 2025

@ajarr what's left to do for this PR?

@github-actions
Copy link

github-actions bot commented Apr 7, 2025

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 github-actions bot added the stale label Apr 7, 2025
@github-actions
Copy link

github-actions bot commented May 7, 2025

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!

@github-actions
Copy link

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 github-actions bot added the stale label Aug 12, 2025
@github-actions
Copy link

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!

@vshankar
Copy link
Contributor

@ajarr - checking if this is something that you would still be willing to work?

@ajarr
Copy link
Contributor Author

ajarr commented Nov 5, 2025

@ajarr - checking if this is something that you would still be willing to work?

@vshankar feel free to take over. Please see pending comment, #51169 (comment)

@github-actions
Copy link

github-actions bot commented Jan 4, 2026

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
Copy link

github-actions bot commented Feb 3, 2026

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!

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.

10 participants