Default tomcat keep-alive timeout does not seem to be compatible with Cloud Foundry router default keep-alive timeout

[peterellisjones]

Hi java-buildpack team,

As far as I can tell the tomcat default timeout for keep alives is 60 seconds

• keepAliveTimeout The number of milliseconds this Connector will wait for another HTTP request before closing the connection. The default value is to use the value that has been set for the connectionTimeout attribute. Use a value of -1 to indicate no (i.e. infinite) timeout
• connectionTimeout The number of milliseconds this Connector will wait, after accepting a connection, for the request URI line to be presented. Use a value of -1 to indicate no (i.e. infinite) timeout. The default value is 60000 (i.e. 60 seconds) but note that the standard server.xml that ships with Tomcat sets this to 20000 (i.e. 20 seconds). Unless disableUploadTimeout is set to false, this timeout will also be used when reading the request body (if any).

However when keep-alive backend connections are enabled in the gorouter, the gorouter will keep connections alive for 90 seconds.

Any time connections are kept open using http keep-alive, it's important that the client always closes the connection before the server does. If the server closes the connections first there is a risk of a race condition where a client will sends a request to the server on a pre-established connection which arrives immediately after a server has closed the connection.

This means that any person using Tomcat in its default configuration in a Cloud Foundry with keep-alive connections enabled will see occasional 502s caused by this race conditions (the clue for diagnosing this is that it happens exactly 60s after the last successful connection).

(I'm not sure if this is better as a gorouter issue or a java-buildpack issue, but in any case default values should work well together, whatever they happen to be)

---

Supporting information about why it's important for senders to close connections before receivers do for any L7 client-server connection using keep-alives.

https://docs.aws.amazon.com/elasticloadbalancing/latest/classic/config-idle-timeout.html

To ensure that the load balancer is responsible for closing the connections to your instance, make sure that the value you set for the HTTP keep-alive time is greater than the idle timeout setting configured for your load balancer.

https://docs.pivotal.io/application-service/2-8/operating/frontend-idle-timeout.html

In general, set the value higher than your load balancer’s back end idle timeout to avoid the race condition where the load balancer sends a request before it discovers that the Gorouter or HAProxy has closed the connection.

https://blog.percy.io/tuning-nginx-behind-google-cloud-platform-http-s-load-balancer-305982ddb340

This causes the load balancer to be the side that closes idle connections, rather than nginx, which fixes the race condition!

https://tanzu.vmware.com/content/pivotal-engineering-journal/understanding-keep-alive-timeouts-in-the-cloud-foundry-networking-stack-2

Let’s look from the “outside” (the client) to the “inside” (the server in the deployment). As you travel further in, the keep-alive timeouts should be configured to be longer. That is, the outermost layer (in this case, the client) should have the shortest backend keep-alive timeout, and as you go in, the keep-alive timeouts should get progressively longer in relation to the corresponding backend or frontend idle timeout

Consider Golang’s HTTP client, which we believe to be somewhat robust to this type of client-side problem. The low level http transport package used by the default client implements a background loop that checks if a connection in its idle pool has been closed by the server before reusing it to make a request. This leaves a small window for a race condition where the server could close the connection between the check and the new request; in the event that the client loses this race, it retries this request on a new connection.

[dmikusa]

Hi @peterellisjones, thanks for this report.

Is this something you have actually seeing causing 502's, that you've been able to attribute to the timeout? Is there something one could do to force this to happen?

I'm asking, cause I don't think this is something I've seen come up before. I'm not trying to discount what you're saying. I understand and believe the technical merit of what you're reporting. Because this is timing-based, I just wonder about the odds of this happening. I'm guessing it's a low probability. I'm looking to see if you have any real-world experience in terms of how frequently this pops up.

I'm not 100% sure I want to change the default, or at least I want to be very careful about doing so. There are implications to having higher timeouts for keep-alive connections. Shorter is typically better from the server's point of view because it drops connections faster and uses fewer resources. I'm not saying this is the most important consideration, but I do think we have to be aware of it.

In addition, the "best" value for the time out is really environment-dependent. Gorouter defaults to 90s, but that can be changed. You also have a layer of external load balancers that also need to be configured to achieve the ideal situation you're talking about.

It makes me wonder if documentation is possibly a better route here? Adding something that mentioned this possibility and links to resources which direct a user how to a.) configure this properly across all levels of LBs in their environment and b.) if necessary, adjust the Tomcat value when using the JBP. Thoughts on this approach?

[peterellisjones]

Hi @dmikusa-pivotal yes this is an issue we've seen with SAP customers with high-traffic applications and was resolved by changing the Tomcat timeout.

It's possible that few customers are experiencing this because it will only occur if the Gorouter keeps connections to backend instances alive, which is disabled by default.

It also only occurs with certain request types as the gorouter automatically retries some types of requests on encountering this race condition (it uses this logic from the standard library https://github.com/golang/go/blob/master/src/net/http/transport.go#L87-L94). So GET requests without bodies, which I expect are the most common type of requests for most users, will be automatically retried.

I've manually recreated the issue by sending traffic to an application with a very short timeout (eg 100ms). By firing many consecutive requests I'm able to recreate the race condition where the server closes the connection just as the gorouter sends a new request.

I'm guessing it's a low probability

Yes it's definitely very low probability per-request although the probability of it happening on an application-level approaches one as the application traffic gets larger and larger. With better-coordinated defaults between the gorouter and buildpacks it could be zero probability though :)

Gorouter defaults to 90s, but that can be changed.

The keep alive timeout for incoming connections to the Gorouter can be modified (ie from LB to Gorouter). The LB to Gorouter connection also needs sensibly-defined keep alive timeouts, but that is a separate issue to this.

The keep alive timeout value for connections from the Gorouter to backend instances is hard-coded to 90 seconds here and is not possible to change. This keep-alive configuration for this connection is the one causing the race condition.

Generally timeouts should be longer as you get closer to the end server to ensure that the caller closes connections first which avoid the race condition (ie timeout durations should be LB < Gorouter < application), but the hardcoded 90s in the gorouter is breaking this rule for the connection to the apps using the default buildpack configuration.

It makes me wonder if documentation is possibly a better route here?

yes perhaps :)

[ywei2017]

• 1 on the issue. @dmikusa-pivotal

I literally have been battling with the issue for over a year, with many rounds, and finally pinpointed the reason, thanking to a large part of this post.

[maxmoehl]

@dmikusa we do now have the documentation but since the value in gorouter cannot be changed I still think it would make sense to align the buildpack and gorouter timeout.

Regarding your point about resource usage, the opposite case could als be made: gorouter maintains connection pools to each application instance to save CPU cycles. Sure, a connection that is kept alive needs memory and two file descriptors, but a TLS handshake isn't exactly cheap either (assuming you have route-integrity enabled, which most environments should have). From our experience gorouter is rather CPU bound then memory bound so using up some more memory doesn't hurt whereas CPU cycles are the factor we are optimising for (however, this is more of an operator POV then a user POV). WDYT?

Are there any other reasons why changing the default is not desirable?

[h0nIg]

I have raised #1078, the fix is even described here: https://knowledge.broadcom.com/external/article/298104/intermittent-502-eof-gorouter-errors-for.html