From 611d65e7912716e801d1e57620c1d70f6b13881a Mon Sep 17 00:00:00 2001 From: Justin Geibel Date: Tue, 16 Mar 2021 10:26:05 -0400 Subject: [PATCH 1/2] Use consistent pluralization for log entries --- src/downloads_counter.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/downloads_counter.rs b/src/downloads_counter.rs index 7fbfd6fdc59..a617da59bda 100644 --- a/src/downloads_counter.rs +++ b/src/downloads_counter.rs @@ -80,7 +80,7 @@ impl DownloadsCounter { } println!( - "download_counter all_shards counted_versions={} counted_downloads={} pending_downloads={}", + "downloads_counter all_shards counted_versions={} counted_downloads={} pending_downloads={}", counted_versions, counted_downloads, pending_downloads, @@ -101,7 +101,7 @@ impl DownloadsCounter { let stats = self.persist_shard(&conn, shard)?; println!( - "download_counter shard={} counted_versions={} counted_downloads={} pending_downloads={}", + "downloads_counter shard={} counted_versions={} counted_downloads={} pending_downloads={}", idx, stats.counted_versions, stats.counted_downloads, From 0061c5fb5891d77181558515e82501cd5deab48a Mon Sep 17 00:00:00 2001 From: Justin Geibel Date: Tue, 16 Mar 2021 12:03:14 -0400 Subject: [PATCH 2/2] Boot nginx from the backend app to fix graceful shutdown in production In production we aren't gracefully shutting down as expected. nginx does a quick shutdown when receiving a `SIGTERM` and once it exits, it appears that our process is aborted. Instead of spawning the backend from the shell script, the backend process is now responsible for spawning the shell script. This allows the backend to finish its shutdown work even if the script has already exited. `cat` is passed to the nginx buildpack script as our "server", which will block on `STDIN` so that `nginx` will run until receiving `SIGTERM`. --- Procfile | 2 +- script/start-web.sh | 7 +++++-- src/bin/server.rs | 12 ++++++++---- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/Procfile b/Procfile index 320f6aa1ca2..107a8581882 100644 --- a/Procfile +++ b/Procfile @@ -1,3 +1,3 @@ release: bin/diesel migration run -web: ./script/start-web.sh +web: ./target/release/server background_worker: ./target/release/background-worker diff --git a/script/start-web.sh b/script/start-web.sh index bcf62b29efc..adee94fb935 100755 --- a/script/start-web.sh +++ b/script/start-web.sh @@ -1,12 +1,15 @@ #! /bin/bash set -ue +# Since this script is launched from our app, we tell the nginx +# buildpack (`bin/start-nginx`) that `cat` is our server. + if [[ -z "${USE_FASTBOOT-}" ]]; then unset USE_FASTBOOT - bin/start-nginx ./target/release/server + bin/start-nginx cat else export USE_FASTBOOT node --optimize_for_size --max_old_space_size=200 fastboot.js & - bin/start-nginx ./target/release/server & + bin/start-nginx cat & wait -n fi diff --git a/src/bin/server.rs b/src/bin/server.rs index 5d5d2102f7c..c9fcb86d485 100644 --- a/src/bin/server.rs +++ b/src/bin/server.rs @@ -5,8 +5,8 @@ use cargo_registry::{boot, App, Env}; use std::{ borrow::Cow, fs::File, + process::Command, sync::{mpsc::channel, Arc, Mutex}, - thread, time::Duration, }; @@ -139,9 +139,6 @@ fn main() -> Result<(), Box> { println!("listening on port {}", port); - // Give tokio a chance to spawn the first worker thread - thread::sleep(Duration::from_millis(10)); - // Creating this file tells heroku to tell nginx that the application is ready // to receive traffic. if heroku { @@ -152,6 +149,13 @@ fn main() -> Result<(), Box> { }; println!("Writing to {}", path); File::create(path).unwrap(); + + // Launch nginx via the Heroku nginx buildpack + // `wait()` is never called on the child process, but it should be okay to leave a zombie + // process around on shutdown when Heroku is tearing down the entire container anyway. + Command::new("./script/start-web.sh") + .spawn() + .expect("Couldn't spawn nginx"); } // Block the main thread until the server has shutdown