Peter,
Thanks for the thorough review of the draft.
Please see in line my proposed resolutions.
Thanks,
Eric Noel
AT&T Labs, Inc.
Rethink Possible
Optimization, Reliability and Customer Analytics
200 South Laurel Avenue, D5-3D19
Middletown, NJ 07748
P: 732.420.4174
ecnoel(_at_)att(_dot_)com
-----Original Message-----
From: Peter Yee [mailto:peter(_at_)akayla(_dot_)com]
Sent: Saturday, August 23, 2014 3:07 AM
To:
draft-ietf-soc-overload-rate-control(_dot_)all(_at_)tools(_dot_)ietf(_dot_)org
Cc: gen-art(_at_)ietf(_dot_)org; ietf(_at_)ietf(_dot_)org
Subject: Gen-ART review of draft-ietf-soc-overload-rate-control-09
I am the assigned Gen-ART reviewer for this draft. For background on
Gen-ART, please see the FAQ at
<http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>
Please resolve these comments along with any other Last Call comments you
may receive.
Document: review of draft-ietf-soc-overload-rate-control-09
Reviewer: Peter Yee
Review Date: August-22-2014
IETF LC End Date: August-22-2014
IESG Telechat date: TBD
Summary: This draft is on the right track but has open issues, described in
the review. [Ready with issues]
The draft describes an alternate SIP overload control mechanism. Instead of
the default, loss-based scheme, it proposes an optional, rate-based scheme.
The draft is mostly straightforward although the language is labored in some
places. Coordination with draft-ietf-soc-overload-control should take place
before this draft advances to ensure proper alignment with the final form of
that draft.
Issues:
Page 4, 4th paragraph: draft-ietf-soc-overload-control tends to speak of
parameters in the singular since they are being dealt with in a single Via
header field between a client and server. In this particular case, the
wording ends up sounding like a single Via header field might have more than
one oc parameter. Perhaps aligning with the base document would be best?
[EN] Aligned with draft-ietf-soc-overload-control by using parameter in the
singular form. Change reflected throughout the document when applicable.
Page 6, 3rd paragraph: this whole paragraph is presumptive of all clients
performing rate-based overload control, but that presumption isn't stated.
Given that this document is introducing a second overload control algorithm,
it might not be realistic to assume that all clients implement or select the
same algorithm let alone any algorithm at all.
[EN] At the end of section 3.4, I added the following text:
"When a client supports the default loss algorithm and not the rate algorithm,
the client would be handled in the same way as in
draft-ietf-soc-overload-control
section 5.10.2. "
Also, there's an unspoken
assumption in the scenario that clients are not initiating any traffic
themselves. Is that always going to be the case?
[EN] In our approach, all load sent to an overloaded server applying
the proposed scheme is subject to the leaky bucket control, whether
generated by clients or traversing clients.
Page 6, 5th paragraph: this paragraph is supposing that all clients of the
server support overload control and furthermore support rate-based control,
but these suppositions aren't mentioned. The oc parameter should only be
returned to clients that have indicated they support overload control. I
know you know this, but the wording isn't clear on that point.
[EN] Would stating
" Unless otherwise specified, all SIP entities described in this
document are assumed to support this specification."
In section 2 suffices? (Basically following draft-ietf-soc-overload-control)
Page 7, 1st full paragraph: what's being called "oc" in the formula isn't
really "oc" but rather "oc value". While it's probably clear what you
meant, the base spec talks about the server inserting a value into this
parameter.
[EN] A few reviewers caught that issue as well. The next version will
use "oc" parameter . So instead of 1/oc we will be using 1/["oc" parameter]
and were ever "oc" is used we will be using "oc" parameter
Page 14, Section 5, "algo-value" extension: This is currently called
algo-list in draft-ietf-soc-overload-control-15, the last version of that
spec. RFC 3261 doesn't have an algo-value definition.
[EN] Made the correction
Nits:
Page 1, author block: far be it for me to tell an author what his name is,
but I would suggesting changing " Philip M Williams" to " Philip M.
Williams" unless M is that author's middle name.
[EN] Replaced M with M.
Page 2, Introduction, 1st paragraph, 1st sentence: change "large scale" to
"large-scale".
[EN] Done
Page 2, Introduction, 1st paragraph, 1st sentence: change "SIP based" to
"SIP-based". Do this globally in the document.
[EN] Done
Page 3, 1st paragraph, 1st sentence: insert "a" before "loss".
[EN] Done
Page 3, 1st paragraph, 2nd sentence: append "scheme" after "control".
Insert "that" before "clients" and delete the subsequent "to".
[EN] Done
Page 3, 2nd paragraph, 1st sentence: insert "scheme" after "control".
[EN] Done
Page 3, 2nd paragraph, last sentence: delete the comma after "client".
[EN] Done
Page 3, 3rd paragraph: change "a rate based" to "a rate-based". Append a
comma after "default". Insert "scheme" between "control" and "in".
[EN] Done everything but the comma. Could you confirm?
Page 4, 3rd paragraph: append a comma after "e.g.". Do this globally and
also for "i.e.".
[EN] Done
Page 4, 3rd paragraph: insert "or" between "utilization" and "queueing".
Delete the comma after "utilization". Delete the ellipsis.
[EN] Done
Page 4, Section 3.2, 1st paragraph, 1st sentence: capitalize "via". Append
"field" after "header".
[EN] Done
Page 5, 1st paragraph, 2nd sentence: insert "the" before "server".
[EN] Done
Page 5, 1st paragraph, 3rd sentence: delete "conjunction for".
[EN] Done
Page 5, Section 3.3, 1st paragraph, 1st sentence: insert "the" before "Via"
and append "field" after "header".
[EN] Done
Page 5, Section 3.3, 1st paragraph, 3rd sentence: append "field" after
"header".
[EN] Done
Page 5, Section 3.3, 1st paragraph, last sentence: change "rate based" to
"rate-based".
[EN] Done
Page 5, Section 3.4, 3rd paragraph, 1st sentence: replace "max" with
"maximum".
[EN] Done
Page 6, 1st partial paragraph: replace "the" with "a".
[EN] Done
Page 6, 1st full paragraph: replace "cpu" with "CPU" in both uses.
[EN] Done
Page 6, 2nd paragraph: delete the comma.
[EN] Done
Page 6, 4th paragraph: delete the first comma.
[EN] Done
Page 6, 4th paragraph: append to the paragraph something like " and that
rate-based control is in effect". The server needs to indicate the rate (in
the oc parameter), but it must also inform the client which algorithm has
been selected.
[EN] Done
Page 6, 5th paragraph: append "the" after "use".
[EN] Done
Page 6, section 3.5.1: move the definition of "T" to this paragraph from the
following one. Or consider just saying "oc value" since that's what 1/T
(1/1/oc value) works out to.
[EN] To address another comment, made the following change:
"In determining whether or not to transmit a specific message, the client may
use any algorithm that limits the message rate to the "oc" parameter in units
of messages per second. For ease of discussion, we define T = 1/["oc"
parameter] as the target inter-SIP request interval. The algorithm may be
strictly deterministic, or it may be probabilistic. It may, or may not, have a
tolerance factor, to allow for short bursts, as long as the long term rate
remains below 1/T."
Page 7, 1st partial paragraph: insert a space into "RFC5390". Or place it
in square brackets as a reference.
[EN] Inserted a space
Page 7, 2nd full paragraph: replace "out of scope" with "out-of-scope".
[EN] Done
Page 7, 4th paragraph: replace "Leaky Bucket" with "leaky bucket" and use
the lower case version throughout the draft.
[EN] Done
Page 8, 2nd paragraph: there's a weird break in the paragraph that should be
fixed.
[EN] Done
Page 8, 2nd paragraph: last sentence: replace "burstyness" with
"burstiness". This spelling seems to be more prevalent.
[EN] Done
Page 9, 3rd paragraph: change "rate" to "rates".
[EN] Done
Page 9, 4th paragraph: delete the commas. Change "is" to "are".
[EN] Done
Page 9, No priority case: TargetRate is not defined. It should be noted
that it is the value of the oc parameter, as received from the server. ta
might be more clearly defined as "arrival time of the most recent SIP
request received by the client". The same applies to the prioritized case
further down.
[EN] Done. Note, I replaced TargetRate with ["oc" parameter]
Page 10, 1st paragraph, phrase after the colon: rewrite the first part as
"Requests that are candidates for reduction and requests not subject to
reduction"
[EN] Done
Page 10, 2nd paragraph: replace the first "Leaky" with "leaky" in all uses
except when used in the term "Leaky Bucket algorithm".
[EN] Done. Note, I replaced "Leaky Bucket" with "leaky bucket" across the whole
document.
Page 10, 2nd paragraph first sentence: replace "requests candidate" with
"request candidates".
[EN] Done
Page 10, 2nd paragraph, 3rd bullet item: insert "at or" before "above".
[EN] Done
Page 10, 3rd paragraph, 1st sentence: the use of "<=" contradicts the
requirement for the Leaky Bucket algorithm as given
[EN] Replaced <= with <
Page 10, 4th paragraph: change "is" to "are".
[EN] Done
Page 10, 5th paragraph: replace the "&" with "and".
[EN] Done
Page 10, 6th paragraph: delete the commas and change "is" to "are".
[EN] Done
Page 11, priority case, TAU1 definition: hyphenate "no priority".
[EN] Done
Page 11, Section 3.5.3, second sentence: I understand what's trying to be
said, but the wording is difficult. Consider rewording.
[EN] Done. Replaced this" with "resonance"
Page 12, 1st partial paragraph: delete the first comma.
[EN] Done
Page 12, 1st full paragraph, else clause: insert " let u be set to a random
value" before "uniformly".
[EN] Done
Page 12, 4th paragraph, 4th bullet item: replace "At" with "As".
[EN] Done
Page 12, Section 4: insert "the" before the reference.
[EN] To address another comment, made the following change:
"Adapting the example in section 6.2 of [draft-ietf-soc-overload-control-15],
where client P1 sends requests to a downstream server P2"
Page 13, 1st full paragraph: append "field" after "header".
[EN] Done
Page 13, 2nd paragraph, 1st sentence: replace both "---" with commas and
space appropriately. Append "field" after "header".
[EN] Done
Page 13, 2nd paragraph, 2nd sentence: drop all of the quotation marks. They
aren't even used consistently in the sentence and don't add much. Change
"rate based" to "rate-based".
[EN] Done
Page 13, 3rd paragraph, 2nd sentence: change "seconds" to "second".
[EN] Done
Page 16, Appendix B: insert "and" before "James".
[EN] Done
Page 16, Eric Noel's address: change "200s Laurel Avenue" to "200 S Laurel
Avenue". Change "Middletown, NJ, 07747" to "Middletown, NJ 07747". Both
changes are made to conform to the USPS standard for address formats.
Aren't you sorry you asked? ;-)
[EN] Done :-)
Page 16, Philip M Williams' name. See change requested on page 1.
[EN] Done