Subject: Re: [nfsv4] Preview of new locking chapter

From: Spencer Shepler <[email-masked]>
Date: Wed, 2 Aug 2006 04:44:36 -0500

On Sun, Noveck, Dave wrote:
> Here is the new revised-for-sessions locking chapter, including the
> changes for SEQUENCE-based notifications and a re-organized treatment of
> lock revocations, that have been proposed previously.  Although the
> reaction to these was generally positive, there wasn't enough discussion
> to really declare a consensus and so I promised everybody a chance to
> look at the new locking chapter, before it went into an official draft.

After reviewing this chapter I have a few comments.

Overall, I believe that the contents of the chapter need to be split
into three areas of discussion.  The first would be the protocol's
state and how it is instantiated, manipulated, and destroyed/released.
This would be in normal operation.  I would like to refer to this
as "state" instead of locks specifically to avoid any mis-interpretations
of locks and byte-range locks.

Following this would be a discussion of the failure/recovery mechanisms
and their impact on the state (much of what is there in the chapter
already.

Finally, a discussion of shares and byte-range locks.

The reason for this is that these three areas are intermixed because
of the structure we had in rfc3530 and was never cleaned up.

A couple of comments that are not directly related to the specifics
of the locking chapter but were things I thought the WG had agreed to
and we need to confirm that and then find a home for them.

The first is the issue of having the principal used at OPEN be the
same one that is used for later use of the resultant stateid.
Simply, does the same user have to use the stateid on READ/WRITE 
as did the OPEN.

The second item was a proposal that Bruce had made with respect
to "blocking" lock requests.  Did we decide to pursue that?

For specific comments... 

Is there a scalability issue, that we care about, with the introduction
of TEST_STATEID and FREE_BADLOCK?  If the client receives the error
indication on the SEQUENCE op results, how does it go about clearing
the condition?  At the moment, it seems that the client must wonder
through all of its state using TEST_STATEID until the error in 
SEQUENCE goes away.  I suppose that client's could essentially
ignore that condition until it received and error on the revoked
state and then did the FREE_BADLOCK.

Also, RELEASE_LOCKOWNER should not be made mandatory not to implement.
The LOCK request with the open_to_lock_owner4 argument served
two purposes.  The first was to establish the sequence number (which
we do not need with the use of sessions) but it also associates the
lock_owner with the open_owner.  If RELEASE_LOCKOWNER is not available,
the client will need to deal with error returns for regular
LOCK requests or make every call with the open_to_lock_owner4 type.
The RELEASE_LOCKOWNER operation should remain and used by the client
when it knows that the lockowner is no longer pursuing locks or makes
the determination itself (because of resource limits) that it can be 
released.

Spencer

_______________________________________________
nfsv4 mailing list
[email-masked]
https://www1.ietf.org/mailman/listinfo/nfsv4

---

From: "J. Bruce Fields" <[email-masked]>
Date: Wed, 2 Aug 2006 10:04:12 -0400

> to "blocking" lock requests.  Did we decide to pursue that?

Yes.  I'll try to get that written soon.

It's entirely backwards-compatible: no new protocol is required, and
even 4.0 servers and clients could implement it without affecting
interoperability.  So I think that makes it somewhat less urgent.

(As an entirely separate issue, I still think we should have a lock
notify callback (NOT a grant).  Nobody agreed with me last time I
suggested it, but I'll probably write it up at some point anyway just so
people can make fun of me....)

--b.

---

From: "J. Bruce Fields" <[email-masked]>
Date: Wed, 16 Aug 2006 18:46:19 -0400

> > to "blocking" lock requests.  Did we decide to pursue that?

Here you are--

This gives us some of the advantages of the NLM "CANCEL" call without
the ugly races.  Also, note that this mechanism is completely backwards
compatible with NFSv4.0, and thus the recommendations can be applied
equally well to 4.0 implementions.

---
 nfsv41_middle_filelocking.xml |   14 +++++++++++++-
 1 files changed, 13 insertions(+), 1 deletions(-)

diff --git a/nfsv41_middle_filelocking.xml b/nfsv41_middle_filelocking.xml
index 1c84cde..7411363 100644
--- a/nfsv41_middle_filelocking.xml
+++ b/nfsv41_middle_filelocking.xml
@@ -675,7 +675,19 @@
       burden of needlessly frequent polling for blocking locks.  The server
       should take care in the length of delay in the event the client
       retransmits the request.
-
+    </t>
+    <t>
+      If a server receives a blocking lock request, denies it, and then
+      later receives a nonblocking request for the same lock, which is
+      also denied, then it should remove the lock in question from its list of
+      pending blocking locks.  Clients should use such a nonblocking request
+      to indicate to the server that this is the last time they intend to poll
+      for the lock, as may happen when the process requesting the lock is
+      interrupted.  This is a courtesy to the server, to prevent it from
+      unnecessarily waiting a lease period before granting other lock requests.
+      However, clients are not required to perform this courtesy, and servers
+      must not depend on them doing so.  Also, clients must be prepared for
+      the possibility that this final locking request will be accepted.
     </t>
   </section>
   <section anchor="lease_renewal" title="Lease Renewal" >
-- 
1.4.1.rc2

---

From: "J. Bruce Fields" <[email-masked]>
Date: Wed, 16 Aug 2006 19:03:44 -0400

And, while we're at it, just for fun:  Here's a CB_NOTIFY_LOCK that's a
hint to the client that now might be a good time to retry a lock that
it's blocking on.

Why is this so brilliant?  Well!:

	- It gives cooperating clients the chance to acquire locks immediately
	  after they're released, without having to set ridiculously small
	  polling intervals.
	- It doesn't actually modify any locking state on its own, so
	  it has none of the raciness and annoying sequencing requirements of
	  a real grant callback.
	- It's entirely optional--servers can omit it, clients can
	  ignore it.
	- It's simple to specify and to implement.

Neat-o.

--b.

22.11.  Operation 13: CB_NOTIFY_LOCK - Notify of possible lock
        availability

22.11.1.  SYNOPSIS

   fh, lockowner -> ()

22.11.2.  ARGUMENT

   struct CB_NOTIFY_LOCK4args {
       nfs_fh4     file;
       lock_owner4 lock_owner;
   }

22.11.3.  RESULT

   struct CB_NOTIFY_LOCK4res {
           nfsstat4        status;
   };

22.11.4.  DESCRIPTION

   The server may use this operation to indicate that a lock for the
   given file and lockowner may have become available.

   This callback is meant to be used by servers to help reduce the
   latency of blocking locks in the case where they recognize that a
   client which has been polling for a blocking lock may now be able to
   acquire the lock.  The notification is purely a hint, provided as a
   possible performance optimization, and is not required for
   correctness.

22.11.5.  IMPLEMENTATION

   The server must not grant the lock to the client unless and until it
   recieves an actual lock request from the client.  Similarly, the
   client receiving this callback cannot assume that it now has the
   lock, or that a subsequent request for the lock will be succesful.

   The server is not required to implement this callback, and even if it
   does, it is not required to use it in any particular case.  Therefore
   the client must still rely on polling for blocking locks, as
   described in the "Blocking Locks" section.

   Similarly, the client is not required to implement this callback, and
   even it does, is still free to ignore it.  Therefore the server must
   not assume that the client will act based on the callback.

---
 index.html                          |    1 +
 nfsv41_middle_op_cb_notify_lock.xml |   61 +++++++++++++++++++++++++++++++++++
 2 files changed, 62 insertions(+), 0 deletions(-)

diff --git a/index.html b/index.html
new file mode 100644
index 0000000..b9322ff
--- /dev/null
+++ b/nfsv41_middle_op_cb_notify_lock.xml
@@ -0,0 +1,61 @@
+<!-- Copyright (C) The Internet Society (2006) -->
+<section anchor="OP_CB_NOTIFY_LOCK" title="Operation 13: CB_NOTIFY_LOCK - Notify of possible lock availability" >
+  <section toc="exclude" anchor="OP_CB_NOTIFY_LOCK_SYNOPSIS" title="SYNOPSIS">
+    <figure>
+      <artwork>
+fh, lockowner -> ()
+      </artwork>
+    </figure>
+  </section>
+  <section toc="exclude" anchor="OP_CB_NOTIFY_LOCK_ARGUMENT" title="ARGUMENT">
+    <figure>
+      <artwork>
+struct CB_NOTIFY_LOCK4args {
+    nfs_fh4     file;
+    lock_owner4 lock_owner;
+}
+      </artwork>
+    </figure>
+  </section>
+  <section toc="exclude" anchor="OP_CB_NOTIFY_LOCK_RESULT" title="RESULT">
+    <figure>
+      <artwork>
+struct CB_NOTIFY_LOCK4res {
+        nfsstat4        status;
+};
+      </artwork>
+    </figure>
+  </section>
+  <section toc="exclude" anchor="OP_CB_NOTIFY_LOCK_DESCRIPTION" title="DESCRIPTION">
+    <t>
+     The server may use this operation to indicate that a lock for the given
+     file and lockowner may have become available.
+    </t>
+    <t>
+     This callback is meant to be used by servers to help reduce the latency of
+     blocking locks in the case where they recognize that a client which has
+     been polling for a blocking lock may now be able to acquire the lock.
+     The notification is purely a hint, provided as a possible performance
+     optimization, and is not required for correctness.
+    </t>
+  </section>
+  <section toc="exclude" anchor="OP_CB_NOTIFY_LOCK_IMPLEMENTATION" title="IMPLEMENTATION">
+    <t>
+     The server must not grant the lock to the client unless and until it
+     recieves an actual lock request from the client.  Similarly, the client
+     receiving this callback cannot assume that it now has the lock, or that a
+     subsequent request for the lock will be succesful.
+    </t>
+    <t>
+     The server is not required to implement this callback, and even if it
+     does, it is not required to use it in any particular case.  Therefore the
+     client must still rely on polling for blocking locks, as described in the
+     "Blocking Locks" section.
+    </t>
+    <t>
+     Similarly, the client is not required to implement this callback, and even
+     it does, is still free to ignore it.  Therefore the server must not assume
+     that the client will act based on the callback.
+    </t>
+  </section>
+</section>
-- 
1.4.1.rc2