Subject: [core] Adam Roach's Discuss on draft-ietf-core-senml-etch-05: (with DISCUSS)

From: Adam Roach via Datatracker <[email-masked]>
Date: Wed, 04 Sep 2019 00:03:20 -0700

Adam Roach has entered the following ballot position for
draft-ietf-core-senml-etch-05: Discuss

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)

Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.

The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-core-senml-etch/

----------------------------------------------------------------------
DISCUSS:
----------------------------------------------------------------------

Thanks to everyone who worked on this document. I have a couple of
concerns that I think make this specification ambiguous in some very
important ways that will prevent interoperation. These should be
pretty simple to fix.

Section 3.2 talks about patching operations. One of the things it
indicates is:

  "The
   names and times of the Patch Records are given and matched in same
   way as for the Fetch Records, except each Patch Record can match at
   most one Target Record."

I kept waiting for text that tells the recipient of a patch what to do
if a Patch Record matches more than one SenML Record (e.g., if the
Patch Record contains no time value and so matches several different
SenML Records), but I couldn't find any. Presumably, this is an error,
and an error should be sent to the client.

The ability for one Patch Record to raise an error, in turn, raises the
question about how to handle other Patch Records in the same Patch Pack. For
example, if I send a Patch Pack with two Patch Records that can be
successfully applied, and one that cannot (because it matches multiple
SenML Records), does the server apply the two that it can, but not the
one that cannot? If so, how does it communicate to the client which
records succeeded, and which failed?

Relevant to the above points, we also need to clearly specify in this document
whether the Patch Records are considered to be applied in sequence. For example,
consider a SenML database containing:

   [
     {"n":"urn:dev:ow:10e2073a01080063","u":"%RH","t":1.320067464e+09,
      "v":20},
     {"n":"urn:dev:ow:10e2073a01080063","u":"%RH","t":1.320067524e+09,
      "v":20.3}
   ]

Now, I send a Patch Pack like:

   [
     {"n":"urn:dev:ow:10e2073a01080063","u":"%RH","t":1.320067464e+09,
      "v":null},
     {"n":"urn:dev:ow:10e2073a01080063","u":"%RH", "v":21}
   ]

Does this succeed, or does it fail? If executed in sequence, this
succeeds, since the first Patch Record will remove the first SenML
record, and the second Patch Record will unambiguously match the
second SenML record (and update it). On the other hand, if we don't
specify that records are processed in order, then this might be
rejected due to the second Patch Record being ambiguous.

Note that this rejection could also arise if an implementation
naively attempted to validate records prior to executing them,
without taking into account the impact of the preceding record
on the state of the SenML record database.

There are other ways of handling this, but my strawman for how
to address these issues would be to add language that specifies:

- Any Patch Record that matches more than one SenML record results
  in an error that is sent to the client.

- If an error is sent to a client for a Patch Pack, then the final
  state of the SenML records on the client must not be changed.
  In other words, either the Patch Pack works in its entirety, or
  it fails in its entirety.

- Patch Records in a Patch Pack are processed sequentially. In order
  to implement both this and the preceding bullet, the implementation
  will need some ability to roll-back any already-applied changes if
  an error is encountered.