Re: [alto] Benjamin Kaduk's No Objection on draft-ietf-alto-incr-update-sse-20: (with COMMENT)

Benjamin Kaduk <kaduk@mit.edu> Sat, 04 April 2020 18:17 UTC

Return-Path: <kaduk@mit.edu>
X-Original-To: alto@ietfa.amsl.com
Delivered-To: alto@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 1AA553A00D3; Sat, 4 Apr 2020 11:17:19 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.899
X-Spam-Level:
X-Spam-Status: No, score=-1.899 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, URIBL_BLOCKED=0.001] autolearn=ham autolearn_force=no
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id FkOoXR1q2dUC; Sat, 4 Apr 2020 11:17:13 -0700 (PDT)
Received: from outgoing.mit.edu (outgoing-auth-1.mit.edu [18.9.28.11]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 35DE43A00C1; Sat, 4 Apr 2020 11:17:12 -0700 (PDT)
Received: from kduck.mit.edu ([24.16.140.251]) (authenticated bits=56) (User authenticated as kaduk@ATHENA.MIT.EDU) by outgoing.mit.edu (8.14.7/8.12.4) with ESMTP id 034IH5b8012742 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Sat, 4 Apr 2020 14:17:07 -0400
Date: Sat, 4 Apr 2020 11:17:04 -0700
From: Benjamin Kaduk <kaduk@mit.edu>
To: "Y. Richard Yang" <yry@cs.yale.edu>
Cc: draft-ietf-alto-incr-update-sse@ietf.org, IETF ALTO <alto@ietf.org>, "Vijay K. Gurbani" <vkg@bell-labs.com>
Message-ID: <20200404181704.GP88064@kduck.mit.edu>
References: <158397136560.19639.8260312978650035895@ietfa.amsl.com> <CAAbpuyrL2yuqUCw2mgqKmdv7LWUWZMAC8nggpvbJ974SeVOM4A@mail.gmail.com> <CANUuoLpXN60KMp7_dnrCRqzPUk0857rQa8Q5zadMNnQrB7WgXA@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CANUuoLpXN60KMp7_dnrCRqzPUk0857rQa8Q5zadMNnQrB7WgXA@mail.gmail.com>
User-Agent: Mutt/1.12.1 (2019-06-15)
Archived-At: <https://mailarchive.ietf.org/arch/msg/alto/JesSBh5ZgpgGh7h-pOTWqQW01Ow>
Subject: Re: [alto] Benjamin Kaduk's No Objection on draft-ietf-alto-incr-update-sse-20: (with COMMENT)
X-BeenThere: alto@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: "Application-Layer Traffic Optimization \(alto\) WG mailing list" <alto.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/alto>, <mailto:alto-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/alto/>
List-Post: <mailto:alto@ietf.org>
List-Help: <mailto:alto-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/alto>, <mailto:alto-request@ietf.org?subject=subscribe>
X-List-Received-Date: Sat, 04 Apr 2020 18:17:19 -0000

Hi Richard,

Now it is my turn to apologize for the late reply -- I got completely
swamped with the decision to move to virtual IETF 107 and all the extra
activity as a result.  I see that the -22 is published and approved, and
think that generally all the updates are good and responsive to my
comments, so I only have limited further clarifications to make (inline).

On Sun, Mar 15, 2020 at 10:36:00PM -0400, Y. Richard Yang wrote:
> Dear Ben,
> 
> Thank you so much for the thorough, thoughtful review. Sorry for the late
> reply---I did no
> t get the initial email containing your review. Please see inline.
> 
> On Wed, Mar 11, 2020 at 8:02 PM Benjamin Kaduk via Datatracker <
> noreply@ietf.org> wrote:
> 
> >
> > Benjamin Kaduk has entered the following ballot position for
> > draft-ietf-alto-incr-update-sse-20: No Objection
> >
> > 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-alto-incr-update-sse/
> >
> > ----------------------------------------------------------------------
> > COMMENT:
> > ----------------------------------------------------------------------
> >
> > Thanks for your response to the genart reviewer; I'm happy to see that
> > change staged for the next version.
> >
> >
> Thanks!
> 
> 
> > It's a bit interesting to see that we present JSON merge patch and JSON
> > patch in the reverse order in which they were defined (RFC 7396/7386 vs.
> > 6902).
> >
> >
> It is indeed interesting. The historical reason was that the early versions
> defined
> only JSON merge patch, and then added JSON patch later, motivated by the
> need
> to handle potential inefficiencies (see the end of Sec. 3.1.1 and the
> beginning of
> Sec. 3.2.1). Although in reverse chronological order, the flow and
> transition appear
> to be good so far, and let's keep the flow. OK?

Makes perfect sense!

> 
> 
> > It's very surprising to me that we replicate the algorithm for JSON
> > merge patch and SSE but just refer to RFC 6902 for the JSON patch
> > algorithm.
> >
> 
> Good catch on consistency. As we have removed the algorithm for the merge
> patch,
> they are treated equally now.
> 
> 
> > What happens if either party closes the update stream without the proper
> > clean-up message(s)?
> >
> >
> Very good discussion. I see you may mean two cases: (C1) the overall update
> stream;
> and (C2) one of the substreams. The case of C1 is a special case of C2.
> 
> If the server closes (e.g., due to an internal error) a substream without
> sending
> a control update of a substream, the client may receive a partial update
> message.
> Hence, it is important that the client uses a transaction based
> implementation
> to commit updates.
> 
> If the client closes a substream, there can be two cases: (1) the client
> uses stream
> control, and the server is in the middle of sending a data update for the
> substream; it is
> recommended that the server finishes sending the data update, and the
> client is
> recommended to know that the close is effective only after getting the
> notification
> control update. (2) the client could just close the http connection (and
> send FIN)
> without properly sending the stop as in (1). In this case, the server TCP
> connection
> will be notified, and the server code should handle it.
> 
> Is the above you are looking for?

That's the kind of thing I had in mind, yes.  I'm sure at this point it
feels quite natural to you (and, to be honest, it feels pretty natural to
me as well), but sometimes it can help a reader new to the topic to have
the additional discussion.  That said, I entirely understand if no further
changes are made to the document at this point in the process.

> Section 1
> >
> >    considerations by both ALTO servers and clients; Section 13 discusses
> >    a design feature that is not supported; Section 10 discusses security
> >    issues; The last two sections review the requirements for future ALTO
> >    services to use SSE and IANA considerations, respectively.
> >
> > I think this last remark perhaps should not have survived a section
> > reordering to put the not-supported design feature (section 13) last.
> >
> 
> Although I do not see that we make section-level changes (e.g., add
> sections
> at the end or reordering sections) at this point, self-updating reference
> is
> always a better idea. We will revise the last remark. Thanks for the
> suggestion.
> 
> 
> > Section 2
> >
> >    Stream Control Server: An stream control server providing the stream
> >    control service.
> >
> > Perhaps we could avoid defining a "stream control server" as a "stream
> > control server" that does some things.
> >
> 
> Sounds good. We will remove the redundant structure of the definitions,
> e.g.,
> Update Stream: An update stream is an HTTP connection ...
> ->
> Update Stream: An HTTP connection ...
> 
> We will revise all definitions to be consistent in style.
> 
> 
> >
> >    Control Update Message: A control update message is a message in an
> >    update stream for the update stream server to notify the ALTO client
> >    of related control information of the update stream.  The first
> >    message of an update stream is a control update message and provides
> >    the URI using which the ALTO client can send stream control requests
> >    to the stream control server.  Additional control update messages in
> >    an update stream allow the update stream server to notify the ALTO
> >    client of status changes (e.g., the server will no longer send
> >    updates for an information resource).
> >
> > The way this is written makes me want to double-check that I have the
> > flow/terminology right: there's an update stream of update messages from
> > server to client, that can include data updates but also can include
> > control update messages.
> >
> 
> Right.
> 
> 
> > Control update messages give the client
> > information about the stream control service, potentially distinct from
> > the (regular) update service, and the client sends requests on the
> > stream control service, that are expected to result in changes to what
> > the server sends through the (regular) update service? Would it ever
> >
> happen that a server sends a message on the control service connection, or
> > that a control update message is present on the control service?
> >
> >
> Good summary. Let me try to make some small edit/comments based on your
> sentences and we may use them:
> 
> "Control update messages give the client information about the stream
> control service,
> potentially distinct from the (regular) update service, ..."
> ->
> "Control update messages give the client information about the transmission
> states of data
> updates, and hence are distinct from the (regular) data updates. A control
> update message
> may be triggered by an internal event at the server, such as server
> overloading and hence
> stoping some updates, or as a result of a client sending a request through
> the stream control
> service."
> 
> "... that are expected to result in changes to what the server sends
> through the (regular)
> update service?"
> 
> ->
> 
> Yes, the client should expect to get a notification (control update) on the
> result of its
> request (e.g., start a new substream to update a resource). The timing of
> when the
> notification happens, however, is asynchronous.
> 
> "Would it ever happen that a server sends a message on the control service
> connection, or
> that a control update message is present on the control service?
> 
> ->
> 
> A control update message is always sent at the same update stream
> (connection).
> 
> Section 3.1
> >
> > Is it worth noting an explanation of why the HTTP PATCH method is
> > unsuitable for ALTO SSE (presumably because it goes client-to-server and
> > we require the opposite)?
> >
> >
> Good point and yes, although one can think of the ALTO client as a holder
> of the info
> and hence could be considered as a server. But the info at the client is
> not required
> to be persistent. We will add a sentence to explain a bit.
> 
> 
> > Section 3.1.1
> >
> >    one JSON value into another.  Specifically, JSON merge patch treats
> >    the two JSON values as trees of nested JSON objects (dictionaries of
> >
> > I suggest clarifying "the two JSON values" (e.g., "the old and new JSON
> > values").
> >
> >
> There is some revision to the text to address the comments by Suresh and
> Adam. We
> will add the clarification (i.e., "the old ...) in the revised text---the
> two JSON values are
> still there.
> 
> 
> > Section 3.3
> >
> >    Despite the two features of HTTP/2, this design chooses an HTTP/1.x
> >    based design for the simplicity of HTTP/1.x.  An HTTP/2 based design
> >
> > I suggest to say "HTTP/1.x-compatible design" rather than "HTTP/1.x
> > based design" -- the SSE API is part of HTML5, and as such is usable
> > with both those versions of HTTP (and future versions).
> >
> >
> Good suggestion. We will revise using the suggested phrase.
> 
> 
> > Section 4.1
> >
> >    An update stream can provide updates to both GET-mode resources, such
> >    as ALTO network and cost maps, and POST-mode resources, such as ALTO
> >    endpoint property service.  Also, to avoid creating too many update
> >    streams, this design allows an ALTO client to use one update stream
> >    to receive updates to multiple requests.  In particular, the client
> >    may request to receive updates for the same resource but with
> >    different parameters for a POST-mode resource.  The updates for each
> >
> > This implies by omission that update-stream consolidation is not
> > possible for GET-mode resources or different POST-mode resources; I'd
> > suggest to reword to clarify that this is in addition to being able to
> > consolidate updates for multiple resources into a single stream.
> >
> >
> Interesting catch on potential ambiguity (implication by omission). We will
> reword to
> clarify as suggested: in addition to being able to consolidate updates for
> multiple
> resources into a single stream.
> 
> 
> >    The objective of an update stream is to continuously update the ALTO
> >    client on data value changes given by the client's requests.  This
> >
> > nit: "data value changes given by the client's requests" almost reads
> > like it's the client's requests that are producing the data value
> > changes, rather than the client's requests indicating which data values
> > to report changes to.
> >
> >
> Interesting point. How about the following:
>    The objective of an update stream is to continuously push to an ALTO
> client
>    data value changes to a set of resources, where the set of resources is
> specified
>    by the ALTO client's requests.  This
> 
>   // Note I used plural requests because the client can issue multiple
> stream control
>    requests to determine the set.
> 
>  Make sense?
> 
> Section 4.2
> >
> >    The ALTO client can then use the URI to ask the update stream server
> >    to (1) send data update messages for additional resources, (2) stop
> >
> > nit(?): if the client is talking to the stream control server, is it
> > really asking the *update stream* server to do anything?
> >
> >
> Good, precise comment. How about the following:
> 
>   The ALTO client can then use the URI to ask the stream control server
>   specified in the URI to request the update stream server to (1) send
>   data update messages for additional resources, (2) stop
> 
> 
> 
> > Section 5.3
> >
> >    control-uri:  the URI providing stream control for this update stream
> >         (see Section 7).  The server MUST send a control update message
> >         with an URI as the first event in an update stream.  If the URI
> >
> > It seems like it's worth mentioning whether/when the server can send a
> > control-uri at other times.
> >
> >
> OK. We will add a sentence that this happens only once:
> 
>        control-uri:  the URI providing stream control for this update stream
>         (see Section 7).  The server sends a control update message
>          notifying the client of the control-uri. This control update
> message notifying the
>          control-uri will be sent once and MUST be the first event in an
> update stream.
> 
>    Will this clarify?
> 
> 
> > (I agree with Alexey about describing the 'description' as
> > "human-readable", which it sounds like is going to happen.)
> >
> >
> Already fixed. Thanks for the note.
> 
> 
> > Section 6.3
> >
> >    If this update stream can provide data update messages with
> >    incremental changes for a resource, the "incremental-change-media-
> >    types" field has an entry for that resource-id, and the value is the
> >    media-type of the incremental change.  Normally this will be
> >
> > Should this be "media-type or types", since a comma-separated list is
> > allowed?
> >
> >
> Yes, good catch. Will change to types.
> 
> 
> >    When choosing the media-type to encode incremental changes for a
> >    resource, the update stream server SHOULD consider the limitations of
> >    the encoding.  For example, when a JSON merge patch specifies that
> >    the value of a field is null, its semantics is that the field is
> >    removed from the target, and hence the field is no longer defined
> >    (i.e., undefined); see the MergePatch algorithm in Section 3.1.1 on
> >    how null value is processed.  This, however, may not be the intended
> >    result for the resource, when null and undefined have different
> >    semantics for the resource.  In such a case, the update stream server
> >    SHOULD choose JSON patch over JSON merge patch.
> >
> > I don't see how this is SHOULD (vs. MUST).  Won't things break if you
> > don't?
> >
> >
> Good specification.  SHOULD -> MUST
> 
> 
> > Section 6.5
> >
> >    add: specifies the resources (and the parameters for the resources)
> >         for which the ALTO client wants updates.  We say that the add-
> >         request creates a substream.  The ALTO client MUST assign a
> >         unique substream-id (Section 5.2) for each entry, and uses those
> >         substream-ids as the keys in the "add" field.
> >
> > Unique within what scope?  (Globally unique could prove challenging...)
> >
> >
> Good to clarify. How about the following:
> 
>         The ALTO client MUST assign a substream-id that is unique to the
>         update stream (Section 5.2) for each entry, and uses those
>         substream-ids as the keys in the "add" field.
> 
> 
> >    incremental-changes:  the ALTO client specifies whether it is willing
> >         to receive incremental changes from the update stream server for
> >         this substream.  If the "incremental-changes" field is "true",
> >         the update stream server MAY send incremental changes for this
> >         substream (assuming the update stream server supports
> >         incremental changes for that resource).  In this case, the
> >
> > Since MAY is not an obligation, I think the parenthetical is not
> > strictly speaking needed.
> >
> >
> Good point. The text in the parentheses will be removed.
> 
> 
> >         changes" to "false".  An alternative design of incremental-
> >         changes control is a more fine-grained control, by allowing a
> >         client to select a subset of incremental methods from the set
> >         announced in the server's capabilities.  But this adds
> >         complexity to the server, which is more likely to be the
> >         bottleneck.  Note that the ALTO client cannot suppress full
> >
> > I suggest rewording to be more clear that this alternative design is not
> > part of this specification (and in fact was rejected from
> > consideration?).
> >
> >   Good to clarify. How about the following:
> 
>       An alternative design of incremental-changes control which was
>       considered is a more fine-grained control, by allowing a client to
> select a
>       subset of incremental methods from the set announced in the server's
>       capabilities.  But the alternative design was not adopted in this
>       document because it adds complexity to the server, which is more
> likely to
>       be the bottleneck.
> 
> 
> >         replacement.  When the ALTO client sets "incremental-changes" to
> >         "false", the update stream server MUST send a full replacement
> >         instead of an incremental change to the ALTO client.  The update
> >         stream server MAY wait until more changes are available, and
> >         send a single full replacement with those changes.  Thus an ALTO
> >         client which declines to accept incremental changes may not get
> >         updates as quickly as an ALTO client which does.
> >
> > Do we have any requirement that incremental changes be sent "as quickly
> > as possible" or can they be delayed as well?
> >
> >
> Good discussion.
> 
> The current design has no specification on the delay requirement. It is a
> (more Internet-style, best-effort) design that allows the server to send
> as fast as it can/desire. The client and server may have external agreements
> on how fast to push; an analogy is that different levels of stock quote
> delays
> are often based on business contracts/offerings. QoS is a future topic.
> It neither has higher-level flow control to avoid a faster server overwhelms
> a slow client, although the client can indeed perform certain control by
> not reading from the buffer. The Operations Consideration has some
> discussions
> on potential issues of such delays.

Ah, I must not have remembered that I wrote this by the time I got to the
Operational Considerations.

> 
> 
> > Section 6.7.1
> >
> >    o  The first event MUST be a control update message with the URI of
> >       the update stream control service Section 7 for this update
> >       stream.
> >
> > I thought it was allowed to send a null control-uri in this message.
> >
> 
> Indeed. The first message must be the control-uri. Suppose the IRD
> announces
> "support-stream-control": false, then the control-uri should be null.
> For null control-uri, as an example, it can be
>      event: application/alto-updatestreamcontrol+json
>      data: {"control-uri": null}
> 
>  We will add a couple of sentences to make it clear.
> 
> 
> >
> >    o  As soon as possible after the ALTO client initiates the
> >       connection, the update stream server MUST send a full replacement
> >       for each resource-id requested with a version tag.  In this case
> >       the update stream server MAY omit the initial full replacement for
> >       that resource, if the "tag" field the ALTO client provided for
> >       that resource-id matches the tag of the update stream's current
> >
> > This second sentence contradicts the MUST in the first one; please
> > rephrase to avoid the internal inconsistency.  (Also, please clarify
> > what "this case" is, since we haven't mentioned a specific one by that
> > point in the text.)
> >
> >
> Good comment. Let me rephrase:
> 
>         As soon as possible after the ALTO client initiates the connection,
> the
>         update stream server checks the "tag" field for each added update
> request.
>         If the "tag" field is not specified in an added update request, the
> update stream server
>         MUST first send a full replacement for the request. If the the
> "tag" field
>         is specified, the client can accept incremental changes, and the
> server can
>         compute an incremental update based on the "tag", the update stream
> server
>         MAY omit the initial full replacement.
> 
> Is this clearer?

Yes; thank you!

> 
> 
> >    o  If this update stream provides update for resource-ids and R0 and
> >       R1, and if R1 depends on R0, then the update stream server MUST
> >
> > nit: s/update/updates/
> >
> >
> OK.
> 
> 
> > Section 6.7.2
> >
> >    A update stream server MAY offer several different update stream
> >    resources that provide updates to the same underlying resource (that
> >    is, a resource-id may appear in the "uses" field of more than one
> >    update stream resource).  In this case, those update stream resources
> >    MUST return the same update data.
> >
> > The phrase "update data" is used only once in this document, and in
> > light of the previous discussion about sending the "same updates" but
> > potentially different patch events, it seems that greater specificity is
> > called for in what is intended here.
> >
> 
> This is a wonderful comment! There were substantial discussions early on
> on the subject and let me try to clarify.
> 
> Although we want the design to be as transparent to implementation
> as possible, the spec can have impacts on the implementation.
> 
> Main implementation concern: allow scaling of the push updates to a
> resource,
> by allowing instantiating multiple update streams to the same resource,
> where
> the multiple update streams can be instantiated either from the same update
> stream resource (e.g., a service provider using DNS or VIP load balancing
> to direct clients to multiple update stream servers) or multiple update
> stream
> resources (e.g., the need to use multiple URIs to direct to multiple update
> stream servers).
> 
> These update stream servers are likely frontend to a backend which commits
> a sequence of internal updates, and the internal updates are likely totally
> serialized using a consistent data store update model such as Paxos/RAFT,
> if the backend needs high reliability:
> s0, s1, s2, s3, ... where each vi is a consistent snapshot.
> 
> An early discussion is to allow a weaker consistency model:
> - client 1 may receive updates to obtain a sequence of updates to derive
> snapshots
> s0, s2, s3, ...
> 
> Client 2 may receive updates to obtain a different sequence of snapshots
> s0, s1, s3, ...
> 
> Such subsequence of the global sequence (safety) allows better
> scaling---the liveness
> requirements of all updates are eventually sent is related to the delay
> requirement discussed
> above. This weaker consistency model gives better flexibility but can be
> harder to understand,
> should an application uses it. Hence, the decision is to use a simpler
> consistency model: each
> update stream delivers s0, s1, s2, ...
> 
> If you can OK, we will clarify that according to the simpler model.

I appreciate reading the background you give above; I sympathsize with the
complexity of the options for writing the protocol!  I think it is okay to
put such discussion in the document, but also okay to leave it out (i.e.,
leave the document as-is).  Given that the document is already with the RFC
Editor, leaving the text as-is seems most prudent, to me, but it's your
call.
> 
> > Section 6.7.3
> >
> >    JSON Patch and Merge Patch provide the incremental encoding benefit
> >    but can be applied to only a single JSON object.  If an update stream
> >    service (1) supports a resource providing a multipart media type and
> >    (2) specifies an incremental media type for the resource in its
> >    capabilities, the server MUST (1) use substream-id.content-id in its
> >    `event` field, (2) include the content-id in the multipart message,
> >    and (3) the content identified by the content-id must be a single
> >    JSON object.
> >
> > Which message is "the multipart message"?  Is it on the update stream?
> >
> >
> Good need for clarification. How about the following:
> 
>    JSON patch and merge patch provide the incremental encoding benefit
>    but can be applied to only a single JSON object.  If an update stream
>    service supports a resource providing a multipart media type, which
>    we refer to as a multipart resource, then the update
>    stream service needs to handle the issue that the message of a full
> multipart
>    resource can include multiple JSON objects. To address the issue, when
> an
>    update stream service specifies that it supports JSON patch or merge
> patch incremental
>    updates for a multipart resource,  the service MUST
>    ensure that (1) each part of a multipart message is a single JSON
> object,
>    (2) each part is specified by a static content-id in the initial full
> message, (3) each
>    data update event applies to only one part; and (4) each data update
> specifies
>    substream-id.content-id as the `event` field of the event, to identify
> the part
>    to be updated.
> 
> Does the preceding clarify?

Very nicely!

> 
> > Section 7
> >
> >    An stream control service allows an ALTO client to remove resources
> >
> > nit: s/An/A/
> >
> >
> Thanks. Will fix.
> 
> 
> > Section 7.1
> >
> > I agree with Alexey to reference RFC 3986 here.
> >
> >    It is expected that the update stream server will assign a unique
> >    stream id to each update stream instance and will embed that id in
> >    the associated stream control URI.  However, the exact mechanism is
> >    left to the update stream server.  ALTO clients MUST NOT attempt to
> >    deduce a stream id from the control URI.
> >
> > Is this stream ID used for anything outside of the update stream
> > server's internal processing?
> >
> >
> It should not. How about the following revision:
> 
>    It is expected that there is an internal mechanism to map a stream
> control
>    URI to the unique update stream instance to be controlled. For example,
>    the update stream service may assign a unique, internal stream id to
>    each update stream instance. However, the exact
>    mechanism is left to the update stream service provider.
> 
> 
> 
> >    To prevent an attacker from forging a stream control URI and sending
> >    bogus requests to disrupt other update streams, stream control URIs
> >    SHOULD contain sufficient random redundancy to make it difficult to
> >    guess valid URIs.
> >
> > I think this needs to be a MUST.
> > It is probably also worth referencing
> > https://www.w3.org/TR/capability-urls/ .
> >
> >
> Good suggestion. We will add the reference and revise.
> 
> 
> > Section 7.5
> >
> >    An stream control service accepts the same input media type and input
> >
> > nit: s/An/A/
> >
> > Thanks, will fix.
> 
> 
> > Section 7.6
> >
> >    o  If any substream-id in the "remove" field was not added in a prior
> >       request, the error code of the error message MUST be
> >       E_INVALID_FIELD_VALUE; the "field" field SHOULD be "remove" and
> >       the "value" field SHOULD be the array of the invalid substream-
> >
> > nit: s/the array/an array/; we are defining this array for the first
> > time.
> >
> >
> Thanks, will change.
> 
> 
> >       id in the same request.  However, it is legal to remove a
> >       substream-id twice.
> >
> > I suggest making this requirement to track
> > previously-used-but-now-closed substream-ids more explicit.  (It also
> > comes uo for the next bullet point.)
> >
> >
> OK. Will add the requirement, which is reasonable.
> 
> 
> 
> >    o  If any substream-id in the "add" field has been used before in
> >       this stream, the error code of the error message MUST be
> >       E_INVALID_FIELD_VALUE, the "field" field SHOULD be "add" and the
> >       "value" field SHOULD be the array of invalid substream-ids.
> >
> > nit: s/the array of/an array of the/ (same reasoning as before)
> >
> >
> OK. See above.
> 
> 
> > Section 8
> >
> > [Just noting that I, as well, did not perform a detailed check of the
> > examples.]
> >
> >
> Sure we will double-check ourselves.
> 
> 
> > Section 8.4
> >
> >    If the ALTO client needs the "bandwidth" property for additional
> >    endpoints, the ALTO client can send an "add" request on the stream
> >    control URI:
> >
> > The "add" request also includes a priv:ietf-load request that we don't
> > mention here.
> >
> >
> OK. We will mention.
> 
> 
> > Section 9.3
> >
> >    An update stream server can avoid such issues by offering update
> >    streams only for filtered cost maps which do not allow constraint
> >    tests.
> >
> > Maybe say something about "having to handle such complicated behavior"
> > instead of "such issues" (which are not particularly clearly indicated)?
> >
> >
> Good suggestion.
>    "An update stream server can avoid having to handle such complicated
> behavior
>    by offering update streams only for filtered cost maps which do not
> allow
>    constraint tests."
> 
> 
> > Section 10
> >
> > I greatly appreciate the way these security considerations are framed,
> > fiting into the context of the base protocol and then with the
> > additional risks discussed separately; thank you!
> >
> >
> > Thanks!
> 
> 
> > Is there anything interesting to say about this mechanism making it
> > easier to use an updated resource with a stale version of a resource
> > used by that first resource?
> >
> >
> Not sure which mechanism. Could you please give a bit more context?

This was something of a meta-comment about the potential for dependencies
across resources.  For example, if I only subscribe to push notifications
for the cost map but not the network map, I might have a long window where
I use cost data with the incorrect network information.  This is not really
a new risk with server-push, and is largely just a matter of clients not
shooting their own foot, so I don't know that there is a specific point to
make in this document.

> 
> > With the possibility for the update server and stream control server to
> > be different entities, there is a risk of information skew or
> > communications breakdown between them, as well as invalid or spoofed
> > messages claiming to be on that private communications path.  It would
> > be worth a brief discussion of what sort of problems can arise from that
> > separation of roles.
> >
> >
> Good addition. We will add.
> 
> 
> > Just to check: SSE guarantees that we get events in order, so there are
> > no reordering considerations within a stream.  IIRC we did have some
> > discussion about synchronization of the same updates appearing on
> > multiple streams and of dependent updates within a single stream, which
> > should take care of the main points in this space.
> >
> >
> Good comments. See above on the cross-stream consistency model.
> 
> 
> > I was also not able to read very closely the SSE spec, but it sounded
> > like perhaps the client-side implementation was allowed (or required?)
> > to introduce linefeed characters into the data buffer for the event in
> > question, e.g., to reflect the line breaks in the actual transferred
> > encoding.  Do we need to say anything about the sender behavior to avoid
> > putting line breaks in places that would have semantic consequences for
> > the ALTO updates?
> >
> >
> Specifying the server behavior can help to avoid bugs. There is some
> discussion on the line length (Sec. 9.5) and figure events (Sec. 11). This
> discussion on server behavior can be added
> (and will be added) in Section 9.5. What do you think?
> 
> 
> > Section 10.1
> >
> >    To avoid these attacks on the update stream server, the server SHOULD
> >    choose to limit the number of active streams and reject new requests
> >    when that threshold is reached.  An update stream server SHOULD also
> >
> > This transfers the attack from the update server to the honst clients
> > trying to use it (as noted in the following paragraph); it may be
> > possible to use somewhat more clever logic involving IP reputation,
> > rate-limiting, and compartmentalizing the overall threshold into smaller
> > thresholds that apply to subsets of potential clients.  (Client
> > authentication is also a potential mitigation for some of these attacks,
> > as is mentioned in the following paragraph.)  None of this is
> > necessarily incompatible with the current "SHOULD choose to limit"
> > guideline, of course, though we may want to give a bit more sense of the
> > nuances involved.
> >
> >
> Good text, and we will adopt some of them right after the first paragraph.
> 
> 
> > Section 10.2
> >
> >    Also, under overloading, the client may no longer be able to detect
> >    whether an information is still fresh or has become stale.  In such a
> >    case, the client should be careful in how it uses the information to
> >    avoid stability or efficiency issues.
> >
> > Is it possible to get into a state where stale ALTO data causes a client
> > to behave in a way worse for the network than not using any ALTO data at
> > all?
> >
> >
> Yes. This is possible, and this a risk that ALTO clients should always pay
> attention
> to. We plan to move some text on this from the recent cost calendar to this
> as well.
> 
> 
> 
> > Section 10.3
> >
> > I thought https was a MUST for ALTO, which would provide the needed
> > confidentiality even in the absence of mutual authentication.
> >
> >
> There still can be cases where http may be OK, when neither confidentiality
> nor
> authentication is an issue.
> 
> 
> > Section 11
> >
> >    At the low level encoding level, new line in SSE has its own
> >    semantics.  Hence, this design requires that resource encoding does
> >    not include new lines that can confuse with SSE encoding.  In
> >    particular, the data update message MUST NOT include "event: " or
> >    "data: " at a new line as part of data message.
> >
> > It may also be worth forbidding these not at a new line, since a modular
> > server implementation might apply an arbitrary line-breaking policy.
> >
> >
> Yep. Will add, as above mentioned.
> 
> 
> > Section 13
> >
> > I suggest adding a little more introduction at the start to give the
> > background that the following description is a protocol design and
> > considerations that could have been used, but was rejected.
> >
> >
> Good suggestion. We are moving it to the appendix in the newer version.
> 
> Thank you so much for the extremely careful, insightful review!

And thank you for the thoughtful updates!

My apologies again for the delayed response, but I think the document is in
good shape regardless.

-Ben