Traffic Ops API/GUI configuration of FQ Pacing plugin #1799

limited · 2018-01-26T20:21:24Z

This activates a Traffic Server Pacing Plugin (apache/trafficserver#3049). Pacing shortens queuing delays in the network and can reduce congestive losses.

This is an optional plugin, with a maximum rate per-Deliveryservice.

Also related is #1942

asfgit · 2018-01-26T20:25:04Z

Can one of the admins verify this patch?

alficles · 2018-01-31T15:03:29Z

traffic_ops/app/lib/Schema/Result/Deliveryservice.pm

@@ -138,7 +138,7 @@ __PACKAGE__->table("deliveryservice");
 =head2 max_dns_answers

  data_type: 'bigint'
-  default_value: 0
+  default_value: 5


Did you mean to be changing the max dns answers here and below?

I'm not intentionally changing it as part of this PR, but the generated schema files currently seem to be out of date for this field. After generating the new schema result, I didn't go through and clean up any of the other changes it made to this file.

Do you think this change is incorrect?

Not necessarily incorrect, but didn't seem to be correct for this particular PR.

alficles · 2018-01-31T15:11:54Z

traffic_ops/app/templates/delivery_service/_form.html.ep

+		<% unless (field('ds.fq_pacing_rate')->valid) { %>
+			<span class="field-with-error"><%= field('ds.fq_pacing_rate')->error %></span>
+		<% } %>
+		%= label_for 'fq_pacing_rate' => 'Max Bytes per Second allowed per session (3000k or 5M are valid entries)', class => 'label'


So, if someone writes 3T, it stores 3000000000 in the DB. And then when someone goes to edit the field a second time, it will just appear as 3000000000, instead of being displayed as 3T? That seems odd, but might be consistent with our other behaviour.

yeah I noticed that in my testing. it is a little weird but didnt seem overly burdensome on an operator.

asfgit · 2018-02-26T22:41:16Z

Can one of the admins verify this patch?

limited · 2018-02-27T13:20:42Z

ok to test

asfgit · 2018-02-27T14:05:22Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/incubator-trafficcontrol-PR/1125/
Test PASSed.

limited · 2018-02-27T15:49:32Z

@alficles thanks for the review chris. any other changes you'd like to see before this gets merged?

alficles · 2018-02-28T04:55:29Z

@limited Nope. I had comments above, but if you're happy with it, it should be fine.

mitchell852 · 2018-02-28T19:09:26Z

@limited - what types of delivery services does this relate to? You should update the Traffic Portal as well.

There are 4 html forms in TP for a delivery service. One for any_map, dns*, http* and steering*. You should add this field to each form where it applies

https://github.com/apache/incubator-trafficcontrol/tree/master/traffic_portal/app/src/common/modules/form/deliveryService

also, you'll need to add an entry for fqpacing to:

https://github.com/apache/incubator-trafficcontrol/blob/master/traffic_portal/app/src/traffic_portal_properties.json#L124

asfgit · 2018-03-01T18:38:35Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/incubator-trafficcontrol-PR/1148/
Test PASSed.

mitchell852

a few suggestions regarding TP changes

mitchell852 · 2018-03-02T04:08:09Z

traffic_portal/app/src/common/modules/form/deliveryService/form.deliveryService.HTTP.tpl.html

@@ -471,6 +471,18 @@
                    </div>
                </div>

+		<div class="form-group" ng-class="{'has-error': hasError(deliveryServiceForm.fqPacingRate), 'has-feedback': hasError(deliveryServiceForm.fqPacingRate)}">
+                    <label class="control-label col-md-2 col-sm-2 col-xs-12">
+                        <span uib-popover-html="label('fqPacingRate', 'desc')" popover-trigger="mouseenter" popover-popup-close-delay="2000" popover-placement="top" popover-append-to-body="true" popover-class="popover-class">{{label('fqPacingRate', 'title')}}</span>


can you remove the

popover-popup-close-delay="2000"

that has been removed from the other form fields

mitchell852 · 2018-03-02T04:08:20Z

traffic_portal/app/src/common/modules/form/deliveryService/form.deliveryService.DNS.tpl.html

@@ -468,6 +468,18 @@
                    </div>
                </div>

+                <div class="form-group" ng-class="{'has-error': hasError(deliveryServiceForm.fqPacingRate), 'has-feedback': hasError(deliveryServiceForm.fqPacingRate)}">
+                    <label class="control-label col-md-2 col-sm-2 col-xs-12">
+                        <span uib-popover-html="label('fqPacingRate', 'desc')" popover-trigger="mouseenter" popover-popup-close-delay="2000" popover-placement="top" popover-append-to-body="true" popover-class="popover-class">{{label('fqPacingRate', 'title')}}</span>


can you remove the

popover-popup-close-delay="2000"

that has been removed from the other form fields

mitchell852 · 2018-03-02T04:12:18Z

traffic_portal/app/src/traffic_portal_properties.json

@@ -199,6 +199,10 @@
            "title": "Global Max TPS",
            "desc": "The maximum transactions this delivery service can serve across all EDGE caches before traffic will be diverted to the bypass destination. For a DNS delivery service, the Bypass Ipv4 or Ipv6 will be used (depending on whether this was a A or AAAA request), and for HTTP delivery services the Bypass FQDN will be used."
          },
+	  "fqPacingRate": {
+	    "title": "Fair Queuing Pacing Rate Bps",
+            "desc": "The maximum bytes per second a cache will delivery on any single TCP connection. This uses the Linux kernel's Fair Queuing setsockopt(SO_MAX_PACING_RATE) to limit the rate of delivery. Traffic exceeding this speed will only be rate-limited and not diverted. This option requires 'net.core.default_qdisc = fq' in /etc/sysctl.conf"


You might want to remove the

"This option requires 'net.core.default_qdisc = fq' in /etc/sysctl.conf"

This description is used in a tooltip in TP and the tooltip is intended for users of the system rather than administrators of the system. That comment is a little too technical for users in my opinion.

removed that sentence

asfgit · 2018-03-02T13:30:38Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/incubator-trafficcontrol-PR/1157/
Test FAILed.

limited · 2018-03-02T14:04:50Z

test this please

asfgit · 2018-03-02T15:06:20Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/incubator-trafficcontrol-PR/1158/
Test FAILed.

mitchell852 · 2018-03-07T20:26:28Z

@limited - looks good to me from a TP perspective

asfgit · 2018-03-07T21:04:51Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/incubator-trafficcontrol-PR/1200/
Test PASSed.

asfgit · 2018-03-07T21:08:58Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/incubator-trafficcontrol-PR/1201/
Test PASSed.

dewrich · 2018-03-07T21:45:48Z

Looks like the test case failed for

Use of uninitialized value in string eq at /Users/dricha209/projects/go/src/github.com/apache/incubator-trafficcontrol/traffic_ops/app/lib/UI/DeliveryService.pm line 208.
t/deliveryservice.t .. 38/?
#   Failed test 'exact match for JSON Pointer "/1/fq_pacing_rate"'
#   at t/deliveryservice.t line 300.
#          got: undef
#     expected: '500000'
# Looks like you failed 1 test of 74.
t/deliveryservice.t .. Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/74 subtests

Test Summary Report
-------------------
t/deliveryservice.t (Wstat: 256 Tests: 74 Failed: 1)
  Failed test:  55
  Non-zero exit status: 1
Files=1, Tests=74,  7 wallclock secs ( 0.03 usr  0.01 sys +  3.15 cusr  0.81 csys =  4.00 CPU)
Result: FAIL```

dewrich · 2018-03-13T18:36:15Z

Ok, looks good all tests pass

limited added this to the 2.3 milestone Jan 26, 2018

limited added new feature A new feature, capability or behavior Traffic Ops related to Traffic Ops Traffic Ops API labels Jan 26, 2018

limited self-assigned this Jan 26, 2018

rob05c added new feature A new feature, capability or behavior and removed new feature A new feature, capability or behavior new feature labels Jan 26, 2018

alficles reviewed Jan 31, 2018

View reviewed changes

limited force-pushed the http_pacing branch from 00fb982 to 619f2e4 Compare February 27, 2018 13:33

limited mentioned this pull request Feb 28, 2018

ORT update for fq_pacing plugin prereqs #1942

Closed

limited force-pushed the http_pacing branch from 619f2e4 to 6ccd070 Compare March 1, 2018 18:31

mitchell852 reviewed Mar 2, 2018

View reviewed changes

limited force-pushed the http_pacing branch from 6ccd070 to 58afb24 Compare March 2, 2018 13:27

limited force-pushed the http_pacing branch from f5d3f2a to c32343a Compare March 7, 2018 21:02

Traffic Ops API/GUI and Traffic Portal configuration of FQ Pacing plugin

3512af8

limited force-pushed the http_pacing branch from c32343a to 3512af8 Compare March 13, 2018 18:37

dewrich merged commit e85eea8 into apache:master Mar 13, 2018

limited deleted the http_pacing branch September 14, 2018 15:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Traffic Ops API/GUI configuration of FQ Pacing plugin #1799

Traffic Ops API/GUI configuration of FQ Pacing plugin #1799

limited commented Jan 26, 2018 •

edited

asfgit commented Jan 26, 2018

alficles Jan 31, 2018

limited Jan 31, 2018

alficles Jan 31, 2018

alficles Jan 31, 2018

limited Jan 31, 2018

asfgit commented Feb 26, 2018

limited commented Feb 27, 2018

asfgit commented Feb 27, 2018

limited commented Feb 27, 2018

alficles commented Feb 28, 2018

mitchell852 commented Feb 28, 2018 •

edited

asfgit commented Mar 1, 2018

mitchell852 left a comment

mitchell852 Mar 2, 2018

limited Mar 2, 2018

mitchell852 Mar 2, 2018

limited Mar 2, 2018

mitchell852 Mar 2, 2018

limited Mar 2, 2018

asfgit commented Mar 2, 2018

limited commented Mar 2, 2018

asfgit commented Mar 2, 2018

mitchell852 commented Mar 7, 2018

asfgit commented Mar 7, 2018

asfgit commented Mar 7, 2018

dewrich commented Mar 7, 2018 •

edited

dewrich commented Mar 13, 2018

Traffic Ops API/GUI configuration of FQ Pacing plugin #1799

Traffic Ops API/GUI configuration of FQ Pacing plugin #1799

Conversation

limited commented Jan 26, 2018 • edited

asfgit commented Jan 26, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

asfgit commented Feb 26, 2018

limited commented Feb 27, 2018

asfgit commented Feb 27, 2018

limited commented Feb 27, 2018

alficles commented Feb 28, 2018

mitchell852 commented Feb 28, 2018 • edited

asfgit commented Mar 1, 2018

mitchell852 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

asfgit commented Mar 2, 2018

limited commented Mar 2, 2018

asfgit commented Mar 2, 2018

mitchell852 commented Mar 7, 2018

asfgit commented Mar 7, 2018

asfgit commented Mar 7, 2018

dewrich commented Mar 7, 2018 • edited

dewrich commented Mar 13, 2018

limited commented Jan 26, 2018 •

edited

mitchell852 commented Feb 28, 2018 •

edited

dewrich commented Mar 7, 2018 •

edited