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:
[ 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
_______________________________________________
Taps mailing list
Taps@ietf.org
https://www.ietf.org/mailman/listinfo/taps
_______________________________________________
Taps mailing list
Taps@ietf.org
https://www.ietf.org/mailman/listinfo/taps
