Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Netty servers: handle requestTimeout properly, add idleTimeout #3815

Merged
merged 2 commits into from
Jun 5, 2024

Conversation

kciesielski
Copy link
Member

@kciesielski kciesielski commented Jun 3, 2024

Fixes #3795

This PR replaces usage of ReadTimeoutHandler with IdleStateHandler.
The ReadStateHandler was configured using NettyConfig.requestTimeout value, but it wasn't serving the purpose of failing when there is no response within given timeout. This handler throws an exception when there is no data read , which means no request from the client.

We will be using IdleStateHandler to deal with timeouts correctly:

  1. One such handler is created for each request, with timeout set to nettyConfig.requestTimeout. If it expires, the channel is closed, a HTTP 503 response is returned, and in-flight requests in the channel are canceled. An error is logged.
  2. if backend manages to produce a response, the handler is removed.
  3. Thus, in case of pieplining, there will be multiple IdleStateHandler instances running, one for each running request processing.
  4. Additionally, we introduce NettyConfig.idleTimeout. Similarly to idle-timeout in akka-http, it defines a time of no reads nor writes, after which the channel will be closed silently (only with a debug log). This is a "normal situation" that may happen after a long-running keep-alive connection expires (typically from a web browser), like in io.netty.handler.timeout.ReadTimeoutException logged for a simple endpoint #3795, which initiated this work.
  5. The value of idleTimeout is set by default to 60s, while requestTimeout is 20s.

@kciesielski kciesielski marked this pull request as ready for review June 3, 2024 12:28
@kciesielski kciesielski requested a review from adamw June 3, 2024 12:28
build.sbt Show resolved Hide resolved
@kciesielski kciesielski merged commit cb5c2fe into master Jun 5, 2024
26 checks passed
@kciesielski kciesielski deleted the fix-netty-timeouts branch June 5, 2024 07:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

io.netty.handler.timeout.ReadTimeoutException logged for a simple endpoint
2 participants