axum-server icon indicating copy to clipboard operation
axum-server copied to clipboard

fix: propagate graceful shutdown to inner hyper connection

Open phlip9 opened this issue 1 year ago • 3 comments

The hyper v1 migration PR (#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).

The fix itself is just one line:

                        let serve_future = builder.serve_connection_with_upgrades(io, service);
                        tokio::pin!(serve_future);

                        tokio::select! {
                            biased;
                            _ = watcher.wait_graceful_shutdown() => {
+                               serve_future.as_mut().graceful_shutdown();
                                tokio::select! {
                                    biased;
                                    _ = watcher.wait_shutdown() => (),
                                    _ = &mut serve_future => (),
                                }
                            }
                            _ = watcher.wait_shutdown() => (),
                            _ = &mut serve_future => (),
                        }

The bulk of the PR is fixing the graceful shutdown tests.

Fixes #114

phlip9 avatar May 20 '24 20:05 phlip9

cc @abs0luty @programatik29

phlip9 avatar May 21 '24 19:05 phlip9

We were having issues with graceful shutdown delays in grapevine, and have confirmed that this patch fixes them. Shutdown now happens in ~30ms.

olivia-fl avatar Jun 07 '24 21:06 olivia-fl

cc @abs0luty @programatik29

Would be great to get this merged upstream : )

phlip9 avatar Jul 03 '24 19:07 phlip9

Sorry for inactivity. tls_openssl::tests::test_graceful_shutdown seems to fail when running cargo hack test --each-feature command.

programatik29 avatar Jul 30 '24 15:07 programatik29

It seems to be caused by an older test which this PR didn't update. Graceful shutdown is working as intended.

programatik29 avatar Jul 30 '24 15:07 programatik29

TLS modules shouldn't have graceful shutdown tests. Fault seems to be mine oops.

programatik29 avatar Jul 30 '24 15:07 programatik29