Skip to content

Commit

Permalink
Changing the shutdown logic for the rest server. (#4173)
Browse files Browse the repository at this point in the history
Instead of relying on the hyper graceful shutdown, which does
not shutdown in the presence of existing http connections.

Closes #4169
  • Loading branch information
fulmicoton authored Nov 22, 2023
1 parent ebdb39f commit fa2efb2
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -335,8 +335,6 @@ impl ClusterSandbox {
pub async fn shutdown(self) -> Result<Vec<HashMap<String, ActorExitStatus>>, anyhow::Error> {
// We need to drop rest clients first because reqwest can hold connections open
// preventing rest server's graceful shutdown.
drop(self.searcher_rest_client);
drop(self.indexer_rest_client);
self.shutdown_trigger.shutdown();
let result = future::join_all(self.join_handles).await;
let mut statuses = Vec::new();
Expand Down
16 changes: 13 additions & 3 deletions quickwit/quickwit-serve/src/rest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,9 +158,19 @@ pub(crate) async fn start_rest_server(
rest_listen_addr=?rest_listen_addr,
"Starting REST server listening on {rest_listen_addr}."
);
let serve_fut = hyper::Server::bind(&rest_listen_addr)
.serve(Shared::new(service))
.with_graceful_shutdown(shutdown_signal);

// `graceful_shutdown()` seems to be blocking in presence of existing connections.
// The following approach of dropping the serve supposedly is not bullet proof, but it seems to
// work in our unit test.
//
// See more of the discussion here:
// https://github.com/hyperium/hyper/issues/2386
let serve_fut = async move {
tokio::select! {
res = hyper::Server::bind(&rest_listen_addr).serve(Shared::new(service)) => { res }
_ = shutdown_signal => { Ok(()) }
}
};

let (serve_res, _trigger_res) = tokio::join!(serve_fut, readiness_trigger);
serve_res?;
Expand Down

0 comments on commit fa2efb2

Please sign in to comment.