mds/quiesce: let clients keep their buffered writes for a quiesced file #56755
Conversation
leonid-s-usov
force-pushed
the
wip-lusov-quiesce-xlock
branch
from
b2e28f5
to
a6584ff
Compare
|
I wasn't able to reproduce the scenario on the vossi server with |
|
@batrick this shouldn't be merged, I haven't tested it with multiple ranks :( |
leonid-s-usov
force-pushed
the
wip-lusov-quiesce-xlock
branch
from
fc422e3
to
a9e20a2
Compare
leonid-s-usov
force-pushed
the
wip-lusov-quiesce-xlock
branch
2 times, most recently
from
92a796c
to
1be0a7e
Compare
leonid-s-usov
dismissed
batrick’s stale review
revoking this review due to the code change
src/mds/MDCache.cc
Outdated
| // NB: this will also wrlock the versionlock | ||
| lov.add_xlock(&in->filelock); | ||
| if (in->is_auth()) { | ||
| lov.add_rdlock(&in->policylock); /* for the quiesce_block xattr test */ |
There was a problem hiding this comment.
replica mds needs this lock too?
There was a problem hiding this comment.
I don't think so, since it's a SimpleLock, so it's mirrored
There was a problem hiding this comment.
hmm... you are right; but this makes me wonder if we have an issue here with a window where the auth and the replica may see different values of the quiesce_block xattr for the same quiesce.
Imagine there is a race of the quiesce xattr set and a quiesce. One of the ranks could catch the value before it's updated on the other rank, which could order its quiesce of the node after the change.
There was a problem hiding this comment.
The only way I can think of that will prevent the inconsistency is that we don't release the policylock, at least not until the quiesce is complete on all ranks.
There was a problem hiding this comment.
OK I don't think we have the time for this. We'll just have to mention in the docs (are there docs about it?) that the quiesce.block shouldn't be set during quiesces, for now.
Though before we add it to the docs, I'd still like to rename it to quiesce.skip cause block still throws me off, TBH.
There was a problem hiding this comment.
Alternatively, we could remove this feature until we re-introduce it consistently.
There was a problem hiding this comment.
Or, how about this: it appears that the policylock doesn't help us with the race of a quiesce and setting of this flag. So, to be safe, the user would need to change this flag and then re-issue a quiesce. If that's the case, we may as well not even bother taking the policylock :D
There was a problem hiding this comment.
If the lock is not in sync state, you cannot safely read it.
Please pull the commit out. I have discovered several bugs surrounding the policylock which I'll post in a separate PR.
There was a problem hiding this comment.
Ack!
We have to figure out a new way of setting the quiesce skip flag to be consistent across the cluster with a racing quiesce. Having a separate PR for that makes a lot of sense.
The current solution seems good enough for the first version of the quiesce, it will work well if we need to enable it in the field as it will most probably mean some more or less static config under support supervision
|
jenkins test windows |
leonid-s-usov
changed the title
leonid-s-usov
force-pushed
the
wip-lusov-quiesce-xlock
branch
from
1be0a7e
to
9775b2e
Compare
With the quiesce protocol taking a `rdlock` on the file, it also revokes the `Fb` capability, which the clients can't release until they are done flushing, and that may take up arbitrarily long, evidently, more than 10 minutes. We went for the rdlock to avoid affecting readonly clients, but given the evidence above we should not optimize for those. Ideally, we’d like to have a QUIESCE file lock mode where both rd and buffer are allowed, but as of now it seems like our best available option is to `xlock` the file which will let the writing clients keep their buffers for the duration of the quiesce. We can only afford this change for a `splitauth` config, i.e. where we drop the lock immediately after all `Fw`s are revoked Signed-off-by: Leonid Usov <leonid.usov@ibm.com>
For every mirrored lock, the auth will message the replica to ensure the replicated lock state. When we take x/rdlock on the auth, it will ensure the LOCK_LOCK state on the replica, which has the file caps we want for quiesce: CACHE and BUFFER. It should be sufficient to only hold the quiesce local lock on the replica side. Signed-off-by: Leonid Usov <leonid.usov@ibm.com>
leonid-s-usov
force-pushed
the
wip-lusov-quiesce-xlock
branch
from
9775b2e
to
eac482b
Compare
|
@batrick here is the latest quiesce test run: https://pulpito.ceph.com/leonidus-2024-04-16_05:41:33-fs-wip-lusov-quiesce-xlock-distro-default-smithi/ The same test has failed multiple times, namely This test passes for me locally. It's important to note that the above suite was running on a version with the I will schedule a new run with the latest code (without the policylock commit), but for now it's important to conclude whether the test run was good enough for merging downstream at https://gitlab.cee.redhat.com/ceph/ceph/-/merge_requests/592. |
|
jenkins test windows |
1 similar comment
|
jenkins test windows |
|
This PR is under test in https://tracker.ceph.com/issues/65530. |
@batrick that run seems doomed for whatever reason. Would you consider this sufficient to merge? |
Yes, I was just including it in the branch I was building because I couldn't merge to main with required jenkins test running and I wanted to include it in my tests. |
With the quiesce protocol taking a `rdlock` on the file, it also revokes the `Fb` capability, which the clients can't release until they are done flushing, and that may take up arbitrarily long, evidently, more than 10 minutes. We went for the rdlock to avoid affecting readonly clients, but given the evidence above we should not optimize for those. Ideally, we’d like to have a QUIESCE file lock mode where both rd and buffer are allowed, but as of now it seems like our best available option is to `xlock` the file which will let the writing clients keep their buffers for the duration of the quiesce. We can only afford this change for a `splitauth` config, i.e. where we drop the lock immediately after all `Fw`s are revoked Signed-off-by: Leonid Usov <leonid.usov@ibm.com> (cherry picked from commit 8ac9842) Fixes: https://tracker.ceph.com/issues/65556 Original-Issue: https://tracker.ceph.com/issues/65472 Original-PR: #56755
For every mirrored lock, the auth will message the replica to ensure the replicated lock state. When we take x/rdlock on the auth, it will ensure the LOCK_LOCK state on the replica, which has the file caps we want for quiesce: CACHE and BUFFER. It should be sufficient to only hold the quiesce local lock on the replica side. Signed-off-by: Leonid Usov <leonid.usov@ibm.com> (cherry picked from commit eac482b) Fixes: https://tracker.ceph.com/issues/65556 Original-Issue: https://tracker.ceph.com/issues/65472 Original-PR: #56755
The analysis of https://bugzilla.redhat.com/show_bug.cgi?id=2273935 has shown that a client may build up a significant backlog of flushes in the buffers.
With the quiesce protocol taking a
rdlockon the file, it also revokes theFbcapability, which the clients can't release until they are done flushing, and that may take up arbitrarily long, evidently, more than 10 minutes.We went for the rdlock to avoid affecting readonly clients, but given the evidence above we should not optimize for those. Ideally, we’d like to have a QUIESCE file lock mode where both rd and buffer are allowed, but as of now, it seems like our best available option is to
xlockthe file which will let the writing clients keep their buffers for the duration of the quiesce.We can only afford this change for a
splitauthconfig, i.e. where we drop the lock immediately after allFws are revokedShow 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 windowsjenkins test rook e2e