mds/quiesce: xlock the file to let clients keep their buffered writes

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 8ac98428769cf45a8d43431ad0fbefe8cb953f06)
Fixes: https://tracker.ceph.com/issues/65556
Original-Issue: https://tracker.ceph.com/issues/65472
Original-PR: https://github.com/ceph/ceph/pull/56755

diff --git a/src/mds/MDCache.cc b/src/mds/MDCache.cc
index 0226ce32f719a70ebeb56c9a86f29c2b2a4dc0ac..a8e6ef50c1463752c9a32bc34b795d8bf3e3e519 100644 (file)
--- a/src/mds/MDCache.cc
+++ b/src/mds/MDCache.cc
@@ -13619,10 +13619,32 @@ void MDCache::dispatch_quiesce_inode(const MDRequestRef& mdr)
 
   if (!(mdr->locking_state & MutationImpl::ALL_LOCKED)) {
     MutationImpl::LockOpVec lov;
+
+    lov.add_xlock(&in->quiescelock); /* !! */
+
+    if (splitauth) {
+      // xlock the file to let the Fb clients stay with buffered writes.
+      // While this will unnecesarily revoke rd caps, it's not as
+      // big of an overhead compared to having the Fb clients flush
+      // their buffers, which evidently can lead to the quiesce timeout
+      // We'll drop the lock after all clients conform to this request
+      // so the file will be still readable during the quiesce after
+      // the interested clients receive their Fr back
+      //
+      // NB: this will also wrlock the versionlock
+      lov.add_xlock(&in->filelock);
+    } else {
+      // if splitauth == false then we won't drop the lock after acquisition (see below)
+      // we can't afford keeping it as xlock for a long time, so we'll have to deal
+      // with the potential quiesce timeout on high-load systems.
+      // The reason we're OK with this is that splitauth is enabled by default,
+      // and really should not be ever disabled outside of the test setups
+      // TODO: consider removing the `splitauth` config option completely.
+      lov.add_rdlock(&in->filelock);
+    }
+    // The rest of caps-related locks - rdlock to revoke write caps
     lov.add_rdlock(&in->authlock);
-    lov.add_rdlock(&in->filelock);
     lov.add_rdlock(&in->linklock);
-    lov.add_xlock(&in->quiescelock); /* !! */
     lov.add_rdlock(&in->xattrlock);
     if (!mds->locker->acquire_locks(mdr, lov, nullptr, {in}, false, true)) {
       return;
@@ -13648,6 +13670,10 @@ void MDCache::dispatch_quiesce_inode(const MDRequestRef& mdr)
        */
       mds->locker->drop_lock(mdr.get(), &in->authlock);
       mds->locker->drop_lock(mdr.get(), &in->filelock);
+      // versionlock will be taken automatically for the file xlock.
+      // We don't really need it, but it doesn't make sense to
+      // change the Locker logic just for this flow
+      mds->locker->drop_lock(mdr.get(), &in->versionlock);
       mds->locker->drop_lock(mdr.get(), &in->linklock);
       mds->locker->drop_lock(mdr.get(), &in->xattrlock);
     }