Subject: [Gen-art] Genart last call review of draft-ietf-alto-incr-update-sse-20

From: Elwyn Davies via Datatracker <[email-masked]>
Date: Mon, 09 Mar 2020 17:45:11 -0700

Reviewer: Elwyn Davies
Review result: Almost Ready

I am the assigned Gen-ART reviewer for this draft. The General Area
Review Team (Gen-ART) reviews all IETF documents being processed
by the IESG for the IETF Chair.  Please treat these comments just
like any other last call comments.

For more information, please see the FAQ at

<https://trac.ietf.org/trac/gen/wiki/GenArtfaq>.

Document: draft-ietf-alto-incr-update-sse-20
Reviewer: Elwyn Davies
Review Date: 2020-03-09
IETF LC End Date: 2020-03-06
IESG Telechat date: 2020-03-12

Summary:
Almost ready.  There are a few editorial issues, but I am not sure that

Major issues:
I am unsure whether this mechanism is proof against loss of messages or
reordering  of messags.  Although there are state tags, it does not appear to
have any way to ensure that the state to which the updates will be applied in
the client are identical to the state that the updates were generated from.  If
I am wrong, it would be useful (IMO) to explain how the proposal avoids getting
updates that don't apply to the state in the client.

Minor issues:

Nits/editorial comments:
Abstract: the abstract is too long; I would suggest deleting the second
sentence of the first paragraph and the whole of the second paragraph.  Ths
would leave sufficient information to explain what the document proposes but
omits the rationale which is not necessary for outlining the contents.   The
deleted text would be usefully incorporated into s1.

Abstract, para 3: s/s ction/section/

s1:  The key role of Server-Sent Events in this proposal is not introduced here
(and isn't mentioned in the Abstract).  In the process SSE needs to be expanded
on first use (currently right at the end of the section) and a pointer to the
document that defines SSE [SSE]

s1, last para: The reference to Section 13 should come right at the end - and
the last two sections are (no longer) the last sectons: s/last two
sections/Sections 11 and 12/

s2 et seq: I am unsure of the rationale for defining a set of special terms and
not capitalizing them on every occurrence.

s2:  There is quite a lot of terminology imported from RFC 7285 .  This should
be mentioned.

s3: A pointer to the SSE document would be useful [SSE].

s3.4: It would be better to use the expanded form of SSE in the first paragraph
rather than waiting till the 2nd para.

s4:  An explanation in advance  of the format of the lines delineated by **....
** would be desirable.

s5.1, next to last para:  s/ So there is no ambiguous decoding/ So there is no
ambiguity when decoding/

s5.1, last para: s/id/data-id/

s6.3, last para: s/will uses/will use/

s6,5, "incremental changes": s/Section Section6.3/Section 6.3/

s6.5, "remove":  Stating that the client SHOULD ignore this if it present is
potentially problematic.  If it is there it is a syntax error - should the
message be ignored and potentailly flagged as an error?

s7.6, last para: s/our modular/the modular/

s13/s13.1:Empty sections are not desirable  Please combine the two titles and
remove s13.1 .

---

From: "Y. Richard Yang" <[email-masked]>
Date: Tue, 10 Mar 2020 00:25:12 -0400

Dear Elwyn,

Thanks a lot for the review! Please see inline below.

> updates that don't apply to the state in the client.

Good comment! A short answer is that the design should have no consistency
problems.

More details:

(1) This design is based on http/1.x as transport, which provides a single,
reliable, in-order serialization of update messages: m1, m2, m3, ...
The transport will guarantee that the messages will be delivered lossless,
in order.

(2) One can consider that the messages consist of substreams (resources).
Each substream is total ordered as well.

(3) The only remaining case is that substreams can have dependencies: for
example a cost map can depend on a network map. The design requires that
the updates to such dependencies are ordered correctly.

One can see that the consistency model can be weakened: from total
serialization to causal consistency. We plan to design such a weaker (with
less head of line blocking of total order) using http/2.

I like this comment. How about that we add a realized consistency model
paragraph in the overview? What do you think?

> deleted text would be usefully incorporated into s1.

Okay.

> Abstract, para 3: s/s ction/section/

Thanks. Will fix.

> document that defines SSE [SSE]

Okay.

> sections/Sections 11 and 12/

Thanks a lot for identifying this. Will fix.

> not capitalizing them on every occurrence.

We feel that this is a style preference. We intended that the terms in Sec
2 are like keywords of a book. Capitalizing them on each occurrence appears
to be a bit too much, for personal style. We prefer to keep this style, but
do agree that some other ALTO documents use all capitalization.

> s2:  There is quite a lot of terminology imported from RFC 7285 .  This

Good catch. Will add a sentence at the beginning.

> s3: A pointer to the SSE document would be useful [SSE].

Yes. Will do.

> rather than waiting till the 2nd para.

Sure. Will do.

> s4:  An explanation in advance  of the format of the lines delineated by

Sure.

> s5.1, next to last para:  s/ So there is no ambiguous decoding/ So there

Good revision and will do.

> s5.1, last para: s/id/data-id/

Good catch, and will fix.

> s6.3, last para: s/will uses/will use/

Thanks. Will fix.

> s6,5, "incremental changes": s/Section Section6.3/Section 6.3/

Thanks. Will fix.

> message be ignored and potentailly flagged as an error?

The overall design strategy of alto is to ignore unknown fields to allow
incremental deployment—a kind of future proof of a future version by a
legacy old version. But in this case, I agree that it is a known error and
it is a good clarification. We will flag it as an error.

> s7.6, last para: s/our modular/the modular/

Thanks. Will fix.

> s13/s13.1:Empty sections are not desirable  Please combine the two titles

Okay.

Thanks again!

Richard

---

From: elwynd <[email-masked]>
Date: Tue, 10 Mar 2020 15:51:51 +0000

Hi, Richard.Sorry I was a bit rushed last night and should have said a bit more.  I think adding some text about how consistency is maintained would be a good solution.  As a non-expert in ALTO I was not really aware of the significance of the tag field when I started readig the draft.  Explaining the nature of the tag field and making sure that it is clear that the old value of the tag field in an update MUST match the value of the tag field as known by the client as the key indicator of state consistency would be a considerable improvement.Cheers,ElwynSent from Samsung tablet.
-------- Original message --------From: "Y. Richard Yang" <[email-masked]> Date: 10/03/2020  04:25  (GMT+00:00) To: Elwyn Davies <[email-masked]> Cc: [email-masked], [email-masked], [email-masked], [email-masked] Subject: Re: Genart last call review of draft-ietf-alto-incr-update-sse-20 Dear Elwyn,Thanks a lot for the review! Please see inline below.On Mon, Mar 9, 2020 at 8:45 PM Elwyn Davies via Datatracker <[email-masked]> wrote:Reviewer: Elwyn Davies
Review result: Almost Ready

Summary:

Major issues:
Good comment! A short answer is that the design should have no consistency problems. More details: (1) This design is based on http/1.x as transport, which provides a single, reliable, in-order serialization of update messages: m1, m2, m3, ...The transport will guarantee that the messages will be delivered lossless, in order.(2) One can consider that the messages consist of substreams (resources). Each substream is total ordered as well. (3) The only remaining case is that substreams can have dependencies: for example a cost map can depend on a network map. The design requires that the updates to such dependencies are ordered correctly.One can see that the consistency model can be weakened: from total serialization to causal consistency. We plan to design such a weaker (with less head of line blocking of total order) using http/2.I like this comment. How about that we add a realized consistency model paragraph in the overview? What do you think?
Minor issues:

Nits/editorial comments:
deleted text would be usefully incorporated into s1.Okay.

Abstract, para 3: s/s ction/section/Thanks. Will fix.

s1:  The key role of Server-Sent Events in this proposal is not introduced here
(and isn't mentioned in the Abstract).  In the process SSE needs to be expanded
on first use (currently right at the end of the section) and a pointer to the
document that defines SSE [SSE]Okay.

s1, last para: The reference to Section 13 should come right at the end - and
the last two sections are (no longer) the last sectons: s/last two
sections/Sections 11 and 12/Thanks a lot for identifying this. Will fix.

s2 et seq: I am unsure of the rationale for defining a set of special terms and
not capitalizing them on every occurrence.We feel that this is a style preference. We intended that the terms in Sec 2 are like keywords of a book. Capitalizing them on each occurrence appears to be a bit too much, for personal style. We prefer to keep this style, but do agree that some other ALTO documents use all capitalization.

rather than waiting till the 2nd para.Sure. Will do.

s4:  An explanation in advance  of the format of the lines delineated by **....
** would be desirable.
Sure.
s5.1, next to last para:  s/ So there is no ambiguous decoding/ So there is no
ambiguity when decoding/
Good revision and will do.
s5.1, last para: s/id/data-id/Good catch, and will fix.

s6.3, last para: s/will uses/will use/Thanks. Will fix.

s6,5, "incremental changes": s/Section Section6.3/Section 6.3/Thanks. Will fix.

s6.5, "remove":  Stating that the client SHOULD ignore this if it present is
potentially problematic.  If it is there it is a syntax error - should the
message be ignored and potentailly flagged as an error?The overall design strategy of alto is to ignore unknown fields to allow incremental deployment—a kind of future proof of a future version by a legacy old version. But in this case, I agree that it is a known error and it is a good clarification. We will flag it as an error.

s7.6, last para: s/our modular/the modular/Thanks. Will fix.

s13/s13.1:Empty sections are not desirable  Please combine the two titles and
remove s13.1 .Okay. Thanks again!Richard

---

From: "Y. Richard Yang" <[email-masked]>
Date: Tue, 10 Mar 2020 15:24:08 -0400

Dear Elwyn,

Thank you so much for the additional comment! Sure we will add the text,
and make clear on the nature and use of tag. It looks that the upload site
is opened again, and we will upload a new version as soon as it will not
lead to confusion of other reviews.

Richard

On Tue, Mar 10, 2020 at 11:51 AM elwynd <[email-masked]> wrote:

> Hi, Richard.
>
> Sorry I was a bit rushed last night and should have said a bit more.
>
> I think adding some text about how consistency is maintained would be a
> good solution.  As a non-expert in ALTO I was not really aware of the
> significance of the tag field when I started readig the draft.  Explaining
> the nature of the tag field and making sure that it is clear that the old
> value of the tag field in an update MUST match the value of the tag field
> as known by the client as the key indicator of state consistency would be a
> considerable improvement.
>
> Cheers,
> Elwyn
>
>
>
> Sent from Samsung tablet.
>
>
> -------- Original message --------
> From: "Y. Richard Yang" <[email-masked]>
> Date: 10/03/2020 04:25 (GMT+00:00)
> To: Elwyn Davies <[email-masked]>
> Cc: [email-masked], [email-masked],
> [email-masked], [email-masked]
> Subject: Re: Genart last call review of draft-ietf-alto-incr-update-sse-20
>
> Dear Elwyn,
>
> (1) This design is based on http/1.x as transport, which provides a
> single, reliable, in-order serialization of update messages: m1, m2, m3, ...

-- 
-- 
 =====================================
| Y. Richard Yang <[email-masked]>   |
| Professor of Computer Science       |
| http://www.cs.yale.edu/~yry/        |
 =====================================

---

From: Alissa Cooper <[email-masked]>
Date: Wed, 11 Mar 2020 20:59:03 -0400

Elwyn, thanks for your review. Richard, thanks for your response. I entered a No Objection ballot.

Alissa

> On Mar 10, 2020, at 3:24 PM, Y. Richard Yang <[email-masked]> wrote:
> 
> Dear Elwyn,
> 
> Thank you so much for the additional comment! Sure we will add the text, and make clear on the nature and use of tag. It looks that the upload site is opened again, and we will upload a new version as soon as it will not lead to confusion of other reviews.
> 
> Richard
> 
> On Tue, Mar 10, 2020 at 11:51 AM elwynd <[email-masked] <mailto:[email-masked]>> wrote:
> Hi, Richard.
> 
> I think adding some text about how consistency is maintained would be a good solution.  As a non-expert in ALTO I was not really aware of the significance of the tag field when I started readig the draft.  Explaining the nature of the tag field and making sure that it is clear that the old value of the tag field in an update MUST match the value of the tag field as known by the client as the key indicator of state consistency would be a considerable improvement.
> 
> Cheers,
> Elwyn
> 
> 
> 
> From: "Y. Richard Yang" <[email-masked] <mailto:[email-masked]>> 
> To: Elwyn Davies <[email-masked] <mailto:[email-masked]>> 
> Cc: [email-masked] <mailto:[email-masked]>, [email-masked] <mailto:[email-masked]>, [email-masked] <mailto:[email-masked]>, [email-masked] <mailto:[email-masked]>
> On Mon, Mar 9, 2020 at 8:45 PM Elwyn Davies via Datatracker <[email-masked] <mailto:[email-masked]>> wrote:
> Reviewer: Elwyn Davies
> <https://trac.ietf.org/trac/gen/wiki/GenArtfaq <https://trac.ietf.org/trac/gen/wiki/GenArtfaq>>.
> 
> Good comment! A short answer is that the design should have no consistency problems. 
> 
> More details: 
> 
> (1) This design is based on http/1.x as transport, which provides a single, reliable, in-order serialization of update messages: m1, m2, m3, ...
> The transport will guarantee that the messages will be delivered lossless, in order.
> 
> (2) One can consider that the messages consist of substreams (resources). Each substream is total ordered as well. 
> 
> (3) The only remaining case is that substreams can have dependencies: for example a cost map can depend on a network map. The design requires that the updates to such dependencies are ordered correctly.
> 
> One can see that the consistency model can be weakened: from total serialization to causal consistency. We plan to design such a weaker (with less head of line blocking of total order) using http/2.
> 
> I like this comment. How about that we add a realized consistency model paragraph in the overview? What do you think?
> 
> 
> 
> Minor issues:
> 
> Nits/editorial comments:
> We feel that this is a style preference. We intended that the terms in Sec 2 are like keywords of a book. Capitalizing them on each occurrence appears to be a bit too much, for personal style. We prefer to keep this style, but do agree that some other ALTO documents use all capitalization.
> 
> 
> 
> The overall design strategy of alto is to ignore unknown fields to allow incremental deployment—a kind of future proof of a future version by a legacy old version. But in this case, I agree that it is a known error and it is a good clarification. We will flag it as an error.
> 
> 
> 
> | Y. Richard Yang <[email-masked] <mailto:[email-masked]>>   |
> | http://www.cs.yale.edu/~yry/ <http://www.cs.yale.edu/~yry/>        |
> _______________________________________________
> Gen-art mailing list
> [email-masked] <mailto:[email-masked]>
> https://www.ietf.org/mailman/listinfo/gen-art <https://www.ietf.org/mailman/listinfo/gen-art>

---

From: "Y. Richard Yang" <[email-masked]>
Date: Thu, 12 Mar 2020 11:22:23 -0400

Ack, Alissa. Thanks!

Richard

On Wed, Mar 11, 2020 at 8:59 PM Alissa Cooper <[email-masked]> wrote:

>> We feel that this is a style preference. We intended that the terms in
>> Sec 2 are like keywords of a book. Capitalizing them on each occurrence
>> appears to be a bit too much, for personal style. We prefer to keep this
>> style, but do agree that some other ALTO documents use all capitalization.
>>
>>
>>>
> [email-masked]
> https://www.ietf.org/mailman/listinfo/gen-art
>
>
> --
Richard

---

From: Benjamin Kaduk <[email-masked]>
Date: Sat, 14 Mar 2020 21:09:12 -0700

> The transport will guarantee that the messages will be delivered lossless,

I did not remember getting the impression that it was required to use
HTTP/1.x transport with this specification, as I attempted to note in my
ballot, SSE is not HTTP/1.x-specific.  If the intent is to only allow the
usage of HTTP/1.x with HTTP/2 (or HTTP/3) left to future work, I think that
(e.g.) the end of Section 3.3 needs to be more explicit about this.

-Ben

---

From: "Y. Richard Yang" <[email-masked]>
Date: Sun, 15 Mar 2020 00:21:26 -0400

Hi Ben,

We are working on finishing a reply to your thorough review, which we will
post tomorrow.

Regarding this comment, please see below.

On Sun, Mar 15, 2020 at 12:09 AM Benjamin Kaduk <[email-masked]> wrote:

> > > reordering  of messags.  Although there are state tags, it does not
> appear
> > > to
> > > have any way to ensure that the state to which the updates will be
> applied
> > > in
> single,

For the current design, one can use SSE with either http/1.x or http/2
transport, but the use of http/2 will not take advantage of essential
features of http/2 such as streams allowing more concurrent transmissions.
Hence, the plan is to finish http/1.x or more generally
single-serialization transport, and we will make it more explicit in the
document such as 3.3.

We will send an update to address your full review tomorrow.

Thank you so much!

Richard

-- 
Richard

---

From: "Y. Richard Yang" <[email-masked]>
Date: Thu, 19 Mar 2020 13:36:13 -0400

Hi Ben,

Just an update that we have submitted -21 and will upload -22 tomorrow (we
submitted -22 twice but somehow it did not post; we will take the
opportunity of waiting for one day to proofread the document and upload by
the end of Friday for you to check if the revision handles your review,
which is exceedingly valuable.

Richard

-- 
Richard