Subject: [Taps] Review of draft-ietf-taps-arch

From: Michael Welzl <[email-masked]>
Date: Wed, 20 Oct 2021 15:31:55 +0200

[ I don't think this is worth time at today's interim. These should generally be quite quick to address. ]

Hello TAPS!

A few interims ago I volunteered to review draft-ietf-taps-arch. I'm obviously biased in everything TAPS, yet I'm not an author of this particular draft, nor have I read it in a long time...
After volunteering, I expected to receive an email pinging me about it. I never did, and now I think that I shouldn't have waited for an email - I believe that this was a misunderstanding. Be that as it may, now I have finally reviewed it   :)

This review concerns the "Editor's copy" version at https://ietf-tapswg.github.io/api-drafts/draft-ietf-taps-arch.html   as of 11:51 (my time) on 19. 10. I believe that it includes the latest PR from Gorry, and it probably matches the latest version at the time of writing this.
I'll just put all my comments here in sequence of appearance in the document instead of grouping them and opening issues, and leave it to the document authors to decide what's a nit and what's a content change that warrants further discussion - I trust you to "do the right thing" (tm)    (*truly* a trademark, though not mine   :-)   ).

Without further ado, here come my comments:

Introduction paragraph 2: here we have 3 occurrences of "provide". I would replace the middle one - e.g.: "...reusable architecture that offers a common interface ..."

Section 1.2, item 3, starting with "Section 4 presents...": I believe that the last ( = second) sentence of this item must have ended here by error, it seems out of place ("The Preconnection allows applications..."), and I think it can safely be removed.

About the last sentence in item 4, see: https://github.com/ietf-tapswg/api-drafts/issues/939

Section 2, right below Figure 2:
"The Transport Services API [I-D.ietf-taps-interface] defines the interface for an application to create network connections and transfer data."
=> Should this be "Connections" instead of "network connections" ?

Section 2.1, paragraph 2, last line: "... made explicit in sockets." seems strange. Should this be "in the sockets API." ?

Section 2.2 paragraph 1, this sentence:
"While TCP does indeed provide the ability to send and receive data as streams, most applications need to interpret structure within these streams."
Two issues here. First, I would switch to singular because TCP is not a multi-streaming protocol. I.e., change this to:
"While TCP does indeed provide the ability to send and receive data as a stream, most applications need to interpret structure within this stream."
Second, is "stream" a known concept? I don't think it's defined in this document, and I'm a little unsure because, even when using singular, someone might read "stream" and think of "one stream out of several, in a multi-streaming protocol". Maybe this is nit-picking, but if you agree, I would prefer to replace "stream" with "data stream", or having a definition of "stream" and capitalizing it (but then the definition would have to appear before the first use of this term...)

Next paragraph: double "provide" in: "Providing a message-based abstraction provides many benefits, such as:"  - I suggest to replace the second with "has".

In the following item list, I would remove the third item: "the ability to manage dependencies between messages, ..."
While it is true that a message-based abstraction enables this, a reader of this document might get the impression that the TAPS system defined in the interface draft offers this specific functionality. It really doesn't, it would still have to be implemented on top of it. For this reason, this item strikes me as a bit misleading, and I think it should be removed.

Section 3, paragraph 2: "it can provide access to multiple sets of protocols and protocol features" - remove "sets of", this adds an unnecessary level of indirection here. I guess the transport property profiles in interface, section B.2 would be such sets, but still...

Last paragraph before section 3.2: "Applications using a Transport Services system interface are REQUIRED to be robust to the automated selection provided by the system."  - why say "system interface" here all of a sudden? This gives an impression as if there were two things: the API, and then perhaps also a separate "system interface". I'd rather replace this with "API".

Last paragraph of section 3.4: "In normal use, a Transport Services system SHOULD result in consistent protocol and interface selection decisions for the same network conditions given the same set of Properties."
Isn't this too restrictive? It prohibits a Transport Services system from improving. I would change this to:
"In normal use, a given version of a Transport Services system SHOULD result in consistent protocol and interface selection decisions for the same network conditions given the same set of Properties."

Section 4.1.5: "For example, a Message can be marked with a Message Property indicating that it is the final message on a connection if the peer sent a TCP FIN."
I would stop this sentence after "on a connection." - or is it just me misunderstanding something?  I thought (and -interface section 9.1.3.5 seems to agree with me) that the idea of "final message for TCP" really is that setting the property means that THIS message that I'm sending gets a FIN flag. Why would a FIN from the peer make me set this property? FWIW I could go on sending more data, and use this property much later?

Section 4.1.7, item 1: "end of stream in the ..."  - see above for a discussion of the word "stream".

Section 4.2, item 5: "One Protocol Stack that can be used by an application for a connection, of which there can be several."  => I believe it should be "Connection" here, not "connection".

Section 4.2.2, first sentence: "or some combination of all of these" - should this be "or using some combination of all of these" ?

Section 4.2.3, item 3: "Performance concerns about Connections introducing head-of-line blocking due to multiplexing or needing to share state on a single thread."
I don't think application programmers should decide against using functions such as Connection Grouping because they try to optimize their performance in this way. If it performs poorly, the Transport Services system shouldn't do it. I know this isn't always black and white... I'm guessing that this was written with HTTP/2 in mind. Still ... if a mechanism is judged as generally performing better most of the time, the system should probably use it. If it doesn't perform better e.g. depending on the length of transfers, which may be known from HTTP header, then maybe the Transport Services system is the ideal place to dynamically make such a decision?
Anyway - I don't think that we should indirectly suggest to application writers that internal functions chosen by the Transport Services system on the basis of high-level information like "these Connections belong together" might in fact perform poorly, and hence they may be better off not using them if they want the best performance for their application. So, I would remove this item.

I hope this is useful !

Cheers,
Michael

---

From: "Aaron Falk" <[email-masked]>
Date: Wed, 20 Oct 2021 10:00:00 -0400

Hi Michael-

Thanks for the review.  Glad to see the doc hasn’t ‘aged’ much as 
the others have been refined.  Let’s assume I dropped the ball on 
reminding you about the review.  :)

--aaron

On 20 Oct 2021, at 9:31, Michael Welzl wrote:

> A few interims ago I volunteered to review draft-ietf-taps-arch. I'm 
> obviously biased in everything TAPS, yet I'm not an author of this 
> particular draft, nor have I read it in a long time...
> After volunteering, I expected to receive an email pinging me about 
> it. I never did, and now I think that I shouldn't have waited for an 
> email - I believe that this was a misunderstanding. Be that as it may, 
> now I have finally reviewed it   :)
>
> This review concerns the "Editor's copy" version at 
> https://ietf-tapswg.github.io/api-drafts/draft-ietf-taps-arch.html   
> as of 11:51 (my time) on 19. 10. I believe that it includes the latest 
> PR from Gorry, and it probably matches the latest version at the time 
> of writing this.
> I'll just put all my comments here in sequence of appearance in the 
> document instead of grouping them and opening issues, and leave it to 
> the document authors to decide what's a nit and what's a content 
> change that warrants further discussion - I trust you to "do the right 
> thing" (tm)    (*truly* a trademark, though not mine   :-)   ).
>
>
> Introduction paragraph 2: here we have 3 occurrences of "provide". I 
> would replace the middle one - e.g.: "...reusable architecture that 
> offers a common interface ..."
>
> Section 1.2, item 3, starting with "Section 4 presents...": I believe 
> that the last ( = second) sentence of this item must have ended here 
> by error, it seems out of place ("The Preconnection allows 
> applications..."), and I think it can safely be removed.
>
> About the last sentence in item 4, see: 
> https://github.com/ietf-tapswg/api-drafts/issues/939
>
> "The Transport Services API [I-D.ietf-taps-interface] defines the 
> interface for an application to create network connections and 
> transfer data."
> Section 2.1, paragraph 2, last line: "... made explicit in sockets." 
> seems strange. Should this be "in the sockets API." ?
>
> "While TCP does indeed provide the ability to send and receive data as 
> streams, most applications need to interpret structure within these 
> streams."
> Two issues here. First, I would switch to singular because TCP is not 
> a multi-streaming protocol. I.e., change this to:
> "While TCP does indeed provide the ability to send and receive data as 
> a stream, most applications need to interpret structure within this 
> stream."
> Second, is "stream" a known concept? I don't think it's defined in 
> this document, and I'm a little unsure because, even when using 
> singular, someone might read "stream" and think of "one stream out of 
> several, in a multi-streaming protocol". Maybe this is nit-picking, 
> but if you agree, I would prefer to replace "stream" with "data 
> stream", or having a definition of "stream" and capitalizing it (but 
> then the definition would have to appear before the first use of this 
> term...)
>
> Next paragraph: double "provide" in: "Providing a message-based 
> abstraction provides many benefits, such as:"  - I suggest to replace 
> the second with "has".
>
> In the following item list, I would remove the third item: "the 
> ability to manage dependencies between messages, ..."
> While it is true that a message-based abstraction enables this, a 
> reader of this document might get the impression that the TAPS system 
> defined in the interface draft offers this specific functionality. It 
> really doesn't, it would still have to be implemented on top of it. 
> For this reason, this item strikes me as a bit misleading, and I think 
> it should be removed.
>
> Section 3, paragraph 2: "it can provide access to multiple sets of 
> protocols and protocol features" - remove "sets of", this adds an 
> unnecessary level of indirection here. I guess the transport property 
> profiles in interface, section B.2 would be such sets, but still...
>
> Last paragraph before section 3.2: "Applications using a Transport 
> Services system interface are REQUIRED to be robust to the automated 
> selection provided by the system."  - why say "system interface" here 
> all of a sudden? This gives an impression as if there were two things: 
> the API, and then perhaps also a separate "system interface". I'd 
> rather replace this with "API".
>
> Last paragraph of section 3.4: "In normal use, a Transport Services 
> system SHOULD result in consistent protocol and interface selection 
> decisions for the same network conditions given the same set of 
> Properties."
> "In normal use, a given version of a Transport Services system SHOULD 
> result in consistent protocol and interface selection decisions for 
> the same network conditions given the same set of Properties."
>
> Section 4.1.5: "For example, a Message can be marked with a Message 
> Property indicating that it is the final message on a connection if 
> the peer sent a TCP FIN."
> I would stop this sentence after "on a connection." - or is it just me 
> misunderstanding something?  I thought (and -interface section 9.1.3.5 
> seems to agree with me) that the idea of "final message for TCP" 
> really is that setting the property means that THIS message that I'm 
> sending gets a FIN flag. Why would a FIN from the peer make me set 
> this property? FWIW I could go on sending more data, and use this 
> property much later?
>
> Section 4.2, item 5: "One Protocol Stack that can be used by an 
> application for a connection, of which there can be several."  => I 
> believe it should be "Connection" here, not "connection".
>
> Section 4.2.2, first sentence: "or some combination of all of these" - 
> should this be "or using some combination of all of these" ?
>
> Section 4.2.3, item 3: "Performance concerns about Connections 
> introducing head-of-line blocking due to multiplexing or needing to 
> share state on a single thread."
> I don't think application programmers should decide against using 
> functions such as Connection Grouping because they try to optimize 
> their performance in this way. If it performs poorly, the Transport 
> Services system shouldn't do it. I know this isn't always black and 
> white... I'm guessing that this was written with HTTP/2 in mind. Still 
> ... if a mechanism is judged as generally performing better most of 
> the time, the system should probably use it. If it doesn't perform 
> better e.g. depending on the length of transfers, which may be known 
> from HTTP header, then maybe the Transport Services system is the 
> ideal place to dynamically make such a decision?
> Anyway - I don't think that we should indirectly suggest to 
> application writers that internal functions chosen by the Transport 
> Services system on the basis of high-level information like "these 
> Connections belong together" might in fact perform poorly, and hence 
> they may be better off not using them if they want the best 
> performance for their application. So, I would remove this item.
>
>
> I hope this is useful !
>
> Cheers,
> Michael
>
> _______________________________________________
> Taps mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/taps

---

From: Gorry Fairhurst <[email-masked]>
Date: Wed, 20 Oct 2021 15:44:16 +0100

These all look reasonable, and most easy to resolve. Now as github issues:-)

Gorry

On 20/10/2021 15:00, Aaron Falk wrote:
>
> Hi Michael-
>
> Thanks for the review. Glad to see the doc hasn’t ‘aged’ much as the 
> others have been refined. Let’s assume I dropped the ball on reminding 
> you about the review. :)
>
> --aaron
>
>     [ I don't think this is worth time at today's interim. These
>     should generally be quite quick to address. ]
>
>
>     Hello TAPS!
>
>     A few interims ago I volunteered to review draft-ietf-taps-arch.
>     I'm obviously biased in everything TAPS, yet I'm not an author of
>     this particular draft, nor have I read it in a long time...
>     After volunteering, I expected to receive an email pinging me
>     about it. I never did, and now I think that I shouldn't have
>     waited for an email - I believe that this was a misunderstanding.
>     Be that as it may, now I have finally reviewed it :)
>
>     <https://ietf-tapswg.github.io/api-drafts/draft-ietf-taps-arch.html>
>     as of 11:51 (my time) on 19. 10. I believe that it includes the
>     latest PR from Gorry, and it probably matches the latest version
>     at the time of writing this.
>     I'll just put all my comments here in sequence of appearance in
>     the document instead of grouping them and opening issues, and
>     leave it to the document authors to decide what's a nit and what's
>     a content change that warrants further discussion - I trust you to
>     "do the right thing" (tm) (*truly* a trademark, though not mine :-) ).
>
>
>     Introduction paragraph 2: here we have 3 occurrences of "provide".
>     I would replace the middle one - e.g.: "...reusable architecture
>     that offers a common interface ..."
>
>     Section 1.2, item 3, starting with "Section 4 presents...": I
>     believe that the last ( = second) sentence of this item must have
>     ended here by error, it seems out of place ("The Preconnection
>     allows applications..."), and I think it can safely be removed.
>
>     <https://github.com/ietf-tapswg/api-drafts/issues/939>
>
>     Section 2.1, paragraph 2, last line: "... made explicit in
>     sockets." seems strange. Should this be "in the sockets API." ?
>
>     "While TCP does indeed provide the ability to send and receive
>     data as streams, most applications need to interpret structure
>     within these streams."
>     Two issues here. First, I would switch to singular because TCP is
>     not a multi-streaming protocol. I.e., change this to:
>     "While TCP does indeed provide the ability to send and receive
>     data as a stream, most applications need to interpret structure
>     within this stream."
>     singular, someone might read "stream" and think of "one stream out
>     of several, in a multi-streaming protocol". Maybe this is
>     nit-picking, but if you agree, I would prefer to replace "stream"
>     with "data stream", or having a definition of "stream" and
>     capitalizing it (but then the definition would have to appear
>     before the first use of this term...)
>
>     abstraction provides many benefits, such as:" - I suggest to
>     replace the second with "has".
>
>     reader of this document might get the impression that the TAPS
>     system defined in the interface draft offers this specific
>     functionality. It really doesn't, it would still have to be
>     implemented on top of it. For this reason, this item strikes me as
>     a bit misleading, and I think it should be removed.
>
>     unnecessary level of indirection here. I guess the transport
>     property profiles in interface, section B.2 would be such sets,
>     but still...
>
>     Services system interface are REQUIRED to be robust to the
>     automated selection provided by the system." - why say "system
>     interface" here all of a sudden? This gives an impression as if
>     there were two things: the API, and then perhaps also a separate
>     "system interface". I'd rather replace this with "API".
>
>     Last paragraph of section 3.4: "In normal use, a Transport
>     Services system SHOULD result in consistent protocol and interface
>     selection decisions for the same network conditions given the same
>     set of Properties."
>     Isn't this too restrictive? It prohibits a Transport Services
>     system from improving. I would change this to:
>     "In normal use, a given version of a Transport Services system
>     SHOULD result in consistent protocol and interface selection
>     Section 4.1.5: "For example, a Message can be marked with a
>     Message Property indicating that it is the final message on a
>     connection if the peer sent a TCP FIN."
>     I would stop this sentence after "on a connection." - or is it
>     just me misunderstanding something? I thought (and -interface
>     section 9.1.3.5 seems to agree with me) that the idea of "final
>     message for TCP" really is that setting the property means that
>     THIS message that I'm sending gets a FIN flag. Why would a FIN
>     from the peer make me set this property? FWIW I could go on
>     sending more data, and use this property much later?
>
>     Section 4.2.2, first sentence: "or some combination of all of
>     these" - should this be "or using some combination of all of these" ?
>
>     introducing head-of-line blocking due to multiplexing or needing
>     to share state on a single thread."
>     their performance in this way. If it performs poorly, the
>     Transport Services system shouldn't do it. I know this isn't
>     always black and white... I'm guessing that this was written with
>     HTTP/2 in mind. Still ... if a mechanism is judged as generally
>     performing better most of the time, the system should probably use
>     it. If it doesn't perform better e.g. depending on the length of
>     transfers, which may be known from HTTP header, then maybe the
>     Transport Services system is the ideal place to dynamically make
>     such a decision?
>     application writers that internal functions chosen by the
>     Transport Services system on the basis of high-level information
>     like "these Connections belong together" might in fact perform
>     poorly, and hence they may be better off not using them if they
>     want the best performance for their application. So, I would
>     remove this item.
>
>
>     I hope this is useful !
>
>     Cheers,
>     Michael
>
>     <https://www.ietf.org/mailman/listinfo/taps>
>
>