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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Sep 30, 2020 at 3:50 AM Jeff Layton <jlayton@xxxxxxxxxx> wrote:
>
> On Tue, 2020-09-29 at 18:44 +0800, Yan, Zheng wrote:
> > On Tue, Sep 29, 2020 at 4:55 PM Ilya Dryomov <idryomov@xxxxxxxxx> wrote:
> > > On Tue, Sep 29, 2020 at 10:28 AM Yan, Zheng <ukernel@xxxxxxxxx> wrote:
> > > > On Fri, Sep 25, 2020 at 10:08 PM Jeff Layton <jlayton@xxxxxxxxxx> 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.
>

The session in rejected state is new session. It should hold no caps.

> 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@xxxxxxxxxx>
>



[Index of Archives]     [CEPH Users]     [Ceph Large]     [Ceph Dev]     [Information on CEPH]     [Linux BTRFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux