Subject: [core] AD review of draft-ietf-core-observe-14

From: Barry Leiba <[email-masked]>
Date: Mon, 21 Jul 2014 15:58:54 -0400

I've gotten my review of the Observe document done, despite IETF week.
My comments are below.  They're all minor, and there's no need to hold
up last call for them, but I'm not going to request last call until
Friday, just for timing purposes.  If you want to post a revised
version that addresses my comments, please do that before then.
Otherwise, you can address my comments later.

And, of course, feel free to argue with me about anything I say below.

Barry

--------------------------------------------------------------
-- Section 1.2 --

   Notification:  Whenever the state of a resource changes, the server
      notifies each client in the list of observers of the resource.
      Each notification is an additional CoAP response sent by the
      server in reply to the GET request and includes a complete,
      updated representation of the new resource state.

I think it'd be a useful clarification to say this:

NEW
   Notification:  Whenever the state of a resource changes, the server
      notifies each client in the list of observers of the resource.
      Each notification is an additional CoAP response sent by the
      server in reply to the single extended GET request that was
      made at registration, and includes a complete, updated
      representation of the new resource state.
END

-- Section 2 --
Is it the case that when the server is unwilling ot unable to add the
request to the observer list, and it falls back to a normal GET
request, the response does not have the Observe Option?  Assuming so,
I think it could be made clearer this way:

OLD
   The Observe Option is not critical for processing the request.  If
   the server is unwilling or unable to add a new entry to the list of
   observers, then the request falls back to a normal GET request.

   In a response, the Observe Option identifies the message as a
   notification.  This implies that the server has added an entry with
   the client endpoint and request token to the list of observers and
   that it will notify the client of changes to the resource state.  The
   option value is a 24-bit sequence number for reordering detection
   (see Section 3.4 and Section 4.4).
NEW
   In a response, the Observe Option identifies the message as a
   notification.  This implies that the server has added an entry with
   the client endpoint and request token to the list of observers and
   that it will notify the client of changes to the resource state.  The
   option value is a 24-bit sequence number for reordering detection
   (see Section 3.4 and Section 4.4).

   The Observe Option is not critical for processing the request.  If
   the server is unwilling or unable to add a new entry to the list of
   observers, then the request falls back to a normal GET request, and
   the response does not include the Observe Option.
END

-- Section 3.1 --

   The target resource SHALL be identified
   for this purpose by all options in the request that are part of the
   cache-key, such as the full request URI and the Accept Option.

I think the SHALL is unncessary here, and that the sentence is a bit
confusingly worded.  This addresses that:

NEW
   For this purpose, all requests in which all options that are part
   of the cache-key match are considered requests for the same
   target resource.  Options that are part of the cache-key include,
   for example, the full request URI and the Accept Option.
END

I would also make that a separate paragraph.

-- Section 3.2 --
As above (comment to Section 1.2), I would make this change:

OLD
   Notifications are additional responses sent by the server in reply to
   the GET request.
NEW
   Notifications are additional responses sent by the server in reply to
   the GET request that created the registration.
END

-- Section 3.3.1 --

   Additionally, the client SHOULD at least wait
   for a random amount of time between 5 and 15 seconds after Max-Age
   expired to avoid synchronicity with other clients.

I think "synchronicity" is not the right word here: it has a
particular connotation that you don't intend.  I suggest "to reduce
collisions with other clients," or some such.

-- Section 3.4 --

   Messages with notifications can arrive in a different order than they
   were sent.  Since the goal is to keep the observed state as closely
   in sync with the actual state as possible, a client MUST NOT consider
   a notification fresh that arrives later than a newer notification.

That's a bit convoluted.  I suggest wording it positively, rather than
negatively, like this:

NEW
   Messages with notifications can arrive in a different order than they
   were sent.  Since the goal is to keep the observed state as closely
   in sync with the actual state as possible, a client MUST consider the
   notification that was sent most recently as the freshest, regardless
   of the order of arrival.
END

-- Section 3.5 --
Just a minor English issue:

OLD
   A notification can be confirmable or non-confirmable, i.e., it can be
   sent in a confirmable or a non-confirmable message.  The message type
   used for a notification is independent from the type used for the
   request or for any previous notification.
NEW
   A notification can be confirmable or non-confirmable, i.e., it can be
   sent in a confirmable or a non-confirmable message.  The message type
   used for a notification is independent of the type used for the
   request and of any previous notification.
END

-- Section 4.1 --

   Upon success, the server returns a current representation
   of the resource and MUST notify the client of subsequent changes to
   the state as long as the client is on the list of observers.

But we've already said it's OK for the server not to send *every*
state change, so the MUST isn't quite right.  I actually think you
don't need MUST here, and that this will explain the situation just
fine, allowing for what was said earlier:

   Upon success, the server returns a current representation
   of the resource, and notifies the client of subsequent changes to
   the state as long as the client is on the list of observers.

-- Section 4.5 --
The "MAY" in the first paragraph doesn't need to be a 2119 key word.

-- Section 6 --

   The "obs" attribute MUST NOT appear more
   than once in a given link-value; occurrences after the first MUST be
   ignored by parsers.

I see where you're going with this, but as "obs" has no value, if
there are multiple occurrences they're all the same.  In this case,
I'd say it this way (putting the second part in active voice while I'm
at it):

NEW
   The "obs" attribute MUST NOT appear more
   than once in a given link-value, and parsers MUST ignore any extra
   occurrences.
END

-- Section 7 --

   The security considerations in Section 11 of RFC 7252 [RFC7252]
   apply.

I think it would be helpful to be more up front about what that is,
rather than saying "7252" twice:

NEW
   The security considerations in Section 11 of the CoAP specification
   [RFC7252] apply.
END

   Servers may want to access-
   control this creation of state.

Please don't make a verb out of "access control":

NEW
   Servers may want to apply access controls to
   this creation of state.
END

-- Section 9 --

We have at our disposal the option of a "Contributors" section, which
would go ahead of the "Acknowledgments" section, and which would be
used for contributors who had a particularly significant contribution.
We normally use that for additional or original authors, who are no
longer listed as current authors.

It sounds like Carsten might be listed in such a section, rather than
in this one.  I'll leave it entirely up to you, but I wanted to make
sure you were aware of the option.

-- References --

I think RFCs 1982 and 5405 are informative references, not normative
ones.  Please think about it again: is it necessary to understand
those RFCs in order to do what this document is telling you to?

If, after thinking about it again, you disagree with me, then leave
one or both of them as they are.
--------------------------------------------------------------

---

From: Carsten Bormann <[email-masked]>
Date: Mon, 21 Jul 2014 22:45:03 -0400

Good suggestions.

I have a bit of a reservation on this one:

>   the state as long as the client is on the list of observers.

I think this is trying to make the point that, while a specific intermediate state can be lost, there is a promise of eventual consistency.
So a server cannot e.g. update the state, even keep the resource stable in the new state, and never update its observers.
It MUST notify at some point.

I trust Klaus can find a better way to phrase this without losing this MUST when he returns from his vacation at the end of next week.

Grüße, Carsten