Skip to content

Increase server.keepAliveTimeout#6692

Merged
lotas merged 1 commit intomainfrom
feat/keepalive-timeouts
Nov 17, 2023
Merged

Increase server.keepAliveTimeout#6692
lotas merged 1 commit intomainfrom
feat/keepalive-timeouts

Conversation

@lotas
Copy link
Copy Markdown
Contributor

@lotas lotas commented Nov 17, 2023

Tweaking server.keepAliveTimeout to fix downstream errors in reverse proxy and load balancer.

Default node's http server.keepAliveTimeout is 5s which might be an issue when working behind a reverse proxy which has bigger timeouts.

To reduce number of 502 errors, application's keep alive timeout should be larger than the one of the reverse proxy,
and that in turn, should be larger than the Load Balancer's one.

This introduces new configuration option for server: KEEP_ALIVE_TIMEOUT_SECONDS that can be used in deployment-specific scenarios.

Related to #6682

Load Balancer (GLB) has 10min keep alive timeout
nginx reverse proxy (iprepd in fxci) has 2min keep alive timeout
taskcluster services are using default node timeout which is 5s

So what happens is that each of the downstream servers tries to keep the connection open to the upstream server, but if request comes in right before this time is out, the connection would be closed, and proxy server would not have a chance to read the response back.

So Ideally GLB 10min -> reverse proxy 10min+X -> app 10min+X+Y to make this chain work without high number of 502s

@lotas lotas requested a review from a team as a code owner November 17, 2023 11:43
@lotas lotas requested review from matt-boris and petemoore and removed request for a team November 17, 2023 11:43
@lotas lotas force-pushed the feat/keepalive-timeouts branch 2 times, most recently from c24cb89 to 0ddca20 Compare November 17, 2023 12:02
Default value is 90s if not overriden in configuration.
Application should have larger keep alive timeout than the reverse
proxy or load balancer.
@lotas lotas force-pushed the feat/keepalive-timeouts branch from 0ddca20 to a304f18 Compare November 17, 2023 12:09
Copy link
Copy Markdown
Member

@petemoore petemoore left a comment

Choose a reason for hiding this comment

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

Got to be worth a try. We can always back out if it doesn't help. Thanks Yarik!

@lotas lotas merged commit 27a7f95 into main Nov 17, 2023
@lotas lotas deleted the feat/keepalive-timeouts branch November 17, 2023 13:26
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.

2 participants