Subject: RE: Changes for CLOSE_ALL operation

From: "Noveck, Dave" <[email-masked]>
Date: Fri, 7 Jan 2000 07:12:34 -0800

> > > > If you can give up on a stateful request (LOCK, LOCKU, 
> OPEN, CLOSE)
> > > > and continue operation of the lockowner (i.e. process), then 
> > > > you have problem.
> > > > 
> > > > One answer, which you might not like, is "Don't do that".
> > > 
> > > Perhaps I'm missing something, but it seems to me that if  
> 
> I missing something but it wasn't fatal. See below.
> 
> > > the client is
> > > capable of maintaining an nfs_lockowner, it is capable of 
> > > maintaining a record
> > > of the last stateful request on that nfs_lockowner that it 
> > > gave up on. Once
> > > the client is ready to continue operation, it's free to 
> > > re-issue the last
> > > request, and if the request is a LOCK or OPEN, issue a LOCKU 
> > > or CLOSE to
> > > nullify it. One could use this strategy to obviate the need 
> > > to issue or a
> > > RELEASE_ALL, though I willing to concede that RELEASE_ALL 
> > > might be simpler for
> > > the common case.
> > 
> > OK.  This works.
> > 
> > I had been worried about seqid management and not knowing the
> > proper stateid for requests that require them.  Your solution
> > is very nice because because it takes advantage of the 
> > existing sequence logic to resolve all that.  If you re-issue
> > a LOCK or OPEN with the original seqid you'll either have it 
> > done if it wasn't or get back the response from the original
> 				^^^^^^^^^^^^^^^^^^^^^
> 
> This is the part that was missing. See below.
> 
> > request, otherwise.  The you have the data to do the CLOSE
> > or LOCKU properly.  In the case of an interrupted LOCKU or
> > CLOSE, the same reissue trick works.  Neat.
> > 
> > Maybe the spec should say something about this, for people
> > (like me) to whom this solution is not as obvious as it 
> > was to you.
> 
> Yes the spec should, but mainly because, on further thought, 
> I also realize
> that the server MUST similarly keep track of the last request 
> it received and
> responded to, including the requests's seqid (which should be 
> obvious from the
> spec), and the request's stateid, and the responses new 
> stateid. Otherwise, we
> are relying on the the duplicate request cache to furnish the correct
> information, and since dup req caches typically have an LRU 
> algorithm, it is
> possible my proposed scheme will fall apart.

I think the spec already requires the necessary stuff.  In
section 8.2.4, it says:

    The server maintains the last sequence number (L) 
    received and the response that was returned.

And then later in the same section, it sayd:

    If a duplicate of a last request (r == L) is received,
    the stored response is returned.

This response will have the stateid that you need.  I
assume that this is all separate from any duplicate 
request cache, since the spec doesn't even acknowledge
the existence of duplicate request caches.

One thing, I was missing, is that CLOSE currently doen't
have a seqid so reissuing a CLOSE will not work the same
way.  I think this is probably OK, since if you interrupt a
close, and then reissue, the CLOSE will either work with
the original stateid, if it hadn't really happened or give
you a NFS4ERR_BAD_STATEID.  Right now, the spec says this
error indicates a logic error so that would have to change.

> 
> Since the server is maintaining state for nfs_lockowner as 
> long as there
> are open files, this is not an onerous requirement. Of 
> course, once the
> client has issued the last CLOSE, RELEASE_ALL, or LOCKU on 
> its last file,
> the server will want to keep the nfs_lockowner around for a 
> while to ensure
> that a retried CLOSE, RELEASE_ALL, LOCKU gets the right 
> results. This is
> sort of analagous to the TCP state machine that has 
> end-points stay in 
> CLOSEWAIT or TIMEWAIT for a while before finally disposing of them.

I think the spec now says that LOCKU should be done before the
CLOSE.  If RELEASE_ALL goes away, then CLOSE is the only issue
and I think that it can be done without requiring the server
to maintain sequence state beyond the point at which there are
no locks, although for pragmatic reasons, it obviously would
like not to get rid of this info immediately.

> 
> How strongly do we feel we need RELEASE_ALL then?
 
I'd say that if can agree that we have a method to recover 
from interrupted requests that works, and we maintain that
agreement for 24 consecutive hours, then I don't see the
need for RELEASE_ALL.

---

From: Mike Eisler <[email-masked]>
Date: Fri, 7 Jan 2000 07:52:17 -0800 (PST)

> have a seqid so reissuing a CLOSE will not work the same
	^^^^^^^^^^^^^^^^^^^^^^^^^
		^^^^^^^^^^^^^^^^^^^^^

> agreement for 24 consecutive hours, then I don't see the

A while back, in some e-mail I sent to David Robinson and others
that argued for a confirmation step in OPEN/SETCLIENTID I noted:

	CLOSE does not contain a sequence number. There might be a risk
	of an old CLOSE removing the share reservation on a file.
	However, because CLOSE has a stateid, the server
	implementation, if it encodes a generation value in stateid,
	can mitigate this.

It seems to me that there are lots of arguments for including a seqid
with CLOSE and few arguments against. It certainly cannot hurt.
Let's drop RELEASE_ALL and add the seqid to CLOSE4args.
Let the 24 hour clock start now.

	-mre