Subject: [core] Concurrent modifications suggestion in draft-vanderstok-core-patch-01

From: Christian Amsüss <[email-masked]>
Date: Tue, 8 Sep 2015 16:59:30 +0200

Hello Peter,

after reading through draft-vanderstok-core-patch-01, I'm curious about
the "Concurrent modification" (more than one incoming patch at the same
time) error handling recommendation: Why 4.09 (Conflict)?

As I'd currently implement a client, this would cause re-fetching and
three-way merging happiness and/or user interaction, while the 5.03 I'd
rather expect in such a situation ("Hold your horses, I can't apply two
patches at once") would just have the client retry again, and, unless
there is a real 4.09 Conflict, send a successful response.

Hoping this is not considered premature nitpicking
Christian

-- 
Christian Amsüss                      | Energy Harvesting Solutions GmbH
founder, system architect             | headquarter:
mailto:[email-masked]  | Arbeitergasse 15, A-4400 Steyr
tel:+43-664-97-90-6-39                | http://www.energyharvesting.at/
                                      | ATU68476614

---

From: peter van der Stok <[email-masked]>
Date: Wed, 09 Sep 2015 09:27:28 +0200

Hi Christian,

thanks for the remark. We were rather hoping that reactions on the error 
codes, like yours, would be sent in.
409 is a http code, but 4.09 is not cited in RFC7252, and is new to 
CoAP.

The text for 409 in http/1.1 status code registry is below, for a 
reminder:

----------
The request could not be completed due to a conflict with the current 
state of the resource. This code is only allowed in situations where it 
is expected that the user might be able to resolve the conflict and 
resubmit the request. The response body SHOULD include enough

information for the user to recognize the source of the conflict. 
Ideally, the response entity would include enough information for the 
user or user agent to fix the problem; however, that might not be 
possible and is not required.

Conflicts are most likely to occur in response to a PUT request. For 
example, if versioning were being used and the entity being PUT included 
changes to a resource which conflict with those made by an earlier 
(third-party) request, the server might use the 409 response to indicate 
that it can't complete the request. In this case, the response entity 
would likely contain a list of the differences between the two versions 
in a format defined by the response Content-Type.

-----------
It looks rather appropriate.

503 is used in "temporary" overload situations independent of server 
state, but (contrary to 4.09) is defined in 7252.
I assume you have no other clients which implemented code for 4.09. So 
you are free to code it as appropriate.
Do you suggest that additional information should be included in the 
4.09 return code?

Looking forward to hear additional details.

Peter

PS May be you could explain what
"re-fetching and three-way merging happiness and/or user interaction" 
means.
For the moment I am guessing.

Peter

---

From: Christian Amsüßs <[email-masked]>
Date: Wed, 9 Sep 2015 12:11:33 +0200

Hi Peter,

The point I'd fokus on in the 4(.)09 description is

> *state of the resource*.

. As PATCH is supposed to be atomic, by the time the conflicting second
request arrives, the resource's state can be the original or the altered
state; let's for the sake of the argument assume that the patches would
apply cleanly in either sequence.

The "Concurrent modification" situation happens because the requests
"cannot be queued", which in my opinion matches well with RFC 2616's
description of "temporary overloading".

> 503 is used in "temporary" overload situations independent of server state,

I assume you mean resource state here -- as far as I understand, 503 is
precisely about the server's (the process's) state.

> This code is only allowed in situations where it is
> expected that the user might be able to resolve the conflict and resubmit
> the request. The response body SHOULD include enough

This is why I mentioned "re-fetching an three-way merging...", so I'll
explain:

If a client sees 4.09, they would be required to resolve the conflict.
This would normally mean fetching the resource again, and constructing a
new resource state that contains both the changes done on the server
since the client last fetched it and the client's own changes. (Whether
this is implemented as a three-way merge[1] or else is an implementation
detail, but in any case has potential of being messy). The client may
even need to call for user interaction because it can't do any merging
by itself, or (as I'd expect of many tiny embedded implementations)
indicate failure.

That is a comparatively complex process that might, as in the example
above with the two PATCH requests applying cleanly in either order, be
completely uncalled for, and the more sane behavior would be for the
client to try sending its request again after some seconds -- *then*
both will see whether it can be applied. For things to happen that way,
the server should not send 4.09, but 5.03.

I hope that clarifies what I've tried to put into a short mail before
Christian

[1] https://en.wikipedia.org/wiki/Merge_(version_control)#Three-way_merge

---

From: peter van der Stok <[email-masked]>
Date: Thu, 10 Sep 2015 12:04:14 +0200

Hi Christian,

Many thanks for the elaborate answer; This helps a lot and makes me 
realize that some additional text may be useful in the draft.

Let me rephrase in my own concepts that I understand.

The following things are possible:

1) The request arrives at the server, and the request queue is full. 
(this also handles the case that the server can handle only one request 
at the time and is busy)

2) The request is entered in the server queue and multiple concurrent 
processes handle as many requests. N processes concurrently overwrite 
the same subset of values leading to inconsistent resource state

3) The request is entered in the server queue; requests are handled 
sequentially (atomically) and an earlier request is handled after a 
later request (whatever way this is detected).

Add 1) error return 5.03 is clearly required
Add 2) error return 4.09 seems required; When patch is atomic, an 
earlier consistent resource state should be recovered by the server.
Add 2.1) when the server recovers to a consistent state, the client is 
not required to do anything and 4.09 is not appropriate; Create a new 
error?
Add 3) This is the point of discussion? You prefer error 5.03? what 
about a new error to cover the situation?

Or is there a 4th possibility that I am not aware of?
I understand "cannot be queued" as case 1. Is that correct?

Sorry to insist so much on clearly understanding; but I think it is 
important to get it right. These things are sources of unreproducible 
errors.

Peter

Christian Amsüßs schreef op 2015-09-09 12:11:
>> state,
> [1] 
> https://en.wikipedia.org/wiki/Merge_(version_control)#Three-way_merge

---

From: Christian Amsüss <[email-masked]>
Date: Thu, 10 Sep 2015 13:07:29 +0200

Hello Peter,

On Thu, Sep 10, 2015 at 12:04:14PM +0200, peter van der Stok wrote:
> important to get it right.

I'm all with you when it comes to taking this seriously.

> [...]

Yes, and this is how I read the "Concurrent modification" paragraph in
section 3 originally.

> processes handle as many requests. N processes concurrently overwrite the
> same subset of values leading to inconsistent resource state
>
> Add 2) error return 4.09 seems required; When patch is atomic, an earlier
> consistent resource state should be recovered by the server.
> Add 2.1) when the server recovers to a consistent state, the client is not
> required to do anything and 4.09 is not appropriate; Create a new error?

Currently, atomicity of a PACH operation is a MUST in the draft, and I
don't see any reason why it shouldn't. Unless the server has other means
of clean handling, it would protect the resource against such a thing
(eg. using locks and queuing the request). A patch either applies in
full (2.04) or not (4.09).

To illustrate what I mean by "other means of clean handling", assume a
resource represents a line-based text, and (by design of the patch
content type) a patch can only update a single line; further assume that
the patches are transferred in multi-block CoAP transfers to make
concurrency really an issue.

Then a server could accept different first blocks affecting different
lines, knowing that they can be handled separately, and 2.04 them at
their respective last blocks. A first block attempting to modify a line
that is currently being patched (ie. transfer in progress / not timed
out) would be rejected early with 5.03 unless the request can be queued
as in 1).

> sequentially (atomically) and an earlier request is handled after a later
> request (whatever way this is detected).
> 
> Add 3) This is the point of discussion? You prefer error 5.03? what about a
> new error to cover the situation?

I haven't thought of this point yet, as I wouldn't expect a client to
send a consecutive patch atop one it has not received a successful
response yet. IMO it is the client's responsibility to ensure that
patches are delivered in sequence unless the patch format takes care of
that anyway (but in that case, probably neither of the two
patches would apply if received out of sequence).

(I could envision an example of when something like that *might* make
sense, where a GIT history is successively transferred, and the server
will belay its answer to a not-yet-applicable commit until it has
received the earlier objects, but that's pretty far-fetched and not how
it'd probably be done.)

> Or is there a 4th possibility that I am not aware of?

If I were to enumerate all the special cases, I would only have
mentioned 1), based on my understanding of atomic patches and general
network practice.

Cases 2) and 3) to me sound rather like server and client trying to be
smart by being sloppy about atomicity or disregarding the fact that
network traffic in general is not ordered, respectively. It is good to
consider those cases in the RFC (as they *will* occur in practice), but
I'd rather have strict phrasing in the normative part (MUST be processed
atomically, MUST NOT rely on the sequence in which they are processed)
and mention cases like 2 and 3 in a (possibly non-normative) examples
part, outlining how participants could be smart and still stick to the
requirements, and under which preconditions that works.

Best regards
Christian

---

From: peter van der Stok <[email-masked]>
Date: Fri, 11 Sep 2015 09:28:35 +0200

Hi Christian,

I think to understand the situation. Below some proposed changes to the 
draft
_____________________________________________________________________
Section 3
A PATCH request may fail under certain known conditions.  These
    situations should be dealt with as expressed below.
Add: When an error has occurred, and the resulting resource state is 
inconsistent, the server MUST return to an earlier consistent resource 
state.

Conflicting state:  If the modification specified by a PATCH request 
causes
       the resource to enter an inconsistent state that the server cannot 
resolve, the server can return
       the 4.09 (Conflict) CoAP response.  The server SHOULD return the 
actual resource state to provide the client with
       the means to create a new consistent resource state. Such a 
situation might be
       encountered when a structural modification is applied to a
       configuration data-store, but the structures being modified do not
       exist.

Conflicting modification: error nr: 4.nr2 => 4.12 "precondition failed"

Concurrent modification: error nr 4.09 => 5.03 "Service unavailable"

Add:
Conflict handling failure: If the modification implies the reservation 
of resources or the waiting on conditions to become true,
      leading to a too long request execution time, the server can return 
5.03 "service unavailable".

_____________________________________________________________________________________

Is this satisfactory to you?

Peter

Christian Amsüss schreef op 2015-09-10 13:07:
>> the
>> earlier
>> not

---

From: Christian Amsüss <[email-masked]>
Date: Mon, 14 Sep 2015 12:24:54 +0200

> inconsistent, the server MUST return to an earlier consistent resource

Such a wording, in my opinion, might do more harm than good. The draft
already says "atomic", that seems comparatively unambiguous to me
provided there are no other statements that weaken thet (eg. this one
might be read as allowing the server to return to an earlier consistent
state than the last one).

>       causes the resource to enter an inconsistent state that the
>       server cannot resolve, the server can return the 4.09 (Conflict)
>       CoAP response.  [...] Such a situation might be encountered
>       when a structural modification is applied to a configuration
>       data-store, but the structures being modified do not exist.
> 

This does sound good to me. I've had a look at RFC5789 (HTTP PATCH)
again, where that structure probably originated, and found that 4(.)12
is tied to having a If-Match present, that would be a good idea here
too.

>       The server SHOULD return the actual resource state to provide
>       the client with the means to create a new consistent resource
>       state.

I think that a diagnostic message (or none) is better suited here.
Having diagnostics is a SHOULD of RFC7252, the HTTP PATCH doesn't
auto-respond with the latest version either. I'm not saying it can't be
done, but it'd need to be done carefully. So far, afaict this would be
the first non-successful response code that would transmit a (possibly
blockwise'd) content payload.

The passage in RFC5789 about concurrent modification made me look up its
history w/rt 5(.)03 responses. In [1], the very issue I've brought up on
it was discussed in its HTTP equivalent before. They stuck with 409, in
my impression didn't drill down deep enough into the "concurrent
modification" case. I'm tempted to ask Lisa Dusseault (the author of
RFC5789 and [1]) to comment on this thread around [2]; she has
definitely given this topic some thought -- have you been in contact
with her during this draft?

Best regards
Christian

[1] http://www.ietf.org/mail-archive/web/ietf/current/msg59171.html
[2] http://mailarchive.ietf.org/arch/msg/core/JI_TqZQFuMfdTs-MnQuKrrNgCPs

---

From: peter van der Stok <[email-masked]>
Date: Tue, 15 Sep 2015 12:40:11 +0200

Hi Christian,

see below.

Christian Amsüss schreef op 2015-09-14 12:24:

> Such a wording, in my opinion, might do more harm than good.

Probably, you are right. It is removed.

For failed precondition the following text is suggested
    Failed precondition:  In case the client uses the conditional If-
       Match or If-None-Match option to define a precondition for the
       PATCH request, and that precondition fails, then the server can
       return the 4.12 (Precondition Failed) CoAP error.

> blockwise'd) content payload.

We will probably revert to the http text, and add a MAY for returning 
the resource state.

> 
> I'm tempted to ask Lisa Dusseault (the author of

Please, go ahead. We did not have contact with her on the topic.

> [2] 
> http://mailarchive.ietf.org/arch/msg/core/JI_TqZQFuMfdTs-MnQuKrrNgCPs

Greetings,

peter

---

From: Christian Amsüss <[email-masked]>
Date: Tue, 22 Sep 2015 20:57:28 +0200

Hello Lisa,

there is an current discussion on the CoRE working group mailing list
that in my opinion would benefit on your feedback:

The draft for a PATCH method in CoAP[1] gives recommendations on how to
respond to particular error situations, taking many ideas from HTTP's
PATCH. I've brought up concerns about servers returning 4.09 (CoAPese
for 409 Conflict) when concurrent modification requests can not be
handled by the server. We've dissected the scenarios in the thread
around [2] and wound up with a rough understanding of when to err with
4.09 and when with 5.03 Service Unavailable.

A bit later, it turned out that same question has been brought up when
you defined HTTP's PATCH in [3], where you argued that indicating a
Conflict is the right thing to do even in a concurrent modification
scenario.

You've obviously spent some thought on the semantics of a PATCH request;
could you have a look at the relevant discussion points and give your
ideas on whether a Service Unavailable response is appropriate?

(For your convenience, the IMO relevant passages are quoted below, see
the linked threads for more context).

Best regards
Christian

RFC 5789 (PATCH Method for HTTP):
> Concurrent modification:  Some applications of PATCH might require
>    the server to process requests in the order in which they are
>    received.  If a server is operating under those restrictions, and
>    it receives concurrent requests to modify the same resource, but
>    is unable to queue those requests, the server can usefully
>    indicate this error by using a 409 (Conflict) response.

Christian Amsüss wrote:
> This would normally mean fetching the resource again, [...] may even
> need to call for user interaction because it can't do any merging by
> itself, or (as I'd expect of many tiny embedded implementations)
> indicate failure.

> Add 1) error return 5.03 is clearly required

[1] https://tools.ietf.org/html/draft-vanderstok-core-patch-01
[2] http://mailarchive.ietf.org/arch/msg/core/JI_TqZQFuMfdTs-MnQuKrrNgCPs
[3] http://www.ietf.org/mail-archive/web/ietf/current/msg59171.html