You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@trafficserver.apache.org by jacksontj <gi...@git.apache.org> on 2016/04/12 07:03:05 UTC
[GitHub] trafficserver pull request: Ts 4341
GitHub user jacksontj opened a pull request:
https://github.com/apache/trafficserver/pull/564
Ts 4341
https://jira01.corp.linkedin.com:8443/browse/TRAFFIC-5688
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/jacksontj/trafficserver TS-4341
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/trafficserver/pull/564.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #564
----
commit 08e2de5abf7095bf41a2a9b052f552ac7077412b
Author: Thomas Jackson <ja...@gmail.com>
Date: 2016-04-11T22:03:16Z
TS-4340 Add tests for per-origin connection limits
commit 78d2bd62e0fe8813ce241d7a474ef67aaf70ad98
Author: Thomas Jackson <ja...@gmail.com>
Date: 2016-04-12T03:02:21Z
TS-4341 add `origin_max_connections_queue`
if you have `origin_max_connections` set, it will limit the number of sessions sent to a given origin. Once the limit is hit ATS would infinitely queue requests waiting for the origin to respond. In the event that the origin is broken (and will stay so) it is better to error the pages, on the flip side if you want ATS to just limit the number of connections you don't want to just close the connections.
To handle both of these I've introduced `origin_max_connections_queue`. This option limits the number of outstanding connections allowed to a given origin. A value <0 defaults to the old behavior (no limits). If set to 0 all requests past the limit are errored, and if the limit is >0 then that many requests are allowed to be outstanding to the origin.
commit 7c1fe5d238e3984437ed704e105894c51ed80cb6
Author: Thomas Jackson <ja...@gmail.com>
Date: 2016-04-12T03:10:44Z
TS-4341 Document origin_max_connections_queue
commit 96d5c900137a20d80d3371deff8db70ca0eaec0d
Author: Thomas Jackson <ja...@gmail.com>
Date: 2016-04-11T22:51:15Z
TS-4342 Add tests for queued/max connections in addition to base tests
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: TS-4341
Posted by jacksontj <gi...@git.apache.org>.
Github user jacksontj commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/564#discussion_r59741127
--- Diff: lib/ts/ink_inet.cc ---
@@ -307,6 +307,26 @@ ats_ip_hash(sockaddr const *addr)
return zret.i;
}
+uint64_t
+ats_ip_port_hash(sockaddr const *addr)
+{
+ union md5sum {
+ uint64_t i;
+ uint16_t b[4];
+ unsigned char c[16];
+ } zret;
+
+ zret.i = 0;
+ if (ats_is_ip4(addr)) {
+ zret.i = (static_cast<uint64_t>(ats_ip4_addr_cast(addr)) << 16) | (ats_ip_port_cast(addr));
+ } else if (ats_is_ip6(addr)) {
+ ink_code_md5(const_cast<uint8_t *>(ats_ip_addr8_cast(addr)), TS_IP6_SIZE, zret.c);
+ // now replace the bottom 16bits so we can account for the port.
+ zret.b[3] = ats_ip_port_cast(addr);
--- End diff --
That being said, it seems to do the same thing in `ats_ip_hash`, which also seems like it wouldn't work :/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: TS-4341
Posted by jacksontj <gi...@git.apache.org>.
Github user jacksontj closed the pull request at:
https://github.com/apache/trafficserver/pull/564
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: TS-4341
Posted by bgaff <gi...@git.apache.org>.
Github user bgaff commented on the pull request:
https://github.com/apache/trafficserver/pull/564#issuecomment-208754987
:eyes: I'm on it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: TS-4341
Posted by jacksontj <gi...@git.apache.org>.
Github user jacksontj commented on the pull request:
https://github.com/apache/trafficserver/pull/564#issuecomment-208718335
This isn't complete yet, still need to add handling for timeouts etc., but I wanted to get the PR opened to get some eyes on it :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: TS-4341
Posted by jacksontj <gi...@git.apache.org>.
Github user jacksontj commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/564#discussion_r59737621
--- Diff: lib/ts/ink_inet.cc ---
@@ -307,6 +307,26 @@ ats_ip_hash(sockaddr const *addr)
return zret.i;
}
+uint64_t
+ats_ip_port_hash(sockaddr const *addr)
+{
+ union md5sum {
+ uint64_t i;
+ uint16_t b[4];
+ unsigned char c[16];
+ } zret;
+
+ zret.i = 0;
+ if (ats_is_ip4(addr)) {
+ zret.i = (static_cast<uint64_t>(ats_ip4_addr_cast(addr)) << 16) | (ats_ip_port_cast(addr));
+ } else if (ats_is_ip6(addr)) {
+ ink_code_md5(const_cast<uint8_t *>(ats_ip_addr8_cast(addr)), TS_IP6_SIZE, zret.c);
+ // now replace the bottom 16bits so we can account for the port.
+ zret.b[3] = ats_ip_port_cast(addr);
--- End diff --
It seems that this returns nothing/garbage in the ipv6 case-- as both lines in ipv6 simply add to `.b` and `.c`, but the method only returns `.i`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: TS-4341
Posted by jpeach <gi...@git.apache.org>.
Github user jpeach commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/564#discussion_r59742958
--- Diff: lib/ts/ink_inet.cc ---
@@ -307,6 +307,26 @@ ats_ip_hash(sockaddr const *addr)
return zret.i;
}
+uint64_t
+ats_ip_port_hash(sockaddr const *addr)
+{
+ union md5sum {
+ uint64_t i;
+ uint16_t b[4];
+ unsigned char c[16];
+ } zret;
+
+ zret.i = 0;
+ if (ats_is_ip4(addr)) {
+ zret.i = (static_cast<uint64_t>(ats_ip4_addr_cast(addr)) << 16) | (ats_ip_port_cast(addr));
+ } else if (ats_is_ip6(addr)) {
+ ink_code_md5(const_cast<uint8_t *>(ats_ip_addr8_cast(addr)), TS_IP6_SIZE, zret.c);
+ // now replace the bottom 16bits so we can account for the port.
+ zret.b[3] = ats_ip_port_cast(addr);
--- End diff --
``zret`` is a union; it's the same data.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: TS-4341
Posted by jacksontj <gi...@git.apache.org>.
Github user jacksontj commented on the pull request:
https://github.com/apache/trafficserver/pull/564#issuecomment-209719516
This PR now includes the patches from https://github.com/apache/trafficserver/pull/562 and https://github.com/apache/trafficserver/pull/569 as they are related (and interdependent)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: TS-4341
Posted by bgaff <gi...@git.apache.org>.
Github user bgaff commented on the pull request:
https://github.com/apache/trafficserver/pull/564#issuecomment-210074340
@jacksontj I'm going to merge my pull request, can you please update yours once my code lands? Thanks
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: TS-4341
Posted by jacksontj <gi...@git.apache.org>.
Github user jacksontj commented on the pull request:
https://github.com/apache/trafficserver/pull/564#issuecomment-209001031
Well, it seems that this doesn't leak in the queue-- but I did find another bug (https://issues.apache.org/jira/browse/TS-4343) which we'll probably want to fix at the same time as this :/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: TS-4341
Posted by jacksontj <gi...@git.apache.org>.
Github user jacksontj commented on the pull request:
https://github.com/apache/trafficserver/pull/564#issuecomment-209719690
@bgaff @jpeach @zwoop ready for review :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] trafficserver pull request: TS-4341
Posted by jacksontj <gi...@git.apache.org>.
Github user jacksontj commented on a diff in the pull request:
https://github.com/apache/trafficserver/pull/564#discussion_r59743447
--- Diff: lib/ts/ink_inet.cc ---
@@ -307,6 +307,26 @@ ats_ip_hash(sockaddr const *addr)
return zret.i;
}
+uint64_t
+ats_ip_port_hash(sockaddr const *addr)
+{
+ union md5sum {
+ uint64_t i;
+ uint16_t b[4];
+ unsigned char c[16];
+ } zret;
+
+ zret.i = 0;
+ if (ats_is_ip4(addr)) {
+ zret.i = (static_cast<uint64_t>(ats_ip4_addr_cast(addr)) << 16) | (ats_ip_port_cast(addr));
+ } else if (ats_is_ip6(addr)) {
+ ink_code_md5(const_cast<uint8_t *>(ats_ip_addr8_cast(addr)), TS_IP6_SIZE, zret.c);
+ // now replace the bottom 16bits so we can account for the port.
+ zret.b[3] = ats_ip_port_cast(addr);
--- End diff --
@jpeach thanks for calling out my stupid, for whatever reason I read it as struct ;)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---