From d53a0cb505ee15c7c3a2c632be2a2dd29a5fb80e Mon Sep 17 00:00:00 2001 From: Sayan Date: Tue, 22 Jun 2021 22:11:59 +0530 Subject: [PATCH] Fix handling of SIGTERM on *nix (#178) * Fix handling of SIGTERM on *nix This is just for future extensibility * Fix error codes I have been silly enough to break error codes --- CHANGELOG.md | 2 ++ cli/src/argparse.rs | 4 ++-- libstress/src/traits.rs | 4 ++-- server/src/config/mod.rs | 2 +- server/src/dbnet/mod.rs | 52 ++++++++++++++++++++++++++++++++++++---- server/src/main.rs | 12 +++++----- sky-bench/src/util.rs | 2 +- 7 files changed, 62 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 38764a6b..d41af9dc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,8 @@ All changes in this project will be noted in this file. - More than one process can no longer concurrently use the same data directory, preventing any possible data corruption [see [#169](https://github.com/skytable/skytable/pull/169), [#167](https://github.com/skytable/skytable/issues/167)] - Fixed longstanding error in `sky-bench` component that caused key collisions - Fixed bug in `sky-bench` that allowed passing 0 for the inputs +- Fixed handling of SIGTERM in `skyd` [see [#178](https://github.com/skytable/skytable/pull/178)] +- Fixed incorrect termination codes [see [#178](https://github.com/skytable/skytable/pull/178)] ### Additions diff --git a/cli/src/argparse.rs b/cli/src/argparse.rs index e1bc6753..584901f6 100644 --- a/cli/src/argparse.rs +++ b/cli/src/argparse.rs @@ -54,7 +54,7 @@ pub async fn start_repl() { Ok(p) => p, Err(_) => { eprintln!("ERROR: Invalid port"); - process::exit(0x100); + process::exit(0x01); } }, None => 2003, @@ -63,7 +63,7 @@ pub async fn start_repl() { Ok(c) => c, Err(e) => { eprintln!("ERROR: {}", e); - process::exit(0x100); + process::exit(0x01); } }; let mut runner = Runner::new(con); diff --git a/libstress/src/traits.rs b/libstress/src/traits.rs index e15cdeb9..12d7c343 100644 --- a/libstress/src/traits.rs +++ b/libstress/src/traits.rs @@ -46,7 +46,7 @@ where match self { Self::Err(e) => { log::error!("{} : '{}'", msg.to_string(), e); - std::process::exit(0x100); + std::process::exit(0x01); } Self::Ok(v) => v, } @@ -61,7 +61,7 @@ impl ExitError for Option { match self { Self::None => { log::error!("{}", msg.to_string()); - std::process::exit(0x100); + std::process::exit(0x01); } Self::Some(v) => v, } diff --git a/server/src/config/mod.rs b/server/src/config/mod.rs index a07a0cd1..e7c57dd6 100644 --- a/server/src/config/mod.rs +++ b/server/src/config/mod.rs @@ -422,7 +422,7 @@ pub fn get_config_file_or_return_cfg() -> Result Result { + let sigterm = fnsignal(SignalKind::terminate()) + .map_err(|e| format!("Failed to bind to signal with: {}", e))?; + Ok(Self { sigterm }) + } +} + +#[cfg(unix)] +impl Future for UnixTerminationSignal { + type Output = Option<()>; + + fn poll(mut self: Pin<&mut Self>, ctx: &mut Context<'_>) -> Poll { + self.sigterm.poll_recv(ctx) + } +} + /// Start the server waiting for incoming connections or a CTRL+C signal pub async fn run( ports: PortConfig, @@ -294,12 +322,28 @@ pub async fn run( MultiListener::new_multi(secure_listener, insecure_listener, ssl).await? } }; - tokio::select! { - _ = server.run_server() => {} - _ = sig => { - log::info!("Signalling all workers to shut down"); + #[cfg(not(unix))] + { + // Non-unix, usually Windows specific signal handling. + // FIXME(@ohsayan): For now, let's just + // bother with ctrl+c, we'll move ahead as users require them + tokio::select! { + _ = server.run_server() => {} + _ = sig => {} + } + } + #[cfg(unix)] + { + let sigterm = UnixTerminationSignal::init()?; + // apart from CTRLC, the only other thing we care about is SIGTERM + // FIXME(@ohsayan): Maybe we should respond to SIGHUP too? + tokio::select! { + _ = server.run_server() => {}, + _ = sig => {}, + _ = sigterm => {} } } + log::info!("Signalling all workers to shut down"); // drop the signal and let others exit drop(signal); server.finish_with_termsig().await; diff --git a/server/src/main.rs b/server/src/main.rs index a9c0275d..73e57e44 100644 --- a/server/src/main.rs +++ b/server/src/main.rs @@ -108,7 +108,7 @@ fn main() { // uh oh, something happened while starting up log::error!("{}", e); pre_shutdown_cleanup(pid_file); - process::exit(0x100); + process::exit(1); } }; assert_eq!( @@ -141,7 +141,7 @@ pub fn pre_shutdown_cleanup(pid_file: fs::File) { drop(pid_file); if let Err(e) = fs::remove_file(PATH) { log::error!("Shutdown failure: Failed to remove pid file: {}", e); - process::exit(0x100); + process::exit(0x01); } } @@ -168,7 +168,7 @@ fn check_args_and_get_cfg() -> (PortConfig, BGSave, SnapshotConfig, Option { log::error!("{}", e); - std::process::exit(0x100); + std::process::exit(0x01); } }; binding_and_cfg @@ -190,7 +190,7 @@ fn run_pre_startup_tasks() -> fs::File { "Startup failure: Another process with parent PID {} is using the data directory", pid ); - process::exit(0x100); + process::exit(0x01); } let mut file = match fs::OpenOptions::new() .create(true) @@ -201,12 +201,12 @@ fn run_pre_startup_tasks() -> fs::File { Ok(fle) => fle, Err(e) => { log::error!("Startup failure: Failed to open pid file: {}", e); - process::exit(0x100); + process::exit(0x01); } }; if let Err(e) = file.write_all(process::id().to_string().as_bytes()) { log::error!("Startup failure: Failed to write to pid file: {}", e); - process::exit(0x100); + process::exit(0x01); } file } diff --git a/sky-bench/src/util.rs b/sky-bench/src/util.rs index 4f86a3f0..2e9b4315 100644 --- a/sky-bench/src/util.rs +++ b/sky-bench/src/util.rs @@ -60,7 +60,7 @@ macro_rules! sanity_test { macro_rules! err { ($note:expr) => {{ eprintln!("ERROR: {}", $note); - std::process::exit(0x100); + std::process::exit(0x01); }}; }