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

Issue #4047 Graceful Cleanup #4094

Closed
wants to merge 16 commits into from
Closed

Conversation

gregw
Copy link
Contributor

@gregw gregw commented Sep 17, 2019

Initially created to just address #4047, this PR has now started addressing other graceful shutdown issues (in preparation for HTTP/2 graceful). See also #4100 for the core fix

Signed-off-by: Greg Wilkins gregw@webtide.com

Added test to reproduce issue
Fixed bug from #2772 where output was shutdown on DONE without checking for END.
Fixed aggregation logic to aggregate last write if aggregation already started.

Signed-off-by: Greg Wilkins <gregw@webtide.com>
@gregw gregw requested a review from sbordet September 17, 2019 02:34
Signed-off-by: Greg Wilkins <gregw@webtide.com>
Signed-off-by: Greg Wilkins <gregw@webtide.com>
@joakime joakime added this to the 9.4.x milestone Sep 17, 2019
Signed-off-by: Greg Wilkins <gregw@webtide.com>
@gregw gregw changed the title Issue #4047 Graceful Write Issue #4047 Graceful Cleanup Sep 18, 2019
Signed-off-by: Greg Wilkins <gregw@webtide.com>
Signed-off-by: Greg Wilkins <gregw@webtide.com>
…47-graceful-write

Signed-off-by: Greg Wilkins <gregw@webtide.com>
Signed-off-by: Greg Wilkins <gregw@webtide.com>
Reverted accidental commit and fixed javadoc error.

Signed-off-by: Greg Wilkins <gregw@webtide.com>
Improved comment
Started stopper earlier

Signed-off-by: Greg Wilkins <gregw@webtide.com>
…47-graceful-write

Signed-off-by: Greg Wilkins <gregw@webtide.com>
Improved comments and clarify conditions

Signed-off-by: Greg Wilkins <gregw@webtide.com>
@gregw gregw removed the WIP / Draft label Sep 24, 2019
Signed-off-by: Greg Wilkins <gregw@webtide.com>
@gregw
Copy link
Contributor Author

gregw commented Oct 14, 2019

I've taken this out of 9.4.22... I'm not even sure it should be in 9.4.x

@gregw
Copy link
Contributor Author

gregw commented Nov 17, 2019

I'm going to rebase this on jetty-10

@gregw gregw closed this Nov 17, 2019
@alefab
Copy link

alefab commented Sep 21, 2020

I've taken this out of 9.4.22... I'm not even sure it should be in 9.4.x

I was searching a way to wait for requests to be handled during shutdown and I find this ticket.
Is there any issue for this ticket not being fixed in the 9.4.x branches ?

@gregw
Copy link
Contributor Author

gregw commented Sep 21, 2020

@alefab There is graceful shutdown support in both 9 and 10. This issue is about cleaning up the implementation.

If you install the stats handler and set the stopTimeout then you'll get graceful shutdown behaviour.

@alefab
Copy link

alefab commented Sep 22, 2020

@gregw Thanks ! After adding a stats handler, the graceful shutdown is working like a charm (9.4.31) :)

@gregw
Copy link
Contributor Author

gregw commented Sep 22, 2020

I know that is kind of counter intuitive..... but counting request can be expensive and cause contention. So if we have to do it, we only want to do it once, hence we make it optional in the stats handler and then hang graceful shutdown off that.

@sbordet in Jetty-10 I guess we could add a graceful-shutdown.mod that depends on the stats.mod ?

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.

3 participants