mds: wait reintegrate to finish when unlinking#49553
Conversation
|
jenkins test api |
kotreshhr
left a comment
There was a problem hiding this comment.
The code looks good but could you explain in the commit message what's the actual deadlock that's being hit with out the fix ?
|
1, In one MDS when evaling the dentry it will send a
And then the And in 3, One client sends a unlink request to the same dentry, but it will do the remote authpin:
And then the unlink request is added into the waiter list too, but before that this dentry's state was set to Since in Step2 the authpin for dentry in 4, Then the 5, So the |
vshankar
left a comment
There was a problem hiding this comment.
FWIW, test run https://pulpito.ceph.com/vshankar-2023-03-08_15:12:36-fs-wip-vshankar-testing-20230308.112059-testing-default-smithi/7197021/ has the same (fsstress.sh) hanging, but its only running a single active mds.
| class ScrubStack; | ||
| class C_ExecAndReply; | ||
|
|
||
| struct MDSMetaRequest { |
There was a problem hiding this comment.
Is this really required? Cannot the MClientRequest (for the rename op to the peer mds) be enough for tracking?
There was a problem hiding this comment.
in Client.cc it will use the MetaRequest {} to track the inflight requests in each session. While the MClientRequest will only be used to do the real request to MDS, we cannot tracker the request with this.
This is why I added MDSMetaRequest in MDS side.
| } | ||
|
|
||
| if (client_inst.name.is_mds() && reply->get_op() == CEPH_MDS_OP_RENAME) { | ||
| mds->send_message(reply, mdr->client_request->get_connection()); |
There was a problem hiding this comment.
Hmmm.. the reintegration (rename) operation was never replied back to the peer (request initiator) mds.
This time the This should be another different issue : The kernel have the same issue with https://tracker.ceph.com/issues/58564, IMO it should be caused by the deadlock issue from kclient. |
|
jenkins retest this please |
|
Rebased it. |
|
Rebased it and updated the commit comments. |
|
jenkins test make check |
|
jenkins test api |
|
jenkins test windows |
|
(after removing a problematic PR) |
There are 3 fsstress.sh workunit failures with a couple of them being timeouts (ret: 124). Looking at those logs, the failures do not resemble the hangs that were seen without this change, but we'd need to be 100% sure that nothing else breaks. I'll have a deeper look when I get a bit more time. I would suggest @lxbsz to have a look too. |
I will check it today. |
Fixes: https://tracker.ceph.com/issues/58340 Signed-off-by: Xiubo Li <xiubli@redhat.com>
This will allow the MDS could handle the MClientReply requests. Fixes: https://tracker.ceph.com/issues/58340 Signed-off-by: Xiubo Li <xiubli@redhat.com>
When unlinking a remote dentry while primary dentry has been just unlinked and was trying to reintegrate, which will finally be a RENAME client request to current MDS, the stray dentry. If the RENAME request is sleeping and waiting for some resources just after authpin the CInode, so the unlinking request will mark the remote dentry as UNLINKING state and then add itself to the wait list just waiting for the CInode to be authpined. Then after the RENAME request is retried later it will add to a wait list again and waiting for the UNLINKING state to be cleared. And finally the RENAME and UNLINK requests will be deadlock. This fix will block the UNLINK request if the reintegrate_stray is going on for current dentry. Fixes: https://tracker.ceph.com/issues/58340 Signed-off-by: Xiubo Li <xiubli@redhat.com>
The failure was introduced by the new commit cdb2275. I will remove it for now. This is a separated issue and will fix it in a later PR. |
Sorry, I misreaded the logs. It should be a new issue. Such as for https://pulpito.ceph.com/pdonnell-2023-05-11_23:23:58-fs-wip-pdonnell-testing-20230511.185220-distro-default-smithi/7272082/, there have some slow requests, but the slow requests were caused by the recursive stray reintergrating. Note: the above When the first stray dn reintegrating is triggered and when received the And in Checked the logs I found that: The This should happen: 1, create a I still have no idea why we couldn't see this before ? How this is related to this PR ? @vshankar Could you have a double check ? Thanks |
|
...
Checked this again and this should be a new issue: In the multiple The And then in But currently the dentry of In It will trigger a new While this PR doesn't change this logic. |
|
It should be a similar issue with cdb2275, the last commit. And it didn't fix all cases. I just remove it from this PR for now. And will fix it later in one separate PR. |
* refs/pull/49553/head: mds: wait migrate to finish when linking mds: wait reintegrate to finish when unlinking message: make MClientReply inherit MMDSOp mds: notify the waiters in replica MDSs
|
@lxbsz fyi, I included your PR in this run: https://pulpito.ceph.com/pdonnell-2023-05-19_20:26:49-fs-wip-pdonnell-testing-20230519.164114-distro-default-smithi/ |
|
jenkins test make check |
|
jenkins test make check arm64 |
* refs/pull/49553/head: mds: wait reintegrate to finish when unlinking message: make MClientReply inherit MMDSOp mds: notify the waiters in replica MDSs
Thanks @batrick, there have 2 The For the other failures I didn't check them all one by one, but mostly also caused by tracker#61148, such as the following Thanks |
|
jenkins test make check |
1 similar comment
|
jenkins test make check |
@batrick @lxbsz please go ahead and merge the PRs if the test runs a fine. |
|
jenkins test make check |
|
@lxbsz @batrick FYI, I still see fsstress failures with multimds: https://pulpito.ceph.com/vshankar-2023-06-15_04:58:28-fs-wip-vshankar-testing-20230614.124123-testing-default-smithi/7305053/ I'm going over the logs to see if its a new issue or a corner case which is still lurking around. |
I just checked one failure, it's not the same issue: The [EDIT] The client has dropped the But in the mds side, it didn't correctly evaled the locker state: It was caused by seqs mismatched between |
There is another I created a tracker to fix this https://tracker.ceph.com/issues/61818. |
Just before unlinking one dentry and another MDS triggerred a
reintegrate_stray request, which will finally be a RENAME client
request to current MDS. The rename and unlink requests will deadlock.
This fix will block the unlink the request if the reintegrate_stray
is going for current dentry.
Fixes: https://tracker.ceph.com/issues/58340
Fixes: https://tracker.ceph.com/issues/59657
Signed-off-by: Xiubo Li xiubli@redhat.com
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. "pacific"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.
Checklist
Show available Jenkins commands
jenkins retest this pleasejenkins test classic perfjenkins test crimson perfjenkins test signedjenkins test make checkjenkins test make check arm64jenkins test submodulesjenkins test dashboardjenkins test dashboard cephadmjenkins test apijenkins test docsjenkins render docsjenkins test ceph-volume alljenkins test ceph-volume toxjenkins test windows