Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mon/MDSMonitor: batch last_metadata update with pending #50862

Merged
merged 2 commits into from Jun 15, 2023

Conversation

batrick
Copy link
Member

@batrick batrick commented Apr 5, 2023

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

Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>
I believe the problem here is that the last_metadata change is lost in a
ECANCELED/EAGAIN transaction but the pending map change goes through in
the next one. I've been unable to find an exact way to reproduce this.
The problem seems to occur when upgrades are performed which would
indicate shuffling in the monitors where quorum would be lost repeatedly.

This seems to be the most likely explanation so let's go ahead and make
this change even without the reproducer. In any case, it has the added
benefit of batching the pending map update (to up:standby) with the
last_metadata update.

Fixes: https://tracker.ceph.com/issues/24403
Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>
@batrick batrick requested a review from a team as a code owner April 5, 2023 02:15
@github-actions github-actions bot added cephfs Ceph File System core mon labels Apr 5, 2023
@rzarzynski
Copy link
Contributor

Haven't gone over the weeds of MDSMonitor but from the overall monitor perspective a regression, if any, should be contained within MDS.

batrick added a commit to batrick/ceph that referenced this pull request May 9, 2023
* refs/pull/50862/head:
	mon/MDSMonitor: batch last_metadata update with pending
	mon/MDSMonitor: add debugging for update_metadata
batrick added a commit to batrick/ceph that referenced this pull request May 18, 2023
* refs/pull/50862/head:
	mon/MDSMonitor: batch last_metadata update with pending
	mon/MDSMonitor: add debugging for update_metadata
batrick added a commit to batrick/ceph that referenced this pull request May 20, 2023
* refs/pull/50862/head:
	mon/MDSMonitor: batch last_metadata update with pending
	mon/MDSMonitor: add debugging for update_metadata
@batrick
Copy link
Member Author

batrick commented May 23, 2023

jenkins test make check arm64

@batrick
Copy link
Member Author

batrick commented May 24, 2023

@batrick batrick requested a review from a team May 24, 2023 21:02
@batrick
Copy link
Member Author

batrick commented May 24, 2023

@mchangir can you review this please?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants