mds: fix stuck client requests and stale retries of committing requests - #71408
Open
xiubli wants to merge 5 commits into
Open
mds: fix stuck client requests and stale retries of committing requests#71408xiubli wants to merge 5 commits into
xiubli wants to merge 5 commits into
Conversation
Locker::wrlock_start() on a replica inode asks the auth rank for the
scatter when the lock can't be taken directly. When the cluster is
degraded and the auth rank is down, the request is parked on the lock's
WAIT_STABLE waiter list without sending a scatter request and without
queueing any recovery callback, so the only possible wakeup is a lock
state change that will never happen. Every later request touching the
same lock then wedges behind it forever:
client_request(client.4275:2606277 rename #0x1000000001f/f31 ...)
currently failed to wrlock, waiting
The rdlock path in acquire_locks() and remote_wrlock_start() both wait
for the failed rank to come back and retry; do the same here.
Fixes: https://tracker.ceph.com/issues/80084
Fixes: 8ea6e08 ("mds: optimize state check of peer mds")
Signed-off-by: Xiubo Li <xiubo.li@clyso.com>
A request can be re-dispatched from scratch by a stale C_MDS_RetryRequest waiter: a lock waiter registered on an earlier retry (e.g. wrlock_start()'s WAIT_STABLE waiter on a gathering scatterlock) stays queued on the lock, and when the lock state changes -- long after the mutation acquired the lock via another path, projected its linkage changes and submitted the journal -- the waiter fires and re-enters the handler from scratch. The handlers then see their own projected state instead of the pre-operation state they expect on entry and trip their assertions: handle_client_openc() re-enters with a non-null projected linkage and takes the "it existed" branch, which asserts is_rdlocked(&dn->lock); the other create handlers and handle_client_unlink() assert the projected linkage's null/removed state. Seen during failover stress: mds.d crashed in mknod's null-linkage assert, mds.a in openc's is_rdlocked() assert (the core shows the request with retry == 7, mdr->committing set, the XLOCK op still in the mutation's lock list while the lock's own xlock_by bookkeeping was gone) and mds.e in unlink's !dnl->is_null() assert. Once the journal entry is in flight the only correct forward progress is the journal finisher, so a retry of a committing request is always stale: mdr->committing is set by journal_and_reply() and the other submission paths and is never cleared before the request completes, and no journal code path uses C_MDS_RetryRequest. Ignore it in C_MDS_RetryRequest::finish() instead of dispatching. The four create handlers (openc/mknod/mkdir/symlink) are also made tolerant of retry re-entry as defense in depth: - if the create was already journaled (mdr->committing), just return -- the C_MDS_*_finish completion finishes the request; - if the projected linkage is still there and the mutation holds the dentry xlock (checked against the mutation's own lock list with is_xlocked(); the lock's xlock_by bookkeeping cannot be trusted after retry cycles, and the xlock op alone is not enough since every normal create pass holds it before projecting), the projection is its own: resume right after dn->push_projected_linkage() instead of re-running the create (which would double-create) or taking the "it existed" branch (which would reply without journaling the create). The resume path only re-runs idempotent setup: add_old_pool() is a set insert, pre_dirty() version bumps are harmless, issuing caps to the existing Capability is a no-op, and the EUpdate is rebuilt from scratch. Fixes: https://tracker.ceph.com/issues/80086 Signed-off-by: Xiubo Li <xiubo.li@clyso.com>
Add a teuthology test for the CephFS MDS request dispatch paths: a POSIX-only metadata load generator (qa/workunits/fs/mdsc_stress/) with a per-operation watchdog and SIGKILLed victim processes, driven by qa/tasks/cephfs/test_mdsc_stress.py which fails two distinct MDS ranks back-to-back while the load is in flight, plus occasional ceph.dir.pin re-rolls for cross-rank forwards. Runs on both kclient and ceph-fuse mounts; on the kernel client the test additionally scans the client dmesg for kernel splats (list corruption, KASAN/UAF, lockdep reports, hung task warnings). Verdicts: load exit code (1 = stuck request, 2 = unexpected errno), HUNG lines in the load log, and kernel splats. Fixes: https://tracker.ceph.com/issues/80084 Fixes: https://tracker.ceph.com/issues/80086 Signed-off-by: Xiubo Li <xiubo.li@clyso.com>
The retry after an MDS health-warning refusal (MDS_TRIM or MDS_CACHE_OVERSIZED) spelled the override flag with a double dash, so the retry always failed with an invalid-command error instead of forcing the failover. Signed-off-by: Xiubo Li <xiubo.li@clyso.com>
The kernel client only accepts AES (CEPH_CRYPTO_AES) cephx keys, but the vstart keyring may hold a client key created with the preferred cipher (aes256k), which makes every kernel mount fail with EINVAL in add_key. For kernel client runs, allow the insecure cipher on the monitors and rotate each client key to aes, refreshing the local keyring entry, mirroring what the kclient task already does in qa/tasks/kclient.py. Signed-off-by: Xiubo Li <xiubo.li@clyso.com>
Member
Author
|
This could be reproduced 100% with my test script in the trackers when doing the mds failover with workload. |
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
1 participant
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.
Under double-failover stress (two ranks failed back-to-back while
metadata load is in flight), two request-dispatch bugs surface.
Fix both, and add the teuthology test that reproduces them.
mds: retry wrlock on a replica when the scatter auth is down
(#80084)
Locker::wrlock_start()on a replica inode asks the auth rank forthe scatter when the lock can't be taken directly. When the auth
rank is down, the request is parked on the lock's WAIT_STABLE waiter
list without sending a scatter request and without queueing any
recovery callback — the only possible wakeup is a lock state change
that will never happen, and every later request touching the same
lock wedges behind it:
The rdlock path and
remote_wrlock_start()already wait for theAlso fixes a regression from 8ea6e08 ("mds: optimize state
check of peer mds").
mds: ignore retries of committing requests
(#80086)
A stale
C_MDS_RetryRequestwaiter can re-dispatch a request fromscratch after it has already projected its linkage changes and
submitted the journal entry. The handlers re-enter, see their own
projected state instead of the pre-operation state, and trip their
assertions (mknod's null-linkage assert, openc's is_rdlocked()
assert, unlink's !is_null() assert — all seen crashing MDSes during
failover stress).
Since
mdr->committingis set byjournal_and_reply()and nevercleared before the request completes, and no journal code path uses
C_MDS_RetryRequest, a retry of a committing request is alwaysstale: the only valid forward progress is the journal finisher.
Ignore it in
C_MDS_RetryRequest::finish(), and make the createhandlers (openc/mknod/mkdir/symlink) tolerant of retry re-entry as
defense in depth.
QA
Teuthology port of the r2 reproducer (
run_mdsc_test.sh -p r2):qa/workunits/fs/mdsc_stress/mdsc_stress.c— POSIX-only metadataload generator: 8 workers running a weighted mix of 19 metadata
operations (write-heavy, fsync-light, so unsafe requests replay on
reconnect), a watchdog reporting operations stuck in flight
(#80084's symptom), and victim processes SIGKILLed and restarted
every 200–2000 ms.
qa/tasks/cephfs/test_mdsc_stress.py—test_load_only(smoke)and
test_double_failover(240 s of back-to-back rank failuresplus occasional
ceph.dir.pinre-rolls, then settle and dumpin-flight operations). Runs on kernel client and ceph-fuse; on
kclient it also scans dmesg for kernel splats.
qa/suites/fs/functional/tasks/mdsc-stress.yaml— fs:functionalregistration with an ignorelist for expected failover noise.
Two small qa framework fixes found while running locally:
--yes-i-really-mean-ittypo inrank_fail(), and vstart_runnerkernel client key handling/blocklist check.
Fixes: https://tracker.ceph.com/issues/80084
Fixes: https://tracker.ceph.com/issues/80086
Contribution Guidelines
To sign and title your commits, please refer to Submitting Patches to Ceph.
If you are submitting a fix for a stable branch (e.g. "quincy"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.
When filling out the below checklist, you may click boxes directly in the GitHub web UI. When entering or editing the entire PR message in the GitHub web UI editor, you may also select a checklist item by adding an
xbetween the brackets:[x]. Spaces and capitalization matter when checking off items this way.Checklist
Show available Jenkins commands
jenkins test classic perfJenkins Job | Jenkins Job Definitionjenkins test crimson perfJenkins Job | Jenkins Job Definitionjenkins test signedJenkins Job | Jenkins Job Definitionjenkins test make checkJenkins Job | Jenkins Job Definitionjenkins test make check arm64Jenkins Job | Jenkins Job Definitionjenkins test submodulesJenkins Job | Jenkins Job Definitionjenkins test dashboardJenkins Job | Jenkins Job Definitionjenkins test dashboard cephadmJenkins Job | Jenkins Job Definitionjenkins test apiJenkins Job | Jenkins Job Definitionjenkins test docsReadTheDocs | Github Workflow Definitionjenkins test ceph-volume allJenkins Jobs | Jenkins Jobs Definitionjenkins test windowsJenkins Job | Jenkins Job Definitionjenkins test rook e2eJenkins Job | Jenkins Job DefinitionYou must only issue one Jenkins command per-comment. Jenkins does not understand
comments with more than one command.