Subject: Re: [Dime] New version of DOIC rate draft

From: Steve Donovan <[email-masked]>
Date: Wed, 3 Oct 2018 13:18:40 -0500

Eric,

Thanks for your feedback. 

My responses are inline.

I will submit a new version of the draft with these changes the
previously made changes shortly.

Steve

On 10/1/18 2:20 PM, NOEL, ERIC C wrote:
>
> Hi Steve,
>
>  
>
> Here are my comments/answers:
>
>  
>
> §7.3: "In situations where reacting nodes are configured with some knowledge
>    about the reporting node (e.g., operator pre-provisioning), it can be
>    beneficial to choose a value of TAU based on how many reacting nodes
>    will be sending requests to the reporting node.”
>  
> I previously commented that this requires knowledge of other traffic sources, not just the reporting node. I did not see a response.
>
> SRD> I don't understand the issue.  I'm hoping Eric can comment or you
> can clarify.
> *EN> Proposing to change "In situations where reacting nodes are
> configured with some knowledge about the reporting node (e.g.,
> operator pre-provisioning), ….” to "In situations where reacting nodes
> are configured with some knowledge about the reporting node and other
> traffic sources (e.g., operator pre-provisioning), …”*
>
SRD> Done.
>
> **
>
>  
>
> §7.3.1: I found the text hard to follow. It would help to declare all the identifiers and initialization up front, and to present things in more of a stepwise fashion.
>
> *EN> This paragraph is the result of a compromise between multiple
> [RFC7415] reviewers on how to describe the rate based algorithm. The
> pseudo-code for the algorithm includes a summary description of all
> the variables used in the algorithm. Proposing to move the pseudo code
> at the beginning of the section (for both 7.3.1 & 7.3.2).*
>
SRD> Done.
>
>
>  
> - T is effectively a time interval, right? It would help to say that, especially later when you subtract a different time interval from it.
>
> SRD> I'll leave this for Eric to handle.
> *EN> Yes, T = 1/[OC-Maximum-Rate] or “ inter-Diameter request
> interval” or “inter-transmission interval”.*
>
SRD> I'm assuming that moving the pseudo-code addresses this issue as it
clearly states that T is the inter-transmission interval.
>
>
> **
>
> §7.3.2:
>  
> - “ Request candidates for reduction, requests not subject to reduction (except under extenuating circumstances when there aren’t any messages in the first category that can be reduced).”:
>  
> That seems like an awkward way to say that the second category is the set of requests that is only subject to reduction if there are no messages left in the first category.
>  
> - “ This can be generalized to n priorities using n thresholds for n>2 in the obvious way.”: I suggest you refrain from calling it “obvious”.
>  *EN> Proposing to change “ This can be generalized to n priorities
> using n thresholds for n>2 in the obvious way.” to  “ This can be
> generalized to n priorities using n thresholds for n>2.”*
SRD> Done
> **
>  
> §7.3.3: Paragraph starting with “ Then (only) if the arrival is admitted, increase the bucket by an amount…”: I think you increase the bucket _count_, right?
>
> SRD> I'll leave these for Eric to handle.
>
> *EN>* *This implementation of the Leaky Bucket algorithm relies on a bucket
> of real-valued content.  Proposing to change “Then (only) if the
> arrival is admitted, increase the bucket by an amount…” into “Then
> (only) if the arrival is admitted, increase the bucket content by an
> amount…”*
SRD> Done
>
>  
>
>  
>
> Thanks,
>
>  
>
> Eric Noel
>
> *AT&T Labs, Inc.*
> /Rethink Possible/
>
>  
>
> Optimization, Reliability and Customer Analytics
>
> 200 South Laurel Avenue, D5-3C38
> Middletown, NJ 07748
> P: 732.420.4174
>
> [email-masked] <mailto:[email-masked]>
>
>  
>
> *From:*Steve Donovan [mailto:[email-masked]]
> *Sent:* Wednesday, September 26, 2018 11:47 AM
> *To:* NOEL, ERIC C <[email-masked]>
> *Subject:* Re: [Dime] New version of DOIC rate draft
>
>  
>
> Thanks
>
> On 9/26/18 10:37 AM, NOEL, ERIC C wrote:
>
>     Hi Steve,
>
>      
>
>     Will take a look shortly (meant to do it last week but got side
>     tracked).
>
>     Thanks,
>
>      
>
>     Eric Noel
>
>     *AT&T Labs, Inc.*
>     /Rethink Possible/
>
>      
>
>     Optimization, Reliability and Customer Analytics
>
>     200 South Laurel Avenue, D5-3C38
>     Middletown, NJ 07748
>     P: 732.420.4174
>
>     [email-masked] <mailto:[email-masked]>
>
>      
>
>     *From:*Steve Donovan [mailto:[email-masked]]
>     *Sent:* Wednesday, September 26, 2018 10:26 AM
>     *To:* NOEL, ERIC C <[email-masked]> <mailto:[email-masked]>
>     *Subject:* Fwd: Re: [Dime] New version of DOIC rate draft
>
>      
>
>     Eric,
>
>     Let me know if you won't be able to address the questions I left
>     for you in Ben's comments on the Diameter rate draft.
>
>     Thanks,
>
>     Steve
>
>
>
>     -------- Forwarded Message --------
>
>     *Subject: *
>
>     	
>
>     Re: [Dime] New version of DOIC rate draft
>
>     *Date: *
>
>     	
>
>     Fri, 21 Sep 2018 11:58:21 -0500
>
>     *From: *
>
>     	
>
>     Steve Donovan <[email-masked]>
>     <mailto:[email-masked]>
>
>     *To: *
>
>     	
>
>     [email-masked] <mailto:[email-masked]>, Eric Noel
>     <[email-masked]> <mailto:[email-masked]>
>
>
>
>     I've dealt with the majority of the remaining issues in this email.
>
>     There are a few questions that deal with section 7 that I have
>     left for Eric to address.
>
>     Steve
>
>     On 9/15/18 6:56 PM, Ben Campbell wrote:
>
>         Hi, thanks for posting the update; I think it is making progress. However,  I still have some comments I would like to address before IETF LC:
>
>          
>
>         Thanks!
>
>          
>
>         Ben.
>
>          
>
>         Substantive Comments:
>
>          
>
>         - General: I still think more discussion is needed about allocating rate to multiple input sources. I get that the actual allocation is a matter of local policy, but there’s still implications that need discussion. I’m not sure I got my concern across in previous discussion, so here’s another attempt:
>
>          
>
>         The issue I think needs elaboration on is how the offered load varies with the number of sources times the (average) rate per each source. That is, if the number of sources changes, the reacting node may need to change the rate limits assigned to each existing source.
>
>          
>
>         As a hypothetical example, lets assume a reporting node wants to limit its entire offered load to 1000 tps. Further assume it has 10 active reacting nodes (all supporting the rate algorithm). Local policy is to allocate the rate limit equally across sources. So it sends an OLR to each of those clients to give it a rate limit of 100 tps. Now, if another 10 reacting nodes become active, it needs to reallocate the load across all 20, giving each a limit of 50 tps. Now, if some of those reacting nodes go off-line, or simply reduce their activity beneath the limit for an extended period of time, the reporting node may need to increase the allocation to the remaining nodes.
>
>          
>
>         This is a fairly fundamental difference between rate and load; rate uses absolute numbers while load uses percentages.
>
>     SRD> See previous email.
>
>
>         §5.1:
>
>          
>
>         - The text is still not clear when a reporting node must send the first OLR. I understand that the choice of when to set a rate limit is local policy, but some of the text in this section suggests to me that you expect a reporting node to send an OLR immediately when it selects the rate algorithm for a specific reacting node.
>
>          
>
>         For example, paragraph 6 says:
>
>          
>
>         "A reporting node that has selected the rate overload abatement algorithm MUST indicate the rate requested to be applied by DOIC reacting nodes in the OC-Maximum-Rate AVP included in the OC-OLR AVP."
>
>          
>
>         This needs to talk about _when_ it must do that. Without some comment, it seems like it means “when it selects the algorithm” (i.e. “when you indicate support” ). That seems to conflict with the idea of this being local policy.
>
>     SRD> Well, it must indicate the rate anytime an OLR is sent.  The
>     when is specified in RFC7683, Section 5.2.3.  Here's part of that
>     section (paragraph 1) included to hopefully illustrate that no
>     additional wording is needed in the rate draft:
>
>        If there is an active OCS entry, then a reporting node SHOULD
>     include
>        the OC-OLR AVP in all answers to requests that contain the
>        OC-Supported-Features AVP and that match the active OCS entry.
>
>
>          
>
>         - new 5th paragraph: Why SHOULD instead of MUST? Is there a situation where you have an OCS but no allocated rate? (e.g. you’ve selected the rate algorithm for the reacting node, but have not send an OLR?)
>
>     SRD> There is a possible implementation that assumes the rate for
>     each reacting node is calculated by dividing the number of active
>     nodes by the maximum rate.  With this implementation the OCS would
>     not require storing the rate.  I did see an error in that
>     paragraph, and as such, I've changed that paragraph to the following:
>
>     The rate OCS entery SHOULD include the rate allocated to the
>     reacting note.
>
>     Old text:
>
>        The rate OCS entery SHOULD include the rate allocated to each
>     reacting note.
>
>     New text:
>
>        The rate OCS entery SHOULD include the rate allocated to the
>     reacting note.
>
>
>
>          
>
>         §5.4, paragraph 1:
>
>          
>
>         Discussion indicated that the intent of this paragraph was the reacting node keeps OCS for each server than indicated support for the rate algorithm. But I don’t see text that says when the reacting note needs to actually create the state entry. I think the answer is “immediately when a reporting node indicates support”, right?
>
>     SRD> This is covered by the following paragraph from RFC7683:
>
>        If the received OLR is for a new overload condition, then a
>     reacting
>        node MUST generate a new OCS entry for the overload condition.
>
>
>          
>
>         §5.6, first two paragraphs:
>
>          
>
>         I still think the text talking about using different algorithms needs to say something normative about the characteristics of those algorithms. Janet’s comments indicated the normative text is in §7.3.1. But that’s part of the algorithm that they MAY use, so it would not be constraining against other algorithm choices. Perhaps the 2nd paragraph should be normative?
>
>     SRD> How about the following change in paragraph 2:
>
>     Old text:
>
>           Note: Other algorithms for controlling the rate can be
>     implemented
>           by the reacting node as long as they result in the correct
>     rate of
>           traffic being sent to the reporting node.
>
>     New text:
>
>        Other algorithms for controlling the rate MAY be implemented
>        by the reacting node.  Any algorithm implemented MUST result
>        in the correct rate of traffic being sent to the reporting node.
>
>
>          
>
>         §7.2: “ But the resulting request rate presented to the overloaded reporting node will converge towards the target Diameter request rate.”
>
>          
>
>         Wasn’t there discussion to change this to “... the target Diameter request rate or a lower rate”?
>
>     SRD> Agreed, change made.
>
>
>          
>
>         §7.3: "In situations where reacting nodes are configured with some knowledge
>
>            about the reporting node (e.g., operator pre-provisioning), it can be
>
>            beneficial to choose a value of TAU based on how many reacting nodes
>
>            will be sending requests to the reporting node.”
>
>          
>
>         I previously commented that this requires knowledge of other traffic sources, not just the reporting node. I did not see a response.
>
>     SRD> I don't understand the issue.  I'm hoping Eric can comment or
>     you can clarify.
>
>
>          
>
>         Editorial Comments:
>
>          
>
>         [I note a number of editorial comments that fell into the “made changes unless indicated otherwise” category did not seem to get changed. I included those here again.]
>
>     SRD> My apologies if I missed these in the last refresh.
>
>
>          
>
>         - There are still things reported by IDNits that need checking. Some are obviously noise, but some appear to be real. (Line length, references in abstract, and references to RFC 5226)
>
>     SRD> There were no issues reported by IDNits when I submitted the
>     document.  Is there a different IDnits check than the one used
>     when submitting drafts?
>
>
>          
>
>         - Was there a reason not to use the RFC 8174 boilerplate in the “Requirements” section? (I thought you had intended to do so.)
>
>     SRD> Yes, this slipped through the cracks.  Change made.
>
>
>          
>
>         §1:
>
>         - first paragraph: There were some editorial fixes that I thought were agreed that did not appear in the new version:
>
>         s/“protect the stability”/“ensure the stability”
>
>     SRD> Change made
>
>
>         s/“subjected with”/“subjected to”
>
>     SRD> Change made
>
>
>         (new comment): In the new last sentence, is there a reason for the all-caps? That’s normally reserved for normative keywords.
>
>     SRD> I don't remember.  I've changed it to all lower.
>
>
>          
>
>         §4:
>
>         - first paragraph: Please consider active voice in the last sentence,
>
>     SRD> Old text:
>
>        This document defines the rate abatement algorithm (referred to as
>        rate in this document) feature.  Support for the rate feature by a
>        DOIC node will be indicated by a new value of the OC-Feature-Vector
>        AVP, as described in Section 6.1.1, per the rules defined in
>        [RFC7683].
>
>     New Text:
>
>        This document defines the rate abatement algorithm (referred to as
>        rate in this document) feature.  A DOIC node indicates support
>     for the
>        rate feature by indicating a new value of the OC-Feature-Vector
>        AVP, as described in Section 6.1.1, per the rules defined in
>        [RFC7683].
>
>
>          
>
>         §5.1:
>
>         - New 5th paragraph:
>
>         s/entery/entry
>
>     SRD> Change made.
>
>
>          
>
>         §7.1, 2nd paragraph: “ signal one another support for rate-based overload
>
>           control”: This seems awkward; are there missing words? Perhaps there should be something like “their” or “that they” between “another” and “support”?
>
>     SRD> Changed from:
>
>        Following the procedures defined in [RFC7683], the reacting
>     node and
>        reporting node signal one another support for rate-based overload
>        control.
>
>     To:
>
>        Following the procedures defined in [RFC7683], the reacting
>     node and
>        reporting node signal their support for rate-based overload
>        control.
>
>
>          
>
>         §7.2, last two paragraphs: The MUSTs do not seem necessary. 2119 keywords should be used when there is some sort of choice or room for error. You don’t need them to define the basic operation of the protocol.
>
>     SRD> I'm okay with removing the MUSTs.  Here the proposed change:
>
>     Old:
>
>        Upon detection of overload, and the determination to invoke
>     overload
>        controls, the reporting node MUST follow the specifications in
>        [RFC7683] to notify its clients of the allocated target maximum
>        Diameter request rate and to notify them that the rate overload
>        abatement is in effect.
>
>        The reporting node MUST use the OC-Maximum-Rate AVP defined in this
>        specification to communicate a target maximum Diameter request rate
>        to each of its clients.
>
>     New:
>
>        Upon detection of overload, and the determination to invoke
>     overload
>        controls, the reporting node follows the specifications in
>        [RFC7683] to notify its clients of the allocated target maximum
>        Diameter request rate and to notify them that the rate overload
>        abatement is in effect.
>
>        The reporting node uses the OC-Maximum-Rate AVP defined in this
>        specification to communicate a target maximum Diameter request rate
>        to each of its clients.
>
>
>          
>
>         §7.3.1: I found the text hard to follow. It would help to declare all the identifiers and initialization up front, and to present things in more of a stepwise fashion.
>
>          
>
>         - T is effectively a time interval, right? It would help to say that, especially later when you subtract a different time interval from it.
>
>     SRD> I'll leave this for Eric to handle.
>
>
>          
>
>         - paragraph 9: Should “admit” be “emit”?
>
>          
>
>         - the example code has several mentions of SIP requests.
>
>     SRD> These have been changed to Diameter requests
>
>
>          
>
>         §7.3.2:
>
>          
>
>         - “ Request candidates for reduction, requests not subject to reduction (except under extenuating circumstances when there aren’t any messages in the first category that can be reduced).”:
>
>          
>
>         That seems like an awkward way to say that the second category is the set of requests that is only subject to reduction if there are no messages left in the first category.
>
>          
>
>         - “ This can be generalized to n priorities using n thresholds for n>2 in the obvious way.”: I suggest you refrain from calling it “obvious”.
>
>          
>
>         §7.3.3: Paragraph starting with “ Then (only) if the arrival is admitted, increase the bucket by an amount…”: I think you increase the bucket _count_, right?
>
>     SRD> I'll leave these for Eric to handle.
>
>          
>
>          
>
>             On Sep 10, 2018, at 3:44 PM, Steve Donovan <[email-masked]> <mailto:[email-masked]> wrote:
>
>              
>
>             I've posted a new version of the rate draft.
>
>              
>
>             I've attached the diff file.
>
>              
>
>             Regards,
>
>              
>
>             Steve
>
>             <Diff  draft-ietf-dime-doic-rate-control-08.txt - draft-ietf-dime-doic-rate-control-09.txt.html>_______________________________________________
>
>             DiME mailing list
>
>             [email-masked] <mailto:[email-masked]>
>
>             https://www.ietf.org/mailman/listinfo/dime
>             <https://urldefense.proofpoint.com/v2/url?u=https-3A__www.ietf.org_mailman_listinfo_dime&d=DwMD-g&c=LFYZ-o9_HUMeMTSQicvjIg&r=DG4T9S-ISU0PvDG75Qp85lKMh2wcMjVfostFTQgiEWw&m=QXNRcVB0dk1yGN4yJHl2xQ25-XfhagPv2Sv1PAFho6E&s=65iSdtPNp6kL-EiO1xTF6ZiyVYHIS57ay4lemiaF9F0&e=>
>
>
>
>
>
>         _______________________________________________
>
>         DiME mailing list
>
>         [email-masked] <mailto:[email-masked]>
>
>         https://www.ietf.org/mailman/listinfo/dime
>         <https://urldefense.proofpoint.com/v2/url?u=https-3A__www.ietf.org_mailman_listinfo_dime&d=DwMD-g&c=LFYZ-o9_HUMeMTSQicvjIg&r=DG4T9S-ISU0PvDG75Qp85lKMh2wcMjVfostFTQgiEWw&m=QXNRcVB0dk1yGN4yJHl2xQ25-XfhagPv2Sv1PAFho6E&s=65iSdtPNp6kL-EiO1xTF6ZiyVYHIS57ay4lemiaF9F0&e=>
>
>      
>
>  
>