Skip to content

squid: mds: use regular dispatch for processing beacons#57682

Merged
joscollin merged 4 commits intoceph:squidfrom
batrick:wip-66196-squid
Jun 17, 2024
Merged

squid: mds: use regular dispatch for processing beacons#57682
joscollin merged 4 commits intoceph:squidfrom
batrick:wip-66196-squid

Conversation

@batrick
Copy link
Member

@batrick batrick commented May 23, 2024

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

batrick added 4 commits May 23, 2024 15:38
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)
@batrick batrick requested a review from a team as a code owner May 23, 2024 19:38
@batrick batrick added this to the squid milestone May 23, 2024
@batrick batrick added the cephfs Ceph File System label May 23, 2024
@joscollin
Copy link
Member

Tested in https://tracker.ceph.com/issues/66423

Copy link
Member

@joscollin joscollin left a comment

Choose a reason for hiding this comment

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

@batrick @vshankar
Does this need a rados suite run?

@batrick
Copy link
Member Author

batrick commented Jun 14, 2024

@batrick @vshankar Does this need a rados suite run?

no

@joscollin joscollin merged commit 0144dd5 into ceph:squid 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
@batrick batrick deleted the wip-66196-squid branch July 16, 2024 18:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cephfs Ceph File System core mon

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants