Merge branch 'fix/drain-does-not-wait-on-blob-streams'

A restart during a long download stalled the drain until TimeoutStopSec
expired and systemd escalated to SIGABRT. The drain is now bounded by
server.shutdown_grace and exits cleanly.

Closes #3

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PqNtYNhov3fukx46KS9R7L

asset/config/config.toml.tmpl

@@ -17,6 +17,11 @@ external_url         = "https://rf.internal"
 # timeout here is an idle timeout instead. This one bounds how long a client may
 # stop reading before the tee detaches it and finishes the transfer anyway.
 client_stall_timeout = "60s"
+# How long in-flight responses may drain on SIGTERM before the service stops
+# anyway. A blob response lasts size/bandwidth, so an unbounded drain never
+# converges: systemd's TimeoutStopSec expires and escalates to SIGABRT. Cutting
+# a download deliberately is the better failure -- the client retries.
+shutdown_grace       = "15s"
 
 [storage]
 endpoint               = "{{S3_ENDPOINT}}"

asset/systemd/rustingface.service

@@ -21,9 +21,12 @@ LoadCredential=s3-access-key:/etc/rustingface/s3-access-key
 LoadCredential=s3-secret-key:/etc/rustingface/s3-secret-key
 LoadCredential=hf-token:/etc/rustingface/hf-token
 
-# Long transfers must not be killed mid-flight on stop; a detached upload that
-# is cut short leaves an orphan blob, which is safe but wasteful.
-TimeoutStopSec=120
+# A backstop, not the mechanism. The service caps its own drain at
+# server.shutdown_grace and then exits cleanly, because a blob response lasts
+# size/bandwidth and waiting for one to finish does not converge -- letting this
+# expire instead means SIGABRT and a core dump. Keep it comfortably above the
+# configured grace so systemd never escalates first.
+TimeoutStopSec=60
 
 # Hardening. The process holds no durable state — the bucket is the whole of it
 # — so there is no StateDirectory to protect and ProtectSystem can stay strict

crates/rustingface-entities/src/config.rs

@@ -51,6 +51,16 @@ pub struct Server {
     /// detached and finished without it.
     #[serde(with = "humantime_serde")]
     pub client_stall_timeout: Duration,
+    /// How long to let in-flight requests drain on SIGTERM before stopping
+    /// anyway.
+    ///
+    /// A blob response is an in-flight request that lasts `size / bandwidth`,
+    /// so an unbounded drain does not converge on a restart: systemd's
+    /// `TimeoutStopSec` expires and escalates to SIGABRT, which is a core dump
+    /// rather than a shutdown. Cutting a download deliberately is the better
+    /// failure -- clients retry, and nothing in the bucket depends on it.
+    #[serde(with = "humantime_serde")]
+    pub shutdown_grace: Duration,
 }
 
 impl Default for Server {
@@ -59,6 +69,7 @@ impl Default for Server {
             listen: "127.0.0.1:20482".into(),
             external_url: None,
             client_stall_timeout: Duration::from_secs(60),
+            shutdown_grace: Duration::from_secs(15),
         }
     }
 }

crates/rustingface/src/cmd/serve.rs

@@ -70,7 +70,7 @@ pub async fn run(config: Config) -> Result<()> {
     let app = router(AppState::new(registry, tokens.into(), metrics));
 
     axum::serve(listener, app)
-        .with_graceful_shutdown(shutdown())
+        .with_graceful_shutdown(shutdown(config.server.shutdown_grace))
         .await
         .context("serving")
 }
@@ -87,12 +87,22 @@ fn token_file_has_content(config: &Config) -> bool {
     })
 }
 
-/// Stop accepting on `SIGTERM`/`SIGINT` and let in-flight requests drain.
+/// Stop accepting on `SIGTERM`/`SIGINT` and let in-flight requests drain --
+/// for a bounded time.
 ///
-/// Detached transfers are not requests and are not waited on here: the process
-/// exiting mid-upload leaves an orphan blob, which is safe and reclaimable,
-/// while a manifest is only ever written after an upload completes.
-async fn shutdown() {
+/// The bound is the point. A blob response *is* an in-flight request and it
+/// lasts `size / bandwidth`, so waiting for one to finish does not converge:
+/// a 3GB shard at 8MB/s holds the drain open for seven minutes, systemd's
+/// `TimeoutStopSec` expires long before that, and the service is killed with
+/// SIGABRT and a core dump. Cutting the download deliberately after
+/// `server.shutdown_grace` is strictly better -- the client retries, which is
+/// a normal event, and the restart is prompt.
+///
+/// Detached transfers are not requests and are not waited on here either: the
+/// process exiting mid-upload leaves an orphan blob, which is safe and
+/// reclaimable, while a manifest is only ever written after an upload
+/// completes.
+async fn shutdown(grace: std::time::Duration) {
     let mut term = match signal(SignalKind::terminate()) {
         Ok(signal) => signal,
         Err(err) => {
@@ -101,11 +111,26 @@ async fn shutdown() {
         }
     };
     tokio::select! {
-        _ = term.recv() => info!("SIGTERM received; draining"),
+        _ = term.recv() => info!(?grace, "SIGTERM received; draining"),
         result = tokio::signal::ctrl_c() => {
             if result.is_ok() {
-                info!("interrupt received; draining");
+                info!(?grace, "interrupt received; draining");
             }
         }
     }
+
+    // Returning here only stops the listener accepting; axum then waits on
+    // every in-flight response, blob streams included. This watchdog is what
+    // makes that wait bounded.
+    tokio::spawn(async move {
+        tokio::time::sleep(grace).await;
+        tracing::warn!(
+            ?grace,
+            "in-flight responses did not finish within server.shutdown_grace; \
+             stopping anyway and cutting them. Clients retry; stored blobs are \
+             unaffected, because a manifest entry is only written once its blob \
+             is durable."
+        );
+        std::process::exit(0);
+    });
 }

doc/spec.md

@@ -226,6 +226,7 @@ config.
 listen         = "127.0.0.1:8080"
 external_url   = "https://models.internal.example"   # used to build redirect URLs
 client_stall_timeout = "60s"     # idle guard on the tee; see below
+shutdown_grace = "15s"           # bounded drain on SIGTERM; see below
 
 [storage]
 endpoint               = "https://s3.internal.example:9000"
@@ -282,6 +283,15 @@ the next attempt is any faster.
   hung backend still fails a small metadata request promptly.
 - `upstream.read_timeout` — the same guard on the Hub side.
 
+`server.shutdown_grace` is the one bound that is not idle-based, and
+deliberately so. It caps how long in-flight responses may drain on SIGTERM
+before the process stops regardless. A blob response is an in-flight request
+lasting `size / bandwidth`, so an unbounded drain does not converge: systemd's
+`TimeoutStopSec` expires first and escalates to `SIGABRT`. Cutting a download
+deliberately is the better failure — the client retries, and nothing in the
+bucket depends on the response, because a manifest entry is only ever written
+once its blob is durable.
+
 This applies to the object-storage client in particular. `object_store`
 defaults to a 30-second cap on the whole request, body included, and then hides
 its own consequences: it catches the resulting body errors and silently retries