Re: [RFC PATCH 0/4] ceph: fix spurious recover_session=clean errors

[Jeff Layton <jlayton@kernel.org>]

On Tue, 2020-09-29 at 18:44 +0800, Yan, Zheng wrote:
> On Tue, Sep 29, 2020 at 4:55 PM Ilya Dryomov <idryomov@gmail.com> wrote:
> > On Tue, Sep 29, 2020 at 10:28 AM Yan, Zheng <ukernel@gmail.com> wrote:
> > > On Fri, Sep 25, 2020 at 10:08 PM Jeff Layton <jlayton@kernel.org> wrote:
> > > > Ilya noticed that he would get spurious EACCES errors on calls done just
> > > > after blocklisting the client on mounts with recover_session=clean. The
> > > > session would get marked as REJECTED and that caused in-flight calls to
> > > > die with EACCES. This patchset seems to smooth over the problem, but I'm
> > > > not fully convinced it's the right approach.
> > > > 
> > > 
> > > the root is cause is that client does not recover session instantly
> > > after getting rejected by mds. Before session gets recovered, client
> > > continues to return error.
> > 
> > Hi Zheng,
> > 
> > I don't think it's about whether that happens instantly or not.
> > In the example from [1], the first "ls" would fail even if issued
> > minutes after the session reject message and the reconnect.  From
> > the user's POV it is well after the automatic recovery promised by
> > recover_session=clean.
> > 
> > [1] https://tracker.ceph.com/issues/47385
> 
> Reconnect should close all old session. It's likely because that
> client didn't detect it's blacklisted.
> 

I should have described this better -- let me explain:

It did detect that it was blocklisted (almost immediately) because the
MDS shuts down the session. I think it immediately sends a
SESSION_REJECT message when blacklisting and indicates that it has been
blocklisted.

At that point the session is CEPH_MDS_SESSION_REJECTED. The next MDS
calls through would see that it was in that state and would return
-EACCES. Eventually, the delayed work runs and then the session gets
reconnected, and further calls proceed normally.

So, I think this is just a timing thing for the most part. The workqueue
job runs on a delay of round_jiffies_relative(HZ * 5);, and that's long
enough for the disruption to be noticeable.

While this was happening during 'ls' for Ilya, it could happen in
anything that involves sending a request to the MDS. I think we want to
prevent new opens from erroring out during this window if we can.

The real question is whether this is safe in all cases. For instance, if
the call that we're idling is dependent on holding certain caps, then
it's possible we will have lost them when we got REJECTED.

Hmm...so that means patch 4/4 is probably wrong. I'll comment further in
a reply to that patch.

> > Thanks,
> > 
> >                 Ilya
> > 
> > > 
> > > > The potential issue I see is that the client could take cap references to
> > > > do a call on a session that has been blocklisted. We then queue the
> > > > message and reestablish the session, but we may not have been granted
> > > > the same caps by the MDS at that point.
> > > > 
> > > > If this is a problem, then we probably need to rework it so that we
> > > > return a distinct error code in this situation and have the upper layers
> > > > issue a completely new mds request (with new cap refs, etc.)
> > > > 
> > > > Obviously, that's a much more invasive approach though, so it would be
> > > > nice to avoid that if this would suffice.
> > > > 
> > > > Jeff Layton (4):
> > > >   ceph: don't WARN when removing caps due to blocklisting
> > > >   ceph: don't mark mount as SHUTDOWN when recovering session
> > > >   ceph: remove timeout on allowing reconnect after blocklisting
> > > >   ceph: queue request when CLEANRECOVER is set
> > > > 
> > > >  fs/ceph/caps.c       |  2 +-
> > > >  fs/ceph/mds_client.c | 10 ++++------
> > > >  fs/ceph/super.c      | 13 +++++++++----
> > > >  fs/ceph/super.h      |  1 -
> > > >  4 files changed, 14 insertions(+), 12 deletions(-)
> > > > 
> > > > --
> > > > 2.26.2
> > > > 

-- 
Jeff Layton <jlayton@kernel.org>