Skip to content

TS-4341#564

Merged
jacksontj merged 6 commits intoapache:masterfrom
jacksontj:TS-4341
Apr 14, 2016
Merged

TS-4341#564
jacksontj merged 6 commits intoapache:masterfrom
jacksontj:TS-4341

Conversation

@jacksontj
Copy link
Contributor

TS-4341

jacksontj added a commit to jacksontj/trafficserver that referenced this pull request Apr 12, 2016
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.

This closes apache#564
@jacksontj jacksontj changed the title Ts 4341 TS-4341 Apr 12, 2016
@jacksontj
Copy link
Contributor Author

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 :)

@bgaff
Copy link
Member

bgaff commented Apr 12, 2016

👀 I'm on it.

jacksontj added a commit to jacksontj/trafficserver that referenced this pull request Apr 12, 2016
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.

This closes apache#564
@jacksontj
Copy link
Contributor Author

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 :/

@jacksontj jacksontj force-pushed the TS-4341 branch 2 times, most recently from c68064c to 4b9c30f Compare April 14, 2016 01:52
jacksontj added a commit to jacksontj/trafficserver that referenced this pull request Apr 14, 2016
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.

This closes apache#564
jacksontj added a commit to jacksontj/trafficserver that referenced this pull request Apr 14, 2016
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.

This closes apache#564
@jacksontj
Copy link
Contributor Author

This PR now includes the patches from #562 and #569 as they are related (and interdependent)

@jacksontj
Copy link
Contributor Author

@bgaff @jpeach @zwoop ready for review :)

} 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);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That being said, it seems to do the same thing in ats_ip_hash, which also seems like it wouldn't work :/

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

zret is a union; it's the same data.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jpeach thanks for calling out my stupid, for whatever reason I read it as struct ;) So nevermind then-- we are all set :)

@bgaff
Copy link
Member

bgaff commented Apr 14, 2016

@jacksontj I'm going to merge my pull request, can you please update yours once my code lands? Thanks

jacksontj added a commit to jacksontj/trafficserver that referenced this pull request Apr 14, 2016
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.

This closes apache#564
bgaff and others added 6 commits April 14, 2016 10:52
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.

This closes apache#564
@jacksontj jacksontj merged commit 972bd4d into apache:master Apr 14, 2016
shinrich added a commit to shinrich/trafficserver that referenced this pull request Jan 9, 2018
Cherrypick: Add SINK_TRANSFORM option to CPPAPI
@zwoop zwoop added this to the Old milestone Jan 8, 2019
masaori335 pushed a commit to masaori335/trafficserver that referenced this pull request Jul 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants