Subject: RE: Stateid and seqid

From: "Noveck, Dave" <[email-masked]>
Date: Thu, 1 Feb 2001 07:00:18 -0800

> > 
> > >    If the operation fails and the state doesn't change, then the
> > >      seqid changes.
> > 
> > The current approach is that seqid does not change if the operation
> > fails.  That's what people are implementing to, but there has been
> > disagreement about whether the spec says that (e.g. Mike Eisler believes

> > it does)

> ButButButButButButButButButButButButButBut

[Example deleted]

> Net result: client thinks that it locked "that other bit" and server
> thinks that it has "this bit" locked.  Not a good situation.

> Am I missing something?

I don't think so.

I've already changed this once in my code, it looks like I'm going to
have to change it again. :-(

I propose we go back to incrementing the seqid on normal errors (ones
that allow you to find the proper lockowner so that you can increment 
the seqid).  Does everybody who will be testing a v4 client or server
at Connectathon agree that this is the basis on which we will (attempt
to) interoperate?

---

From: "Kendrick M. Smith" <[email-masked]>
Date: Thu, 1 Feb 2001 15:13:59 -0500 (EST)

> at Connectathon agree that this is the basis on which we will (attempt

Because of Neil Brown's example, I agree that this is necessary.  We will
have to agree on a list of precisely which errors do not trigger
incrementation of the seqid.  We will definitely need
	NFS4ERR_STALE_CLIENTID
	NFS4ERR_STALE_STATEID
	NFS4ERR_BAD_STATEID
	NFS4ERR_BAD_SEQID.
What about NFS4ERR_OLD_STATEID?  This error is only returned if the
server can locate the lockowner structure corresponding to the stateid, in
which case it can increment the seqid, so it seems safe to leave this
error off the list.  In general, it seems most prudent to leave errors off
the list whenever possible, to minimize the risk of further
NealBrown-esque examples (even though in the specific case of
NFS4ERR_OLD_STATEID, I think it is easy to argue that no such examples
exist).  What about NFS4ERR_EXPIRED?  This error is a little troublesome 
because it can be retured in three distinct cases: if a client's lease has
expired, if the locking state of a file has been manually changed by the
server's administrator, or if a delegation has been revoked unbeknownst to
the client.  Can we guarantee that, in all these cases, the server will be
able to locate the lockowner structure to increment the seqid?  If so, I
think that this error should also be left off the list.  Are there any
other errors which should be considered for the list?

On Wed, 31 Jan 2001, Noveck, Dave wrote:

> Sergei Klyushin wrote:
> > I would not check and renew lease on replay also.
> > It would be consistent with RPC layer cashing.   
> > Not renewing lease on replay could cause some problems: if NFS client
> > finally
> > receives reply, the next request may get ERR_EXPIRED, but using RENEW
> could
> > help,
> > and my feeling, that it's better, than keeping file opened
(locked,...)
> > forever, plus
> > it will be consistent with RPC layer cashing.
> 
> I agree.  When a client first issues a request at time X and receives a
> response at time Y, all it can assume is that the server renewed the   
> lease at some time between X and Y, and he should schedule his RENEWAL 
> logic accordingly.  (The case of relativistic server-client relative   
> motion is left as an exercise for the (not-yet-but-soon-to-be) truly   
> deranged :-).
> 
> > What about lease? I would NOT renew lease on ANY error also.
>
> That seems troublesome to me.  The purpose of the renewal mechanism
> is so that the server can know the client is still up and responsive.
> There may be situations in which many requests will get errors
> because of server conditions.  It seems wrong to me to say that
> in such a situation, the server must give up the ability to keep
> track of the leases of clients.  Of course that means that we   
> have to define what errors will renew and those that won't.  Sigh.

I think that we can either agree that only certain errors trigger lease
renewal or that no errors do, as long as the client and server use the 
same convention.  Even if leases are never renewed on error, this only
costs an extra RENEW operation if all state-manipulating operations return
an error during the lease period (which seems like an uncommon case
anyway).  I don't think that there are any correctness issues associated
with lease renewal on error, like there are with seqid incrementation.
For this reason, my vote is to have the lease renewed only for successful
operations; I think that the performance impact is negligable here, so we
should use whichever convention is simplest.

Kendrick

---

From: "Sergey Klyushin" <[email-masked]>
Date: Thu, 1 Feb 2001 17:23:00 -0500

> -----Original Message-----
    > From: Kendrick M. Smith [mailto:[email-masked]]
    > Sent: Thursday, February 01, 2001 3:14 PM
    > To: [email-masked]
    > Subject: RE: Stateid and seqid
    >
    >
    >
I've already agreed since my email from 1/31/2001. I've posted some changes
to
Andy's diagram, please, review it and change as much as you want to make it
better. This diagram separates OPEN_CONFIRM processing and adds checking for
confirmed seqid, but doesn't cover all correct seqid bumping.

There is another question about stateid processing (which is not covered in
diagram
also). All operations that take stateid also have current file handle to
OPENED file
(in case of OPEN_CONFIRM it must be also the same file seqid started from).
But
stateid also tied to a file. So, Server should check stateid first and if
it's good
or old should save new seqid. Secondary, Server should check if current file
handle
refer to the same file as stateid. If not, what error should be returned?
INVAL?
What if it's the same file, but closed. I guess that it's ERROR_SERVERFAULT,
because
sateid should be different for OPENED and CLOSED files (state changed). In
both
cases sequence is OK. Probably, current file handle as "parameter" kept from
the time,
when stateid was per lockowner, but not lockowner+file. Should we keep it?
READ and WRITE are special cases, because of stateid of all 0 or all 1.
    > exist).
The only case when NFS4ERR_OLD_STATEID is generated in our implementation is
old_stateid+good_seq_num+confirmed_seq_num. So, I would keep this error out
of the
list and save new seqid on server side and increment it on client side also
(even though it's different from my diagram). Lease should be renewed also.
    > What about NFS4ERR_EXPIRED?  This error is a little troublesome
I don't think that we need to include NFS4ERR_EXPIRED in the list. This
error means,
that Server was able to find lockowner and found out that lease expired.
Next call
to OPEN with seqid+1 could succeed, if Server did or did not remove
lockowner. If
lockowner was removed OPEN_CONFIRM will be required and new sequence will
start from
seqid+1.
    > > > Not renewing lease on replay could cause some problems:
    > > > if NFS client finally
    > > > could help, and my feeling, that it's better, than keeping file
opened
    > > > (locked,...)forever, plus it will be consistent with RPC layer
cashing.
    > should use whichever convention is simplest.
On second thought, I would agree with Dave to renew lease whenever Server is
sure,
that it's a new request from the client, but not replay. I guess the same
list of
errors is also appropriate here plus NFS4ERR_EXPIRED.

Sergey