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

"Y. Richard Yang" <yry@cs.yale.edu> Sat, 21 March 2020 03:18 UTC

Return-Path: <yang.r.yang@gmail.com>
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 1CFBB3A1100; Fri, 20 Mar 2020 20:18:41 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -3.11
X-Spam-Level:
X-Spam-Status: No, score=-3.11 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, FREEMAIL_FORGED_FROMDOMAIN=0.001, FREEMAIL_FROM=0.001, HEADER_FROM_DIFFERENT_DOMAINS=0.249, HTML_MESSAGE=0.001, RCVD_IN_MSPIKE_H2=-1.463, 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 ns43p9PPZbQF; Fri, 20 Mar 2020 20:18:33 -0700 (PDT)
Received: from mail-ua1-f44.google.com (mail-ua1-f44.google.com [209.85.222.44]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 2902C3A10FC; Fri, 20 Mar 2020 20:18:32 -0700 (PDT)
Received: by mail-ua1-f44.google.com with SMTP id j2so3012815uak.9; Fri, 20 Mar 2020 20:18:32 -0700 (PDT)
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to; bh=UoHAldLR1d5oxuvdywpVkR2vnJ7MJnKIGQvKwhMNiN4=; b=r78qTYwxqmuVDXp+DDoUiQVpwN5y5DtPHtAYsw1Tzdhe6F/tsLSYR15DGybCsE1nFz bADmkIGvWB1J6bKcUBskAubiI4C6nyPsi7UwCGaI+E1FP3bYdO9/WsMS3iJapAj7Dbwi dWrch2cnnBMBZgq1GkMzgvBtGQ+j/PPYe5WhDAvWwv1vxSHRFrBRALXnK1mjUgNwOOLj crQFkNj60cVzDuMTpFG7yKfbSCv7My0+eMLkt21BJbprDzgPGjuyomYjyQoYNHoiFavu Q1RmzlOrH0kGfLVPnrHYHtL2XyTt69JYOci3CUmQlu1Xe9sibRAkAGNUhE9VNnJ0gTYW bd8A==
X-Gm-Message-State: ANhLgQ3HGhzTUBO5QhQDkqyfzRamCCA/NHHJwplXKV/bKByAPxqqfGm8 o7GQfBsOMVzGCxULxg6PNLSJfGDe6QdnFwtzpxteyDiY
X-Google-Smtp-Source: ADFU+vsbLq3cEgWLhHZZEkLpvlOrAtjVShfdYCu6XGFm3E5uDIQi5YEGgm7UWBv2EGLq1+SAf+3hr+MZ3vdFmTfWt3c=
X-Received: by 2002:ab0:7418:: with SMTP id r24mr7966468uap.128.1584760710633; Fri, 20 Mar 2020 20:18:30 -0700 (PDT)
MIME-Version: 1.0
References: <158397136560.19639.8260312978650035895@ietfa.amsl.com> <CAAbpuyrL2yuqUCw2mgqKmdv7LWUWZMAC8nggpvbJ974SeVOM4A@mail.gmail.com> <CANUuoLpXN60KMp7_dnrCRqzPUk0857rQa8Q5zadMNnQrB7WgXA@mail.gmail.com>
In-Reply-To: <CANUuoLpXN60KMp7_dnrCRqzPUk0857rQa8Q5zadMNnQrB7WgXA@mail.gmail.com>
From: "Y. Richard Yang" <yry@cs.yale.edu>
Date: Fri, 20 Mar 2020 23:18:19 -0400
Message-ID: <CANUuoLrhDkskjW2n99VEN1qaewMXkKb1QbCdB_fiVeDpF3_99g@mail.gmail.com>
To: The IESG <iesg@ietf.org>, alto-chairs@ietf.org, draft-ietf-alto-incr-update-sse@ietf.org, IETF ALTO <alto@ietf.org>, "Vijay K. Gurbani" <vkg@bell-labs.com>
Content-Type: multipart/alternative; boundary="000000000000361b4905a154db66"
Archived-At: <https://mailarchive.ietf.org/arch/msg/alto/MAaagYzZr2hQVsHaT2YgiOizr3M>
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, 21 Mar 2020 03:18:42 -0000

Dear Ben,

We have submitted a new version -22, and please see this link for the
revisions that we have made from -20 to -22
https://www.ietf.org/rfcdiff?url1=draft-ietf-alto-incr-update-sse-20&url2=draft-ietf-alto-incr-update-sse-22

Thank you so much!
Richard

On Sun, Mar 15, 2020 at 10:36 PM Y. Richard Yang <yry@cs.yale.edu> 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?
>
>
>
>> 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?
>
> 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.
>
>
>
>> 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?
>
>
>
>>    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.
>
>
>> 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?
>
>
>> 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?
>
>
>> 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!
>
> Richard
>
>>
>>
>> _______________________________________________
>> alto mailing list
>> alto@ietf.org
>> https://www.ietf.org/mailman/listinfo/alto
>>
>