From da6d0d23e44de47aa25fcda2006c46215a94dff4 Mon Sep 17 00:00:00 2001
From: Javier Honduvilla Coto <javierhonduco@gmail.com>
Date: Thu, 30 Jan 2025 23:30:25 +0000
Subject: [PATCH] Increase debuginfo upload timeout (#149)

As it was too slow for larger files. This issue is now more apparent as
the file is read in chunks, which is good to reduce peak memory usage,
but might take slightly longer.

Also now the two http clients are created once, in the constructor, and
the timeout of the two calls are configurable separately.

Test Plan
=========

ci
---
 src/cli/main.rs   |  7 ++++---
 src/debug_info.rs | 39 ++++++++++++++++++++++++++++++---------
 src/profiler.rs   | 12 +++++++++++-
 3 files changed, 45 insertions(+), 13 deletions(-)

diff --git a/src/cli/main.rs b/src/cli/main.rs
index bd981ec..b6809c9 100644
--- a/src/cli/main.rs
+++ b/src/cli/main.rs
@@ -145,10 +145,11 @@ fn main() -> Result<(), Box<dyn Error>> {
         DebugInfoBackend::Copy => Box::new(DebugInfoBackendFilesystem {
             path: PathBuf::from("/tmp"),
         }),
-        DebugInfoBackend::Remote => Box::new(DebugInfoBackendRemote {
-            http_client_timeout: Duration::from_millis(500),
+        DebugInfoBackend::Remote => Box::new(DebugInfoBackendRemote::new(
             server_url,
-        }),
+            Duration::from_millis(500),
+            Duration::from_secs(15),
+        )?),
     };
 
     let profiler_config = ProfilerConfig {
diff --git a/src/debug_info.rs b/src/debug_info.rs
index 9b873b0..5069a25 100644
--- a/src/debug_info.rs
+++ b/src/debug_info.rs
@@ -4,6 +4,7 @@ use std::path::Path;
 use std::path::PathBuf;
 use std::time::Duration;
 
+use anyhow::anyhow;
 use reqwest::StatusCode;
 use tracing::instrument;
 
@@ -84,9 +85,29 @@ impl DebugInfoBackendFilesystem {
 
 #[derive(Debug)]
 pub struct DebugInfoBackendRemote {
-    pub http_client_timeout: Duration,
     pub server_url: String,
+    pub query_client: reqwest::blocking::Client,
+    pub upload_client: reqwest::blocking::Client,
 }
+
+impl DebugInfoBackendRemote {
+    pub fn new(
+        server_url: String,
+        query_timeout: Duration,
+        upload_timeout: Duration,
+    ) -> anyhow::Result<Self> {
+        Ok(DebugInfoBackendRemote {
+            server_url,
+            query_client: reqwest::blocking::Client::builder()
+                .timeout(query_timeout)
+                .build()?,
+            upload_client: reqwest::blocking::Client::builder()
+                .timeout(upload_timeout)
+                .build()?,
+        })
+    }
+}
+
 impl DebugInfoManager for DebugInfoBackendRemote {
     #[instrument(level = "debug")]
     fn add_if_not_present(
@@ -115,9 +136,8 @@ impl DebugInfoBackendRemote {
     /// Whether the backend knows about some debug information.
     #[instrument(level = "debug")]
     fn find_in_backend(&self, build_id: &BuildId) -> anyhow::Result<bool> {
-        let client_builder = reqwest::blocking::Client::builder().timeout(self.http_client_timeout);
-        let client = client_builder.build()?;
-        let response = client
+        let response = self
+            .query_client
             .get(format!(
                 "{}/debuginfo/{}",
                 self.server_url.clone(),
@@ -136,10 +156,8 @@ impl DebugInfoBackendRemote {
         build_id: &BuildId,
         debug_info: &Path,
     ) -> anyhow::Result<()> {
-        let client_builder = reqwest::blocking::Client::builder().timeout(self.http_client_timeout);
-        let client = client_builder.build()?;
-
-        let response = client
+        let response = self
+            .upload_client
             .post(format!(
                 "{}/debuginfo/new/{}/{}",
                 self.server_url.clone(),
@@ -148,7 +166,10 @@ impl DebugInfoBackendRemote {
             ))
             .body(File::open(debug_info)?)
             .send()?;
-        println!("wrote debug info to server {:?}", response);
+
+        if !response.status().is_success() {
+            return Err(anyhow!("debuginfo upload failed with {:?}", response));
+        }
         Ok(())
     }
 }
diff --git a/src/profiler.rs b/src/profiler.rs
index 311126f..10640cc 100644
--- a/src/profiler.rs
+++ b/src/profiler.rs
@@ -1635,7 +1635,17 @@ impl Profiler {
                         let res = self
                             .debug_info_manager
                             .add_if_not_present(&name, build_id, &abs_path);
-                        debug!("debug info manager add result {:?}", res);
+                        match res {
+                            Ok(_) => {
+                                debug!("debuginfo add_if_not_present succeded {:?}", res);
+                            }
+                            Err(e) => {
+                                error!(
+                                    "debuginfo add_if_not_present failed with: {}",
+                                    e.root_cause()
+                                );
+                            }
+                        }
                     } else {
                         debug!(
                             "could not find debug information for {}",