Subject: [nfsv4] persistent sessions and compound atomicity

From: [email-masked] (J. Bruce Fields)
Date: Tue, 8 Dec 2020 19:26:40 -0500

I hadn't given much thought to this language in RFC 5661, and now that I
do, it surprises me.  If you have any pointers to discussion I forgot or
overlooked, I'd be interested:

	https://tools.ietf.org/html/rfc5661#section-2.10.6.5

	The execution of the sequence of operations (starting with
	SEQUENCE) and placement of its results in the persistent cache
	MUST be atomic.  If a client retries a sequence of operations
	that was previously executed on the server, the only acceptable
	outcomes are either the original cached reply or an indication
	that the client ID or session has been lost (indicating a
	catastrophic loss of the reply cache or a session that has been
	deleted because the client failed to use the session for an
	extended period of time).

	A server could fail and restart in the middle of a COMPOUND
	procedure that contains one or more non-idempotent or
	idempotent-but-modifying operations. This creates an even higher
	challenge for atomic execution and placement of results in the
	reply cache....

And I don't see why that MUST is necessary.  Why can't the server, on
encountering a nonidempotent operation, store partial compound results
in its reply cache together with whatever compound state (current
filehandles, etc.) it needs to resume processing on restart?

Or, why not allow it to return the result so far and then DEADSESSION on
any following operation?

One problem is that a create operation followed by a GETATTR really
shouldn't fail in between the two--that'd make it hard to implement
O_CREAT, for example.

But it seems like a server should be able to avoid that without making
every sa_cachethis operation completely atomic.

Am I missing some reason for that MUST?

--b.

---

From: David Noveck <[email-masked]>
Date: Wed, 9 Dec 2020 07:46:40 -0500

> overlooked, I'd be interested:

I don't know of any but. I have comments that might be helpful.

>         MUST be atomic.

Although this is ambiguous, I don't think the intention here is to say the
execution of the Compound needs to be a atomic.  The text is citing two
things that must be atomic.  It would normally be reasonable to infer that
each of them must be atomic.  However, the difficulties of doing that,
given that most individual ops don't need to be atomic. means I think it is
not intended meaning.  An alternative interpretation is that the transition
between an existing Compound and a completed compound must be atomic.

If a client retries a sequence of operations
>         extended period of time).

In my view, the atomic transition is between encountering an executing
compound and getting the cached reply for a completed compound.

>         reply cache...

As I read that,it seems an option and not a requirement.

> filehandles, etc.) it needs to resume processing on restart?

I don't see any indication the server can't do that.

> any following operation

> I don't think DEAD SESSION is right for a persistent session. I think you
> can and should return DELAY. Server restart is not quick.

> shouldn't fail in between the two--that'd make it hard to implement

It shouldn't but I don't see raising that to a MUST NOT or SHOULD NOT.

> every sa_cachethis operation completely atomic.

Yes and if that choice were made, the spec would have to a whole lot more
explicit about the matter.

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

---

From: Tom Talpey <[email-masked]>
Date: Wed, 9 Dec 2020 08:55:03 -0500

> filehandles, etc.) it needs to resume processing on restart?

I think the atomicity is referring to the building of the response,
not the execution of the COMPOUND itself (which would be impossible,
generally).

The main issue it's protecting is the server's response never changing.
The operation must always have one result. If the client were to replay
a request while the server is still executing the compound, and get a
partial answer, followed by the final answer, that would be very bad
indeed.

> Or, why not allow it to return the result so far and then DEADSESSION on

"So far"? That only makes sense if the compound were totally complete.
At which point, there is one and only one result.

Tom.

---

From: "J. Bruce Fields" <[email-masked]>
Date: Wed, 9 Dec 2020 09:42:09 -0500

> compound and getting the cached reply for a completed compound.

OK, got it, I think that makes sense.

--b.

---

From: [email-masked] (J. Bruce Fields)
Date: Wed, 9 Dec 2020 09:49:27 -0500

> partial answer, followed by the final answer, that would be very bad

Got it, thanks.

> At which point, there is one and only one result.

Hm, I'm not sure what you mean.  The server comes up from a crash and
determines that it was in the middle of processing a compound, and that
the last thing it does was succesfully execute a nonidempotent op in the
middle of the compound.  it has to decide what it's going to use as the
final reply for that compound.

--b.

---

From: Tom Talpey <[email-masked]>
Date: Wed, 9 Dec 2020 10:29:23 -0500

> final reply for that compound.

In the persistent restarted server case, I agree with you. I thought
you were somehow arguing that the server could provide a reply from
cache, for an in-progress operation.

In the persistence case, I guess this means the server will build the
compound response in-progress, and keep it "secret" as long as it's
partial. On restart, it will need to sweep the cache, add any
necessary error for the compounds that didn't get executed, and mark
it complete and ready for responding whne the client replays.

That seems like a lot of fiddly stuff, but I guess it's important
for any non-idempotent operations. I bet there's more language that
needs tightening up.

Tom.

---

From: Rick Macklem <[email-masked]>
Date: Wed, 9 Dec 2020 16:43:59 +0000

>>>     extended period of time).
I think this is what others are already saying, but just in case...

I would say that the ATOMIC refers to the individual operations of the
compound. In other words, it must cache the reply to each operation
when it is completed, in the persistent cache.

Then, if a retry of the compound is received after a server reboot, the
reply for the first K operations that were completed prior to the reboot
are generated from the cache and the remaining N-K are performed on
the file system to complete the reply. (At least for the non-idempotent
operations in the compound.)

For this to all work correctly, I think the server file system would need to perform
each non-idempotent operation ATOMICALLY as well, such that after the
reboot the file system is in either the pre-operation or post-operation state.

rick, who has no intention of ever trying to implement a persistent session cache.

Tom.

---

From: "J. Bruce Fields" <[email-masked]>
Date: Wed, 9 Dec 2020 16:03:41 -0500

> >>>Or, why not allow it to return the result so far and then DEADSESSION on

Hm, maybe allowing DEADSESSION for operations other than SEQUENCE is
just too weird, and DELAY would work just as well.

> it complete and ready for responding whne the client replays.

Yeah.  I think the server should at least execute any immediately
following GETFH.

It could just go on executing the rest of the compound, but I wonder
whether the results could be weirder than a client is prepared to deal
with.  I don't have a good example, though.  (PUTHF+OPEN+WRITE where the
OPEN succeeds with the old state and then the WRITE returns a new
verifier?  A compound with two LOCKs where the first succeeds and the
second returns GRACE?)

> for any non-idempotent operations. I bet there's more language that

Probably.

--b.

---

From: Tom Talpey <[email-masked]>
Date: Wed, 9 Dec 2020 16:22:24 -0500

> just too weird, and DELAY would work just as well.

That's an interesting idea! DELAY basically says "busy, try again" and
means the client's retry will pick up where the client chooses to.

On the other hand, returning DELAY in the middle of the compound is
kind of awkward, it hands the mess back. But I guess that can happen
in the protocol. I will resist repeating my rant that DELAY is the
spawn of the devil and should never have gone into NFSv3, much less
survived until today.

> verifier?  A compound with two LOCKs where the first succeeds and the

That idea occurred to me too. It's arguably the most transparent and
correct thing the server could do for the client. But it would also
delay server availability - it can't begin serving until all such
operations were complete.

I don't see how the last example should be possible. The GRACE timer
is determined by the server. Calling it in the middle of a compound,
where the server shouldn't even have been running the grace timer,
is beyond the pale.

Tom.

---

From: "J. Bruce Fields" <[email-masked]>
Date: Wed, 9 Dec 2020 17:46:09 -0500

> where the server shouldn't even have been running the grace timer,

So the server could come up, do its journal replay and reload its DRC or
whatever, and finish any in-progress compounds as part of that process.
And then only after its done all that, start the grace timer and start
accepting new RPCs.  OK, that probably makes sense.

Well, this is still all a little theoretical for now.

--b.

---

From: "J. Bruce Fields" <[email-masked]>
Date: Wed, 9 Dec 2020 17:59:44 -0500

On Wed, Dec 09, 2020 at 04:43:59PM +0000, Rick Macklem wrote:
> Then, if a retry of the compound is received after a server reboot,
> the reply for the first K operations that were completed prior to the
> reboot are generated from the cache and the remaining N-K are
> performed on the file system to complete the reply. (At least for the
> non-idempotent operations in the compound.)
> 
> For this to all work correctly, I think the server file system would
> need to perform each non-idempotent operation ATOMICALLY as well, such
> that after the reboot the file system is in either the pre-operation
> or post-operation state.

Great, sounds like everyone agrees, I'll calm down.

> rick, who has no intention of ever trying to implement a persistent

I can't decide.  It looks like kind of a pain.  It'll require some
cooperation with the filesystem folks--I think it needs to be integrated
with filesystem journaling somehow.

But I'm not looking forward to someone running into this, and having to
say, "oh yeah, we've known about that bug for decades; just give me
another few months or a year, and I'll fix it right up for you...."

--b.

---

From: David Noveck <[email-masked]>
Date: Thu, 10 Dec 2020 17:07:27 -0500

> Great, sounds like everyone agrees, I'll calm down.

I don't see the need for individual ops to be atomic especially since they
(e.g. writes) are not defined as atomic in general.

I don't think we have total agreement but we are moving in the same
direction.  No reason to be anything but calm.
.

> > rick, who has no intention of ever trying to implement a persistent

Your input is still valuable.

> with filesystem journaling somehow.

That makes sense.

> another few months or a year, and I'll fix it right up for you...."

I have a revised version of this section in draft-dnoveck-rfc5661bis-00.
(Will be out before next year).  I hope we will discuss that and get to a
text that everybody does agree on, and might even be implementable :-)

---

From: Trond Myklebust <[email-masked]>
Date: Thu, 10 Dec 2020 21:23:52 -0500

On Thu, 2020-12-10 at 17:07 -0500, David Noveck wrote:
> I don't see the need for individual ops to be atomic especially since
> they (e.g. writes) are not defined as atomic in general.
> 

in Section 2.10.6.5 of RFC5661, there is the following snippet:

   One way to
   view the problem is as a single transaction consisting of each
   operation in the COMPOUND followed by storing the result in
   persistent storage, then finally a transaction commit.  If there is a
   failure before the transaction is committed, then the server rolls
   back the transaction.  If the server itself fails, then when it
   restarts, its recovery logic could roll back the transaction before
   starting the NFSv4.1 server.

Why shouldn't a client implementer reading the above be able to
conclude that a NFS4ERR_DEADSESSION error implies that the operation
failed with no side-effects?

> . 
> > 
> > > rick, who has no intention of ever trying to implement a
> > persistent
> > > session cache.
> 
> I have a revised version of this section in draft-dnoveck-rfc5661bis-
> 00.  (Will be out before next year).  I hope we will discuss that and
> get to a text that everybody does agree on, and might even be
> implementable :-)
> > 
> > --b.
> > 

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
[email-masked]

---

From: David Noveck <[email-masked]>
Date: Fri, 11 Dec 2020 06:12:35 -0500

> in Section 2.10.6.5 of RFC5661, there is the following snippet:

Deleted snippet about making the execution of a COMPOUND a transaction.

>
>
> Why shouldn't a client implementer reading the above be able to conclude
> that a NFS4ERR_DEADSESSION error implies that the operation failed with no
> side-effects?
>

No reason at all.  The problem is that requiring the server to implement
COMPOUNDs as atomic transactions if it implements persistent sessions has
no real justification and probably cannot be implemented.

Also I don't think it is right for the sever to send DEADSESSION since the
persistent session is not dead.  I still like DELAY which Tom thinks is the
spawn of the devil, but feel that if the server reboots, SERVERFAULT is
justified for ops abandoned because of that.

Maybe we need an nfsv4 error with error code 666, even if DELAY is not it.
Could be a subject for an I-D to be submitted 4/1/2021😊

> [email-masked] <[email-masked]>
>
>
>
>

---

From: Rick Macklem <[email-masked]>
Date: Fri, 11 Dec 2020 16:20:09 +0000

>> that after the reboot the file system is in either the pre-operation
[stuff snipped]
>I don't see the need for individual ops to be atomic especially since they (e.g. >writes) are not defined as atomic in general.
Well, I'd actually say that "write" is not a non-idempotent operation for this
case. Long ago, Chet Juszczak defined it as one in his original paper on the
DRC he had invented, because interactions between "write" retries and other
non-idempotent RPCs could result in inconsistent results. (He also coined
the (mis)use of the term in his paper to refer tp RPCs that could result in
inconsistent outcomes due to retries.)

However, since sessions allow a client to serialize RPCs (same session  slot)
and operations within a compound are assumed to be serialized,
I think allowing retries of partially completed writes will be ok for 4.1/4.2.

However, if the operation is an exclusive open create, I think the server
will need to either complete the operation and capture the reply for the
persistent session or roll the file system back to the pre-exclusive create open
state to get things completely correct.

rick

I don't think we have total agreement but we are moving in the same direction.  No reason to be anything but calm.
.

That makes sense.

I have a revised version of this section in draft-dnoveck-rfc5661bis-00.  (Will be out before next year).  I hope we will discuss that and get to a text that everybody does agree on, and might even be implementable :-)

--b.

_______________________________________________
nfsv4 mailing list
[email-masked]<mailto:[email-masked]>
https://www.ietf.org/mailman/listinfo/nfsv4

---

From: "J. Bruce Fields" <[email-masked]>
Date: Fri, 11 Dec 2020 11:57:25 -0500

> failed with no side-effects?

Yes--they can conclude that the one operation failed.  But previous
operations may have succeeded and had side effects.

I had that wrong--I thought the spec said somewhere that DEADSESSION
could only be returned to the initial SEQUENCE op, but actually it's
quite explicit that it can be returned later:

	https://tools.ietf.org/html/rfc5661#section-18.46.4

	In the case of a partially executed COMPOUND, processing may
	reach an operation not processed during the earlier server
	instance, making this operation a new one and not performable on
	the existing session.  In this case, NFS4ERR_DEADSESSION will be
	returned from that operation.

	https://tools.ietf.org/html/rfc5661#section-15.1.11.5

	15.1.11.5.  NFS4ERR_DEADSESSION (Error Code 10078)

	The specified session is a persistent session that is dead and
	does not accept new requests or perform new operations on
	existing requests (in the case in which a request was partially
	executed before server restart).

The table in https://tools.ietf.org/html/rfc5661#section-8.4.2 gives it
as a valid error return for any operation, I think, except GETFH.

So actually, forget DELAY, the server returns DEADSESSION at the point
it stopped processing.  (With the one exception of GETFH as otherwise it
would be impossible to implement stuff like O_CREAT reliably.)

Thus the 2.10.6.5 language was clearly meant to treat each op (and
storage of its encoded result in the persistent DRC) as a transaction,
not to treat the entire compound as a transaction.

--b.

---

From: Trond Myklebust <[email-masked]>
Date: Fri, 11 Dec 2020 17:30:05 +0000

On Fri, 2020-12-11 at 11:57 -0500, J. Bruce Fields wrote:
> > > On Wed, Dec 9, 2020, 5:59 PM J. Bruce Fields
> > > <[email-masked]>
> > > wrote:
> > > > > I would say that the ATOMIC refers to the individual
> > > > > operations
> > > > of the
> > > > > the reply for the first K operations that were completed
> > > > > prior to
> > > > the
> > > > > performed on the file system to complete the reply. (At least
> > > > > for
> > > > the
> > > > > For this to all work correctly, I think the server file
> > > > > system
> > > > would
> > > > > need to perform each non-idempotent operation ATOMICALLY as
> > > > > well,
> > > > such

Sure. That's how I've been reading it as well.

---

From: Trond Myklebust <[email-masked]>
Date: Fri, 11 Dec 2020 12:36:29 -0500

> Deleted snippet about making the execution of a COMPOUND a

The thing in the spec was talking about a COMPOUND, but I believe it is
easier to implement as a per-op transaction. I believe that still
satisfies the language in the rest of the spec.

> > conclude that a NFS4ERR_DEADSESSION error implies that the
> > operation failed with no side-effects?
> 
> No reason at all.  The problem is that requiring the server to
> implement COMPOUNDs as atomic transactions if it implements
> persistent sessions has no real justification and probably cannot be
> implemented.
> 

The fact that it hasn't been implemented yet means that we still the
option of throwing it out of the current spec.

Hammerspace could probably implement this spec as it is defined today,
but it would very likely cause a performance penalty that I don't see
us being willing to take. Is anyone else planning on doing so?

> Also I don't think it is right for the sever to send DEADSESSION
> since the persistent session is not dead.  I still like DELAY which
> Tom thinks is the spawn of the devil, but feel that if the server
> reboots, SERVERFAULT is justified for ops abandoned because of that.
> 
> Maybe we need an nfsv4 error with error code 666, even if DELAY is
> not it.  Could be a subject for an I-D to be submitted 4/1/2021😊
> 

Make it an op:

   OP_SPAWN_DELAY  = 666,

---

From: "J. Bruce Fields" <[email-masked]>
Date: Fri, 11 Dec 2020 13:27:57 -0500

> us being willing to take. Is anyone else planning on doing so?

Now that I've reread it all, I really think the spec is fine, the *only*
part that bugs me is that language in 2.10.6.5:

	MUST be atomic....

	One way to view the problem is as a single transaction
	consisting of each operation in the COMPOUND followed by storing
	the result in persistent storage....

	While the description of the implementation for atomic execution
	of the request and caching of the reply....

>From the rest of the spec I think it's clear this is just clumsily
worded; maybe it would be clearer like:

	The execution of an operation and placement of its results in
	the persistent cache MUST be atomic....

	One way to implement this is as series of transactions, one for
	each operation in a compound, with each transaction consisting
	of executing that operation and storing its result in persistent
	storage....

	While the description of the implementation for atomic execution
	of an operation and caching of its result....

--b.