Ceph » CephFS

Hmm, ok. This may be related to another bug I've been chasing where umount hangs waiting for the session to close. I wonder if this is the same issue...

The kclient never appears to retransmit a CEPH_SESSION_REQUEST_CLOSE request on a session. One and done:

static int __close_session(struct ceph_mds_client *mdsc,                                            
                         struct ceph_mds_session *session)                                          
{                                                                                                   
        if (session->s_state >= CEPH_MDS_SESSION_CLOSING)                                           
                return 0;                                                                           
        session->s_state = CEPH_MDS_SESSION_CLOSING;                                                
        return request_close_session(session);                                                      
}                                                                

I'm not that familiar yet with the seq number handling in the client. Is there some documentation on how this is intended to work? Under what circumstances should the client retransmit a request that might have been dropped? AFAICT, the client doesn't ever drop an MDS request due to s_seq being too old. Should it be doing that?

If the client sent the request with too old a s_seq value, then probably the cap request came in after that was sent, and we never resend the request. I guess I should look at libcephfs and see if it does this more sanely...

Jeff Layton wrote:

Hmm, ok. This may be related to another bug I've been chasing where umount hangs waiting for the session to close. I wonder if this is the same issue...

The kclient never appears to retransmit a CEPH_SESSION_REQUEST_CLOSE request on a session. One and done:

[...]

I'm not that familiar yet with the seq number handling in the client. Is there some documentation on how this is intended to work?

Not to my knowledge. This code path has been mostly unchanged since Ceph started: https://github.com/ceph/ceph/blob/c1865445c5b88cae2df499e4d2047af156225177/src/mds/Server.cc#L163-L168

Under what circumstances should the client retransmit a request that might have been dropped? AFAICT, the client doesn't ever drop an MDS request due to s_seq being too old. Should it be doing that?

I don't think so. Only the MDS increments the sequence number.It would be a bug in the MDS if there were an older seq number I would think. Maybe the kernel client should detect that and issue warnings?

If the client sent the request with too old a s_seq value, then probably the cap request came in after that was sent, and we never resend the request. I guess I should look at libcephfs and see if it does this more sanely...

Yes, that'd be a good idea. It may be a bug in both clients. It's weird we're only seeing this for the first time in the last few months.

FWIW, the push sequence is one way the MDS apparently figures out where the client is in processing the MDS messages. We only use that in a few places in the mds (see `git grep -E get_push_seq\|get_seq`). Mostly for cap handling.

Doesn't look like libcephfs does anything saner:

  while (!mds_sessions.empty()) {                                                                   
    // send session closes!                                                                         
    for (auto &p : mds_sessions) {                                                                  
      if (p.second.state != MetaSession::STATE_CLOSING) {                                           
        _close_mds_session(&p.second);                                                              
        mds_ranks_closing.insert(p.first);                                                          
      }                                                                                             
    }                                                                                               

    // wait for sessions to close                                                                   
    double timo = cct->_conf.get_val<std::chrono::seconds>("client_shutdown_timeout").count();      
    ldout(cct, 2) << "waiting for " << mds_ranks_closing.size() << " mds session(s) to close (timeout: " 
                  << timo << "s)" << dendl;                                                         
    std::unique_lock l{client_lock, std::adopt_lock};                                               

...and it looks like the timeout defaults to 2m there vs. 60s in kernel client. Basically, it's just sending the REQUEST_CLOSE once and then giving up after the wait times out.

A simple MDS-side fix might be to just have the MDS ignore the seq on a REQUEST_CLOSE. The client's not going to decide to keep the session after that point anyway.

On the client side, we could just retransmit the request several times until we get a reply, with a small delay between transmissions. It's not pretty but it would probably work.

Jeff Layton wrote:

Doesn't look like libcephfs does anything saner:

[...]

...and it looks like the timeout defaults to 2m there vs. 60s in kernel client. Basically, it's just sending the REQUEST_CLOSE once and then giving up after the wait times out.

A simple MDS-side fix might be to just have the MDS ignore the seq on a REQUEST_CLOSE. The client's not going to decide to keep the session after that point anyway.

I think the concern is that hte client could conceivably dirty the cap the MDS just issued. I don't really think that makes sense though.

On the client side, we could just retransmit the request several times until we get a reply, with a small delay between transmissions. It's not pretty but it would probably work.

Perhaps the MDS is the right thing to fix but I'm not sure on the implications. We'll need Zheng's input on this.

Patrick Donnelly wrote:

I think the concern is that hte client could conceivably dirty the cap the MDS just issued. I don't really think that makes sense though.

Where we send a REQUEST_CLOSE is pretty close to final. In most cases, we're tearing the superblock down at that point. Nothing should be getting dirtied after that.

I'll see if I can draft something up to do some retries.

I have a patch I'm testing now that seems to also anecdotally fix some of the umount hangs I've seen lately during xfstests runs. I wonder if we ought to combine that with some patches to make the client better vet incoming messages vs. the s_seq. I worry a bit though that the rules on this are vague.

Another idea might be to have the client ignore most messages when the state is >= CEPH_MDS_SESSION_CLOSING. We would still need to bump the s_seq for those messages too. FWIW, it doesn't seem like the session sequence is bumped for every message. On what sorts of messages is it incremented?

Jeff, this is just waiting on kcephfs patches now right?