Skip to content

Commit

Permalink
Increase debuginfo upload timeout (#149)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
javierhonduco authored Jan 30, 2025
1 parent 7807241 commit da6d0d2
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 13 deletions.
7 changes: 4 additions & 3 deletions src/cli/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
39 changes: 30 additions & 9 deletions src/debug_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(),
Expand All @@ -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(),
Expand All @@ -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(())
}
}
12 changes: 11 additions & 1 deletion src/profiler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {}",
Expand Down

0 comments on commit da6d0d2

Please sign in to comment.