Enforce Active Connection limits

[sudheerv]

1. Throttle connections when there's no room in active conn queue
2. Adjust manage_active_queue() to not fail when the conn is already in active queue
3. Return true for PluginVC (dummy connection) add_to_active_queue
4. Metrics for throttling

The idea is to tune active connections that can be handled (based on the available resources/capacity (CPU, Memory, Network bandwidth) and minimize/remove dependency on having to tune connection timeouts (active/inactive/keep-alive etc) which are very hard to tune.

The primary goal is to limit the max active connections allowed at any given instant. The resource requirement for an idle/inactive vs active connections are completely different - For e.g an idle connection really only consumes memory resource, while an active connection consumes CPU, network besides memory. And allowing to tune/cap the max active connections based on the deployed capacity for the resources available, would make timeout tuning almost redundant and no op. Otherwise, we'd have to tune the timeouts to estimate throughput which is very hard (it's hard to justify how large or small we want the active timeout to be or keep alive timeout to be. For e.g in a non-peak hour, we could let the active timeout be much higher than the default, while during peak hour, we'd want to limit it to ensure we are not starving resources on one connection).

Note: there's one little TODO item here. PluginVC's connections are not tracked in the NetVC's active queue because these are bogus internal connections. However, for some users, internal traffic is significant (e.g sideways calls, or background fetches etc) and does consume plenty of resources. Since these internal requests don't impact ingress network, and have a slightly different resource requirements than client originated requests, it might be better to track these using a separate config for internal requests. Will follow that part up with a separate PR.

[vmamidi]

Why do you want to close the connection immediately without any retries? Also, Why is the throttling not done while accepting the connection?

[sudheerv]

Why do you want to close the connection immediately without any retries? Also, Why is the throttling not done while accepting the connection?

Yeah, so the difference here is that this is active connection limiting - the total connections are still under the overall connection limit and we only want to throttle when the new connection becomes "active" (ie sends a request). And, the "active" connection queue can change between the time a new connection is accepted and when the client actually sends request over the connection.

"retries" is a good question - What sort of retry did you have in mind? Given that we want to limit the max active connections we allow into the system, and the client has already sent the request, retrying at that point can only be limited to retrying the add operation to the queue. Not sure if it's really worth it. Thoughts?

Why do you want to close the connection immediately without any retries? Also, Why is the throttling not done while accepting the connection?

Yeah, so the difference here is that this is active connection limiting - the total connections are still under the overall connection limit and we only want to throttle when the new connection becomes "active" (ie sends a request). And, the "active" connection queue can change between the time a new connection is accepted and when the client actually sends request over the connection.

Does this mean the connection is going to reset as the client might have already sent the data?

"retries" is a good question - What sort of retry did you have in mind? Given that we want to limit the max active connections we allow into the system, and the client has already sent the request, retrying at that point can only be limited to retrying the add operation to the queue. Not sure if it's really worth it. Thoughts?

I was thinking that active connections are checked at the time of accepting the connection. Once the connection is accepted, we try a configured number of times to add to the queue until a configured timeout. I do not think we are doing that here.

Also, your change is only handling H1 connections. Is this intentional?

Why do you want to close the connection immediately without any retries? Also, Why is the throttling not done while accepting the connection?

Yeah, so the difference here is that this is active connection limiting - the total connections are still under the overall connection limit and we only want to throttle when the new connection becomes "active" (ie sends a request). And, the "active" connection queue can change between the time a new connection is accepted and when the client actually sends request over the connection.

Does this mean the connection is going to reset as the client might have already sent the data?

Yeah, that's correct. That is the only way we know the connection is "active", otherwise browsers tend to make connections ahead of time without sending bytes.

"retries" is a good question - What sort of retry did you have in mind? Given that we want to limit the max active connections we allow into the system, and the client has already sent the request, retrying at that point can only be limited to retrying the add operation to the queue. Not sure if it's really worth it. Thoughts?

I was thinking that active connections are checked at the time of accepting the connection. Once the connection is accepted, we try a configured number of times to add to the queue until a configured timeout. I do not think we are doing that here.

Also, your change is only handling H1 connections. Is this intentional?

The diff also has the change for h/2 in Http2ConnectionState.cc.

Why do you want to close the connection immediately without any retries? Also, Why is the throttling not done while accepting the connection?

Yeah, so the difference here is that this is active connection limiting - the total connections are still under the overall connection limit and we only want to throttle when the new connection becomes "active" (ie sends a request). And, the "active" connection queue can change between the time a new connection is accepted and when the client actually sends request over the connection.

Does this mean the connection is going to reset as the client might have already sent the data?

Yeah, that's correct. That is the only way we know the connection is "active", otherwise browsers tend to make connections ahead of time without sending bytes.

"retries" is a good question - What sort of retry did you have in mind? Given that we want to limit the max active connections we allow into the system, and the client has already sent the request, retrying at that point can only be limited to retrying the add operation to the queue. Not sure if it's really worth it. Thoughts?

I was thinking that active connections are checked at the time of accepting the connection. Once the connection is accepted, we try a configured number of times to add to the queue until a configured timeout. I do not think we are doing that here.
Also, your change is only handling H1 connections. Is this intentional?

The diff also has the change for h/2 in Http2ConnectionState.cc.
Do we consider one H2 stream as one connection?

No, but, I've modified add_to_active_queue() to only fail for a "new" connection - if the connection is already in the queue, it doesn't fail. So, the change will only throttle a new h/2 connection, but not new streams on an already accepted connection (we've max stream concurrency setting for that)

Why do you want to close the connection immediately without any retries? Also, Why is the throttling not done while accepting the connection?

Yeah, so the difference here is that this is active connection limiting - the total connections are still under the overall connection limit and we only want to throttle when the new connection becomes "active" (ie sends a request). And, the "active" connection queue can change between the time a new connection is accepted and when the client actually sends request over the connection.

Does this mean the connection is going to reset as the client might have already sent the data?

Yeah, that's correct. That is the only way we know the connection is "active", otherwise browsers tend to make connections ahead of time without sending bytes.

That means we accept new connections until the idle connections limit even if the active_queue is full and close the connection when it becomes active.

"retries" is a good question - What sort of retry did you have in mind? Given that we want to limit the max active connections we allow into the system, and the client has already sent the request, retrying at that point can only be limited to retrying the add operation to the queue. Not sure if it's really worth it. Thoughts?

I was thinking that active connections are checked at the time of accepting the connection. Once the connection is accepted, we try a configured number of times to add to the queue until a configured timeout. I do not think we are doing that here.
Also, your change is only handling H1 connections. Is this intentional?

The diff also has the change for h/2 in Http2ConnectionState.cc.

Originally I thought this would be a problem because of multiplexing is HTTP/2 the session may already be on the active queue, but looking at add_to_active_queue, it already deals with that case.

Of course in the case of HTTP/2 it may be better if add_to_active queue does not call do_io_close and instead let the error handling in the HTTP/2 logic send a useful error and close the connection.

And looking again, that is exactly what the logic does, so this looks good.

@shinrich

Yeah, I tweaked add_to_active_queue() to handle the case where the session is already inside the active queue just for the HTTP/2 scenario.

One thing I'm wondering about was whether the throttling should be done with HTTP/2 error HTTP2_ERROR_REFUSED_STREAM instead of a generic "internal error". HTTP2_ERROR_REFUSED_STREAM will allow the client to retry the request safely. Thoughts?

HTTP2_ERROR_REFUSED_STREAM (with HTTP2_ERROR_CLASS_STREAM) would make more sense. It might cause rapid retries, but in that case the session would be closed with ENHANCE_YOUR_CALM eventually because of an excessive error rate.

If we don't want retries, I'd use NO_ERROR instead because it's a managed situation but not something unexpected (e.g. memory allocation failure).

Yeah, that's exactly my thought as well. If one or few boxes in a cluster are overloaded, a retry from the client is desirable (as it may then end up on a new box). On the other hand, if the entire cluster is overloaded, we don't want to create a retry storm and make matters worse, by allowing a fast retry. I'll change it to NO_ERROR as this is intended/designed failure.

We cannot use HTTP2_ERROR_CLASS_NONE. It's something like null. Valid options to close a connection/stream are HTTP2_ERROR_CLASS_CONNECTION or HTTP2_ERROR_CLASS_STREAM.

For this case, these two are possible options, IMO.
HTTP2_ERROR_CLASS_CONNECTION + NO_ERROR : Just close the connection
HTTP2_ERROR_CLASS_STREAM + REFUSED_STREAM : Close the stream but allows a client to retry on the same connection

NO_ERROR does not ensure that clients don't retry. If we just close a connection with HTTP2_ERROR_CLASS_CONNECTION + NO_ERROR, client may reconnect to a server because the client don't now why the connection was closed. It's basically the same as HTTP/1.1 behavior.

Reasons I suggested REFUSED_STREAM are 1) it's relatively lighter than retry on a new connection, and 2) it would end up with ENHANCE_YOUR_CALM, which suggests the behavior is generating excessive load, if retries seem like making things worse. There is still no guarantee but I hope the error code ceases retries.

Another option is retuning 503, (and then HTTP2_ERROR_CLASS_CONNECTION + NO_ERROR). Most clients would not retry soon if they received it.

That’s not correct. Most clients will retry as soon as they see a 503 including automatically at the Network layer. Infact, 502/503 are considered the safest status codes for the clients to automatically retry.

I’ll just change it back to return (connection class + no error). I don’t think this is a big deal and the only way to prevent a retry would be If there was a custom protocol between server and client.

It's a bummer if clients ignore Retry-After header sent with 503.

Looks good. While you are in there, it would be nice to put in documentation for the proxy.config controls for this feature.

Why do you want to close the connection immediately without any retries? Also, Why is the throttling not done while accepting the connection?

Yeah, so the difference here is that this is active connection limiting - the total connections are still under the overall connection limit and we only want to throttle when the new connection becomes "active" (ie sends a request). And, the "active" connection queue can change between the time a new connection is accepted and when the client actually sends request over the connection.

Does this mean the connection is going to reset as the client might have already sent the data?

Yeah, that's correct. That is the only way we know the connection is "active", otherwise browsers tend to make connections ahead of time without sending bytes.

"retries" is a good question - What sort of retry did you have in mind? Given that we want to limit the max active connections we allow into the system, and the client has already sent the request, retrying at that point can only be limited to retrying the add operation to the queue. Not sure if it's really worth it. Thoughts?

I was thinking that active connections are checked at the time of accepting the connection. Once the connection is accepted, we try a configured number of times to add to the queue until a configured timeout. I do not think we are doing that here.
Also, your change is only handling H1 connections. Is this intentional?

The diff also has the change for h/2 in Http2ConnectionState.cc.
Do we consider one H2 stream as one connection?

No, but, I've modified add_to_active_queue() to only fail for a "new" connection - if the connection is already in the queue, it doesn't fail. So, the change will only throttle a new h/2 connection, but not new streams on an already accepted connection (we've max stream concurrency setting for that)

That means theoretically, we may have 100x requests in H2?
number of h1 requests for h2 clients= proxy.config.http2.max_concurrent_streams_in * proxy.process.net.max.active.connections_throttled_in http1.
By default, proxy.config.http2.max_concurrent_streams_in is 100.

Why do you want to close the connection immediately without any retries? Also, Why is the throttling not done while accepting the connection?

Yeah, so the difference here is that this is active connection limiting - the total connections are still under the overall connection limit and we only want to throttle when the new connection becomes "active" (ie sends a request). And, the "active" connection queue can change between the time a new connection is accepted and when the client actually sends request over the connection.

Does this mean the connection is going to reset as the client might have already sent the data?

Yeah, that's correct. That is the only way we know the connection is "active", otherwise browsers tend to make connections ahead of time without sending bytes.

That means we accept new connections until the idle connections limit even if the active_queue is full and close the connection when it becomes active.

Yeah, it sounds a bit confusing :) - but there's no direct idle connection limit. There's just the active connection limit and the rest is idle connections bucket. It does sound a bit odd that we throttle new connections, while we've idle connections sitting around, but, I think the key is that these two are not identical. Active connections are far more demanding in terms of resource consumption while idle connections are cheaper (just memory). You could argue that, we drop the idle connections and accept new connections in their place. But, the problem is active connections require other resources besides memory that can be reaped by dropping idle connections. The idea is that, active conns are tuned based on network and CPU resources and some memory is left aside for idle connections, because it has its own advantages in minimizing round trips (TCP/TLS).

"retries" is a good question - What sort of retry did you have in mind? Given that we want to limit the max active connections we allow into the system, and the client has already sent the request, retrying at that point can only be limited to retrying the add operation to the queue. Not sure if it's really worth it. Thoughts?

I was thinking that active connections are checked at the time of accepting the connection. Once the connection is accepted, we try a configured number of times to add to the queue until a configured timeout. I do not think we are doing that here.
Also, your change is only handling H1 connections. Is this intentional?

The diff also has the change for h/2 in Http2ConnectionState.cc.

Why do you want to close the connection immediately without any retries? Also, Why is the throttling not done while accepting the connection?

Yeah, so the difference here is that this is active connection limiting - the total connections are still under the overall connection limit and we only want to throttle when the new connection becomes "active" (ie sends a request). And, the "active" connection queue can change between the time a new connection is accepted and when the client actually sends request over the connection.

Does this mean the connection is going to reset as the client might have already sent the data?

Yeah, that's correct. That is the only way we know the connection is "active", otherwise browsers tend to make connections ahead of time without sending bytes.

That means we accept new connections until the idle connections limit even if the active_queue is full and close the connection when it becomes active.

Yeah, it sounds a bit confusing :) - but there's no direct idle connection limit. There's just the active connection limit and the rest is idle connections bucket. It does sound a bit odd that we throttle new connections, while we've idle connections sitting around, but, I think the key is that these two are not identical. Active connections are far more demanding in terms of resource consumption while idle connections are cheaper (just memory). You could argue that, we drop the idle connections and accept new connections in their place. But, the problem is active connections require other resources besides memory that can be reaped by dropping idle connections. The idea is that, active conns are tuned based on network and CPU resources and some memory is left aside for idle connections, because it has its own advantages in minimizing round trips (TCP/TLS).

Yeah, I can understand why we need an "active" limit, but I am wondering if we need to wait until read_ready to close a connection when we know that the active queue is full.

"retries" is a good question - What sort of retry did you have in mind? Given that we want to limit the max active connections we allow into the system, and the client has already sent the request, retrying at that point can only be limited to retrying the add operation to the queue. Not sure if it's really worth it. Thoughts?

I was thinking that active connections are checked at the time of accepting the connection. Once the connection is accepted, we try a configured number of times to add to the queue until a configured timeout. I do not think we are doing that here.
Also, your change is only handling H1 connections. Is this intentional?

The diff also has the change for h/2 in Http2ConnectionState.cc.

Why do you want to close the connection immediately without any retries? Also, Why is the throttling not done while accepting the connection?

Yeah, so the difference here is that this is active connection limiting - the total connections are still under the overall connection limit and we only want to throttle when the new connection becomes "active" (ie sends a request). And, the "active" connection queue can change between the time a new connection is accepted and when the client actually sends request over the connection.

Does this mean the connection is going to reset as the client might have already sent the data?

Yeah, that's correct. That is the only way we know the connection is "active", otherwise browsers tend to make connections ahead of time without sending bytes.

That means we accept new connections until the idle connections limit even if the active_queue is full and close the connection when it becomes active.

Yeah, it sounds a bit confusing :) - but there's no direct idle connection limit. There's just the active connection limit and the rest is idle connections bucket. It does sound a bit odd that we throttle new connections, while we've idle connections sitting around, but, I think the key is that these two are not identical. Active connections are far more demanding in terms of resource consumption while idle connections are cheaper (just memory). You could argue that, we drop the idle connections and accept new connections in their place. But, the problem is active connections require other resources besides memory that can be reaped by dropping idle connections. The idea is that, active conns are tuned based on network and CPU resources and some memory is left aside for idle connections, because it has its own advantages in minimizing round trips (TCP/TLS).

Yeah, I can understand why we need an "active" limit, but I am wondering if we need to wait until read_ready to close a connection when we know that the active queue is full.

Yeah, that's a good point. I thought about that as well, but, I think delaying the close until actual bytes are seen, gives a sort of a compromise in a sense in terms of how late we want to make the decision (this extra delay sort of implicitly bakes in the "retry" that you described earlier).

To be clear, these limits should be configured in such way that, they don't trip just like that, but only when things are either under-provisioned or if there's a malicious attack/retry storm or other abnormal traffic spike. Given that, I think delaying the conn until read_ready seems like a good start. I think we can start with this and it should be a simple change to move this to earlier than read_ready (at the time of accept like you are implying) if needed.

Thoughts?

Fwiw, I just put this patch in our prod, will observe it for a few hours to see if there's anything to be concerned about.

"retries" is a good question - What sort of retry did you have in mind? Given that we want to limit the max active connections we allow into the system, and the client has already sent the request, retrying at that point can only be limited to retrying the add operation to the queue. Not sure if it's really worth it. Thoughts?

I was thinking that active connections are checked at the time of accepting the connection. Once the connection is accepted, we try a configured number of times to add to the queue until a configured timeout. I do not think we are doing that here.
Also, your change is only handling H1 connections. Is this intentional?

The diff also has the change for h/2 in Http2ConnectionState.cc.

Why do you want to close the connection immediately without any retries? Also, Why is the throttling not done while accepting the connection?

Yeah, so the difference here is that this is active connection limiting - the total connections are still under the overall connection limit and we only want to throttle when the new connection becomes "active" (ie sends a request). And, the "active" connection queue can change between the time a new connection is accepted and when the client actually sends request over the connection.

Does this mean the connection is going to reset as the client might have already sent the data?

Yeah, that's correct. That is the only way we know the connection is "active", otherwise browsers tend to make connections ahead of time without sending bytes.

"retries" is a good question - What sort of retry did you have in mind? Given that we want to limit the max active connections we allow into the system, and the client has already sent the request, retrying at that point can only be limited to retrying the add operation to the queue. Not sure if it's really worth it. Thoughts?

I was thinking that active connections are checked at the time of accepting the connection. Once the connection is accepted, we try a configured number of times to add to the queue until a configured timeout. I do not think we are doing that here.
Also, your change is only handling H1 connections. Is this intentional?

The diff also has the change for h/2 in Http2ConnectionState.cc.
Do we consider one H2 stream as one connection?

No, but, I've modified add_to_active_queue() to only fail for a "new" connection - if the connection is already in the queue, it doesn't fail. So, the change will only throttle a new h/2 connection, but not new streams on an already accepted connection (we've max stream concurrency setting for that)

That means theoretically, we may have 100x requests in H2?
number of h1 requests for h2 clients= proxy.config.http2.max_concurrent_streams_in * proxy.process.net.max.active.connections_throttled_in http1.
By default, proxy.config.http2.max_concurrent_streams_in is 100.

Yeah, that is right. That is fortunately or unfortunately a limitation with having to use connection based throttling for h2 (it's not nearly as effective on h2, compared to http/1.1).

That said, I think it again comes down to how you tune the connection limit. If for e.g, you expect all your traffic is h/2, then it's only a matter of dropping the active connection limit to what you'd set for a http/1.1 site by a factor = average number of streams.

We can probably consider a different configs for max active connections for http/1.1 vs h/2 at a later phase, but, I think it should be possible to arrive at those numbers given a reasonable estimate of your traffic profile/mix.

Orthogonal to this, I've also added a concurrency based rate limiter in our Traffic layer recently which basically monitors concurrency of the configured endpoints and throttles past that. Since it's based on request concurrency, that works at a level that is agnostic to the transport (h2 or http/1.1). I plan to open source that soon, and I think coupled with that, tuning active connections should give a good coverage.

Thoughts?

Delaying the close doesn't concern me as much as the H2 problem. I think it will be challenging to configure the number of active connections and the maximum number of streams as we do not know the average number of streams that can occur in the system. Also, I think the idle connection limit is guarding the connection resources. So, to defend the other resources, we can have an active connection limit as a maximum of (H2 streams+ H1 active requests).

What do you think?

Delaying the close doesn't concern me as much as the H2 problem. I think it will be challenging to configure the number of active connections and the maximum number of streams as we do not know the average number of streams that can occur in the system. Also, I think the idle connection limit is guarding the connection resources. So, to defend the other resources, we can have an active connection limit as a maximum of (H2 streams+ H1 active requests).

What do you think?

Right, but that’s exactly how you are going to configure the active connection limit - maximum QPS (concurrency, to be precise), you can support. There are no automatic configs for qps/concurrency in ATS and active connection limit here is basically what that will represent.

Looks good. While you are in there, it would be nice to put in documentation for the proxy.config controls for this feature.

Updated docs.

HTTP2_ERROR_CLASS_NONE has to be changed at minimum.

HTTP2_ERROR_CLASS_NONE has to be changed at minimum.

Changed to connection class

Confirmed the requested change.

Looks good to me as long as you and @maskit came to an agreement on the appropriate HTTP/2 return codes.

Looks good to me as long as you and @maskit came to an agreement on the appropriate HTTP/2 return codes.

Yeah, IIUC @maskit didn't have a problem with using (connection::no_error) return code and that is how it is in the final PR.

Cherry-picked to v9.0.x branch.