Fix MDS shutdown deadlocks and timeouts#60320
Fix MDS shutdown deadlocks and timeouts#60320MaxKellermann wants to merge 4 commits intoceph:mainfrom
Conversation
Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
This eliminates the `wait_for()` delay and speeds up MDS shutdown. Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
This fixes a deadlock bug during MDS shutdown. Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
During shutdown, the MDS sends a `MSG_MDS_BEACON` with `MDSMap::STATE_DNE` (in `MDSDaemon::suicide()`) and then waits for a `MSG_MDS_BEACON` reply from the MON. The MON, however, suppresses replies to `STATE_DNE`; in `MDSMonitor::preprocess_beacon()`, it returns early on `STATE_DNE` and `MDSMonitor::prepare_beacon()` silently evicts the dying MDS without any reply. This delays the MDS shutdown until the MDS times out. Since `MDSDaemon::suicide()` has code to wait for a beacon reply, I figure that the MON reply was suppressed accidently, therefore I suggest adding it. Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
batrick
left a comment
There was a problem hiding this comment.
Pretty much every commit should be a separate PR. If appropriate for backport, each PR should have a tracker ticket:
Please create a tracker issue so that this issue can be tracked for backporting.
Then, please annotate the commit which fixes/resolves a Ceph tracker issue with:
Fixes: http://tracker.ceph.com/issues/...
This is essential when examining the history of the repository (this commit fixes what) and helps merge scripts identify issues that have been resolved by a merge. See this article on GitHub on how to amend commits and update your pull request.
| _notify_mdsmap(mdsmap); | ||
|
|
||
| sender = std::thread([this]() { | ||
| ceph_pthread_setname(pthread_self(), "beacon"); |
There was a problem hiding this comment.
is this just a cleanup patch or does it have some useful side-effect?
There was a problem hiding this comment.
Both. The useful side effect is that top/gdb/etc. show a proper thread name. No effect for the process itself.
Please help me understand this policy. Even if this is a single PR, there are still 4 distinct commits, and each commit can be identified to fix one thing. What does splitting into 4 PRs change for that? |
|
I split this PR into 4 separate PRs, each with one commit. Not my taste, but if you prefer it that way... |
It simplifies backporting. We don't necessarily want to backport all of these changes. |
|
I don't get that - you can easily backport single commits with |
This PR fixes several bugs that delay the MDS process shutdown by several seconds. With these changes, the MDS process exits almost instantly.
Checklist