From f54474d296c6118f6774ef086ecd93ea0ee721e6 Mon Sep 17 00:00:00 2001
From: Christian Schwarz <christian@neon.tech>
Date: Mon, 29 Jan 2024 12:23:25 +0000
Subject: [PATCH 1/2] refactor(pageserver main): signal handling

This refactoring makes it easier to experimentally replace
BACKGROUND_RUNTIME with a single-threaded runtime.
Found this useful during benchmarking.
---
 pageserver/src/bin/pageserver.rs | 62 +++++++++++++++++---------------
 1 file changed, 34 insertions(+), 28 deletions(-)

diff --git a/pageserver/src/bin/pageserver.rs b/pageserver/src/bin/pageserver.rs
index 84de76e55e86..23bd334902cc 100644
--- a/pageserver/src/bin/pageserver.rs
+++ b/pageserver/src/bin/pageserver.rs
@@ -33,12 +33,10 @@ use pageserver::{
 use postgres_backend::AuthType;
 use utils::failpoint_support;
 use utils::logging::TracingErrorLayerEnablement;
-use utils::signals::ShutdownSignals;
 use utils::{
     auth::{JwtAuth, SwappableJwtAuth},
     logging, project_build_tag, project_git_version,
     sentry_init::init_sentry,
-    signals::Signal,
     tcp_listener,
 };
 
@@ -656,34 +654,42 @@ fn start_pageserver(
     let mut shutdown_pageserver = Some(shutdown_pageserver.drop_guard());
 
     // All started up! Now just sit and wait for shutdown signal.
-    ShutdownSignals::handle(|signal| match signal {
-        Signal::Quit => {
-            info!(
-                "Got {}. Terminating in immediate shutdown mode",
-                signal.name()
-            );
-            std::process::exit(111);
-        }
-
-        Signal::Interrupt | Signal::Terminate => {
-            info!(
-                "Got {}. Terminating gracefully in fast shutdown mode",
-                signal.name()
-            );
-
-            // This cancels the `shutdown_pageserver` cancellation tree.
-            // Right now that tree doesn't reach very far, and `task_mgr` is used instead.
-            // The plan is to change that over time.
-            shutdown_pageserver.take();
-            let bg_remote_storage = remote_storage.clone();
-            let bg_deletion_queue = deletion_queue.clone();
-            BACKGROUND_RUNTIME.block_on(pageserver::shutdown_pageserver(
-                bg_remote_storage.map(|_| bg_deletion_queue),
-                0,
-            ));
+    {
+        use signal_hook::consts::*;
+        let signal_handler = BACKGROUND_RUNTIME.spawn_blocking(move || {
+            let mut signals =
+                signal_hook::iterator::Signals::new([SIGINT, SIGTERM, SIGQUIT]).unwrap();
+            for sig in signals.forever() {
+                return sig;
+            }
             unreachable!()
+        });
+        let signal = BACKGROUND_RUNTIME
+            .block_on(signal_handler)
+            .expect("join error");
+        match signal {
+            SIGQUIT => {
+                info!("Got {signal}. Terminating in immediate shutdown mode",);
+                std::process::exit(111);
+            }
+            SIGINT | SIGTERM => {
+                info!("Got {signal}. Terminating gracefully in fast shutdown mode",);
+
+                // This cancels the `shutdown_pageserver` cancellation tree.
+                // Right now that tree doesn't reach very far, and `task_mgr` is used instead.
+                // The plan is to change that over time.
+                shutdown_pageserver.take();
+                let bg_remote_storage = remote_storage.clone();
+                let bg_deletion_queue = deletion_queue.clone();
+                BACKGROUND_RUNTIME.block_on(pageserver::shutdown_pageserver(
+                    bg_remote_storage.map(|_| bg_deletion_queue),
+                    0,
+                ));
+                unreachable!()
+            }
+            _ => unreachable!(),
         }
-    })
+    }
 }
 
 fn create_remote_storage_client(

From 92df35324ee9c6865f5b9afc17bc3791c42826a9 Mon Sep 17 00:00:00 2001
From: Christian Schwarz <christian@neon.tech>
Date: Wed, 31 Jan 2024 15:13:25 +0000
Subject: [PATCH 2/2] clippy & minor log message improvement

---
 pageserver/src/bin/pageserver.rs | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/pageserver/src/bin/pageserver.rs b/pageserver/src/bin/pageserver.rs
index 23bd334902cc..eaddcb4607c0 100644
--- a/pageserver/src/bin/pageserver.rs
+++ b/pageserver/src/bin/pageserver.rs
@@ -659,21 +659,21 @@ fn start_pageserver(
         let signal_handler = BACKGROUND_RUNTIME.spawn_blocking(move || {
             let mut signals =
                 signal_hook::iterator::Signals::new([SIGINT, SIGTERM, SIGQUIT]).unwrap();
-            for sig in signals.forever() {
-                return sig;
-            }
-            unreachable!()
+            return signals
+                .forever()
+                .next()
+                .expect("forever() never returns None unless explicitly closed");
         });
         let signal = BACKGROUND_RUNTIME
             .block_on(signal_handler)
             .expect("join error");
         match signal {
             SIGQUIT => {
-                info!("Got {signal}. Terminating in immediate shutdown mode",);
+                info!("Got signal {signal}. Terminating in immediate shutdown mode",);
                 std::process::exit(111);
             }
             SIGINT | SIGTERM => {
-                info!("Got {signal}. Terminating gracefully in fast shutdown mode",);
+                info!("Got signal {signal}. Terminating gracefully in fast shutdown mode",);
 
                 // This cancels the `shutdown_pageserver` cancellation tree.
                 // Right now that tree doesn't reach very far, and `task_mgr` is used instead.