squid: mds: use regular dispatch for processing beacons#57682
Merged
joscollin merged 4 commits intoceph:squidfrom Jun 17, 2024
Merged
squid: mds: use regular dispatch for processing beacons#57682joscollin merged 4 commits intoceph:squidfrom
joscollin merged 4 commits intoceph:squidfrom
Conversation
For order checking. Signed-off-by: Patrick Donnelly <pdonnell@redhat.com> (cherry picked from commit 1b93fd5)
So we can ensure that e.g. MDSRank::ms_dispatch is lowest priority so that we do not acquire the mds_lock when looking at beacons. This change maintains the current behavior when the priority is unset: the use of std::stable_sort will ensure that the add_dispatcher_head and add_dispatcher_tail calls will preserve order when dispatcher priorities are equal. Fixes: 7fc04be Signed-off-by: Patrick Donnelly <pdonnell@redhat.com> (cherry picked from commit b463d93)
Similar to the issue with MClientMetrics, beacons should also not be handled via fast dispatch because it's necessary to acquire Beacon::mutex. This is a big no-no as it may block one of the Messenger threads leading to improbable deadlocks or DoS. Instead, use the normal dispatch where acquiring locks is okay to do. Fixes: 7fc04be See-also: linux.git/f7c2f4f6ce16fb58f7d024f3e1b40023c4b43ff9 Fixes: https://tracker.ceph.com/issues/65658 Signed-off-by: Patrick Donnelly <pdonnell@redhat.com> (cherry picked from commit 7fc2a65)
This tries to preserve existing order but uses priorities to make it explicit and robust to future dispatchers being added. Except: - The beacon and metrics dispatcher have the highest priorities. This is to ensure we process these messages before trying to acquire any expensive locks (like mds_lock). - The monc dispatcher also has a relatively high priority for the same reasons. This change affects other daemons which may have ordered a dispatcher ahead of the monc but I cannot think of a legitimate reason to nor do I see an instance of it. Fixes: 7fc04be Signed-off-by: Patrick Donnelly <pdonnell@redhat.com> (cherry picked from commit 3291f39)
Member
|
Tested in https://tracker.ceph.com/issues/66423 |
Member
Author
joscollin
approved these changes
Jun 17, 2024
joscollin
pushed a commit
to joscollin/ceph
that referenced
this pull request
Jun 27, 2024
* refs/pull/57682/head: mds: set dispatcher order mds: use regular dispatch for processing beacons msg: add priority to dispatcher invocation order mds: note when dispatcher is called
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
backport tracker: https://tracker.ceph.com/issues/66196
backport of #57469
parent tracker: https://tracker.ceph.com/issues/66014
this backport was staged using ceph-backport.sh version 16.0.0.6848
find the latest version at https://github.com/ceph/ceph/blob/main/src/script/ceph-backport.sh