Implement a 120s timeout in case lctl process is taking too long

whamcloud · Sep 6, 2024 · 33af655 · 33af655
1 parent fbb9f54
commit 33af655
Show file tree

Hide file tree

Showing 5 changed files with 35 additions and 4 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/lustrefs-exporter/Cargo.toml b/lustrefs-exporter/Cargo.toml
@@ -19,11 +19,13 @@ tokio = {workspace = true, features = [
   "rt-multi-thread",
   "macros",
   "process",
+  "time",
 ]}
 tokio-stream = "0.1.15"
 tower = {version = "0.4.13", features = ["timeout", "load-shed", "limit"]}
 tracing-subscriber = {workspace = true, features = ["env-filter"]}
 tracing.workspace = true
+wait-timeout = "0.2.0"
 
 [dev-dependencies]
 combine.workspace = true

diff --git a/lustrefs-exporter/src/jobstats.rs b/lustrefs-exporter/src/jobstats.rs
@@ -264,7 +264,7 @@ fn render_stat(
                     );
                 }
                 x => {
-                    tracing::debug!("Unhandled OST jobstats stats: {x}");
+                    // tracing::debug!("Unhandled OST jobstats stats: {x}");
                     continue;
                 }
             };

diff --git a/lustrefs-exporter/src/lib.rs b/lustrefs-exporter/src/lib.rs
@@ -40,6 +40,8 @@ pub enum Error {
     Utf8(#[from] std::str::Utf8Error),
     #[error("Could not find match for {0} in {1}")]
     NoCap(&'static str, String),
+    #[error(transparent)]
+    Timeout(#[from] tokio::time::error::Elapsed),
 }
 
 impl IntoResponse for Error {

diff --git a/lustrefs-exporter/src/main.rs b/lustrefs-exporter/src/main.rs
@@ -20,13 +20,17 @@ use std::{
     convert::Infallible,
     io::{self, BufRead, BufReader},
     net::SocketAddr,
+    time::Duration,
 };
 use tokio::process::Command;
 use tokio_stream::{wrappers::ReceiverStream, StreamExt};
 use tower::ServiceBuilder;
+use wait_timeout::ChildExt;
 
 const LUSTREFS_EXPORTER_PORT: &str = "32221";
 
+static TIMEOUT_DURATION: Duration = Duration::from_secs(120);
+
 #[derive(Debug, Parser)]
 pub struct CommandOpts {
     /// Port that exporter will listen to
@@ -81,14 +85,18 @@ async fn main() -> Result<(), Error> {
         .concurrency_limit(10); // Max 10 concurrent scrape
 
     let app = Router::new()
-        .route("/metrics", get(scrape))
+        .route("/metrics", get(outer_scrape))
         .layer(load_shedder);
 
     axum::serve(listener, app).await?;
 
     Ok(())
 }
 
+async fn outer_scrape(Query(params): Query<Params>) -> Result<Response<Body>, Error> {
+    tokio::time::timeout(TIMEOUT_DURATION, scrape(Query(params))).await?
+}
+
 async fn scrape(Query(params): Query<Params>) -> Result<Response<Body>, Error> {
     let jobstats = if params.jobstats {
         let child = tokio::task::spawn_blocking(move || {
@@ -126,8 +134,17 @@ async fn scrape(Query(params): Query<Params>) -> Result<Response<Body>, Error> {
 
                 let (_, rx) = lustrefs_exporter::jobstats::jobstats_stream(reader);
 
-                tokio::task::spawn_blocking(move || {
-                    if let Err(e) = child.wait() {
+                tokio::task::spawn_blocking(move || match child.wait_timeout(TIMEOUT_DURATION) {
+                    Ok(Some(status)) => {
+                        if !status.success() {
+                            tracing::debug!("lctl jobstats failed: {status}");
+                        }
+                    }
+                    Ok(None) => {
+                        tracing::debug!("lctl jobstats timed out");
+                        _ = child.kill();
+                    }
+                    Err(e) => {
                         tracing::debug!("Unexpected error when waiting for child: {e}");
                     }
                 });