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 #4919 - WebSocket container graceful stop #4931

Merged
merged 11 commits into from
Jul 29, 2020

Conversation

lachlan-roberts
Copy link
Contributor

Closes #4919

Make sure all active websocket connections are closed before the connections are hard closed.

Server.doStop() now initiates a graceful shutdown but only waits for the CompletableFuture if the stopTimeout is set.

@lachlan-roberts lachlan-roberts requested a review from gregw June 3, 2020 01:38
…4919-WebSocketContainerStop

Signed-off-by: Lachlan Roberts <[email protected]>
@lachlan-roberts lachlan-roberts requested a review from gregw July 1, 2020 12:32
@lachlan-roberts
Copy link
Contributor Author

@gregw implemented the changes.

There is a problem that there is no API for a Javax WebSocketContainer stop timeout, so it can't be stopped gracefully. I could try putting a short set length stop timeout to give some minimal effort to close gracefully, but not sure how that would interact with the ServerContainer shutdown.

…4919-WebSocketContainerStop

Signed-off-by: Lachlan Roberts <[email protected]>
Copy link
Contributor

@gregw gregw left a comment

Choose a reason for hiding this comment

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

I'm approving this other than a javadoc and naming niggle and a suggestion to make cancel work a bit better.

I think this PR should be merged, but then we should start another one looking at other improvements in graceful (eg like actually calling cancel if the stopTimeout expires).

@lachlan-roberts
Copy link
Contributor Author

@gregw I didn't like the state of this PR so I've done some more changes and now I think it is more in line with the way Graceful should be used.

Now the SessionTracker always remains a bean on the websocket container and the SessionTracker is the one to implement Graceful not the container itself. The SessionTracker is then shutdown by Graceful.shutdown(Container) which will find the SessionTracker in the Container hierarchy.

@lachlan-roberts lachlan-roberts merged commit fe6f0eb into jetty-10.0.x Jul 29, 2020
@lachlan-roberts lachlan-roberts deleted the jetty-10.0.x-4919-WebSocketContainerStop branch July 29, 2020 03:20
@chylek-qr
Copy link

chylek-qr commented Nov 22, 2023

EDIT: Moved to #10918

@joakime
Copy link
Contributor

joakime commented Nov 22, 2023

@chylek-qr please open a new issue, a comment on a 3 year old PR is unlikely to be tracked.

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.

websocket container stop ordering
4 participants