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

graceful_shutdown doesn't shut down gracefully #114

Closed
droundy opened this issue Mar 20, 2024 · 1 comment · Fixed by #119
Closed

graceful_shutdown doesn't shut down gracefully #114

droundy opened this issue Mar 20, 2024 · 1 comment · Fixed by #119

Comments

@droundy
Copy link

droundy commented Mar 20, 2024

I'm seeing that there is always a single connection registered if the Watcher for a connection hasn't yet been dropped when graceful_shutdown is called (which is the point of graceful_shutdown!).

connection made, and so graceful_shutdown doesn't exit until the timeout happens.

I've reproduced this both with axum-server 0.6.0, and with the current master branch. I added prints to confirm which code branches are getting triggered. This is readily triggered by integration tests in my code (which I cannot share), which will uniformly fail if I don't insert a brief wait between when I receive a response and when I send a SIGINT to the process.

Specifically, I'm seeing that after the code reaches this line:

https://github.com/programatik29/axum-server/blob/master/src/server.rs#L207

the serve_future in the inner select! doesn't ever finish.

If I wait a millisecond before sending the SIGINT, then the connection is marked as completed and graceful shutdown exits cleanly. Except of course that this is the scenario where one wants to gracefully shut down.

@droundy
Copy link
Author

droundy commented Mar 20, 2024

I should probably add that this problem surfaced with the upgrade from axum-server 0.5.1 to axum-server 0.6.0, which happened at the same time as a move to axum 0.7.4, in case that helps.

@droundy droundy changed the title graceful_shutdown doesn't shut down graceful_shutdown doesn't shut down gracefully Mar 20, 2024
phlip9 added a commit to lexe-app/axum-server that referenced this issue May 18, 2024
The hyper v1 migration PR (programatik29#93) accidentally (?) removed the call to
`serve_future.graceful_shutdown()`, which prevents the server from shutting
down until all client _connections_ close on their own (or we hit the hard
shutdown timeout).

In practice, this meant we always hit the hard shutdown timeout, since most
clients use connection pools with connection keepalive.

With this change, graceful shutdown will now correctly stop handling any
new requests after it's triggered, while letting existing requests
complete (within the timeout of course).

Fixes programatik29#114
phlip9 added a commit to lexe-app/axum-server that referenced this issue May 20, 2024
The hyper v1 migration PR (programatik29#93) accidentally (?) removed the call to
`serve_future.graceful_shutdown()`, which prevents the server from shutting
down until all client _connections_ close on their own (or we hit the hard
shutdown timeout).

In practice, this meant we always hit the hard shutdown timeout, since most
clients use connection pools with connection keepalive.

With this change, graceful shutdown will now correctly stop handling any
new requests after it's triggered, while letting existing requests
complete (within the timeout of course).

Fixes programatik29#114
phlip9 added a commit to lexe-app/axum-server that referenced this issue May 20, 2024
The hyper v1 migration PR (programatik29#93) accidentally (?) removed the call to
`serve_future.graceful_shutdown()`, which prevents the server from shutting
down until all client _connections_ close on their own (or we hit the hard
shutdown timeout).

In practice, this meant we always hit the hard shutdown timeout, since most
clients use connection pools with connection keepalive.

With this change, graceful shutdown will now correctly stop handling any
new requests after it's triggered, while letting existing requests
complete (within the timeout of course).

Fixes programatik29#114
phlip9 added a commit to lexe-app/axum-server that referenced this issue May 20, 2024
The hyper v1 migration PR (programatik29#93) accidentally (?) removed the call to
`serve_future.graceful_shutdown()`, which prevents the server from shutting
down until all client _connections_ close on their own (or we hit the hard
shutdown timeout).

In practice, this meant we always hit the hard shutdown timeout, since most
clients use connection pools with connection keepalive.

With this change, graceful shutdown will now correctly stop handling any
new requests after it's triggered, while letting existing requests
complete (within the timeout of course).

Fixes programatik29#114
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 a pull request may close this issue.

1 participant