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

Add GracefulShutdownHandler #5156

Closed

Conversation

thomasdraebing
Copy link
Contributor

Work in progress

This pull requests tries to resolve Issue #5105.

This adds a handler that takes care of keeping the connection alive
during a shutdown until the request is finished or the graceful stop
timeout is reached.

This is similar to what the StatisticsHandler already does, but does
not collect the metrics, thereby reducing resource usage. Also the
StatisticsHandler does not wait for requests in dispatched state.

TODOs:

  • reduce code duplication with StatisticsHandler
  • more tests
  • documentation
  • requests cancelled by the client do not release the future and allow the shutdown.

@joakime
Copy link
Contributor

joakime commented Aug 14, 2020

Thank you for the pull request.

But we need to satisfy the requirements of Eclipse Legal in order to accept / merge this pull request.

The Eclipse Foundation ECA validation has failed for the commits on this pull request.

See the "Details" link on the eclipsefdn/eca check to know what you have failed to do.

See also https://github.com/eclipse/jetty.project/blob/jetty-10.0.x/CONTRIBUTING.md#eclipse-contributor-agreement

This adds a handler that takes care of keeping the connection alive
during a shutdown until the request is finished or the graceful stop
timeout is reached.

This is similar to what the StatisticsHandler already does, but does
not collect the metrics, thereby reducing resource usage. Also the
StatisticsHandler does not wait for requests in dispatched state.

Signed-off-by: Thomas Draebing <thomas.draebing@sap.com>
@gregw
Copy link
Contributor

gregw commented Aug 17, 2020

I'm against a separate GracefulShutdownHandler as keep global counts is expensive in terms of contended memory/locks etc. It would be bad to do that twice if you wish to have stats and graceful shutdown.

@gregw
Copy link
Contributor

gregw commented Aug 17, 2020

Hmmmm maybe we could have one that doesn't do stats, but the behaviour should be the same as for the stats one and they should be mutually exclusive (at the module level) so we don't get both. We need to work out what the problem is with the stats handler rather than just avoiding the issue.

@gregw
Copy link
Contributor

gregw commented Aug 17, 2020

See more comments in #5105 that discuss a potential way forward.

@gregw gregw linked an issue Aug 17, 2020 that may be closed by this pull request
@thomasdraebing
Copy link
Contributor Author

An easier fix for #5105 was found. Thus, there should be no need to add an additional handler.

Jetty 9.4.32 automation moved this from In progress to Done Aug 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Jetty 9.4.32
  
Done
Development

Successfully merging this pull request may close these issues.

Graceful shutdown does not wait for resumed requests
3 participants