Skip to content

Commit

Permalink
Revert "ref(download): Retry downloads to S3/GCS/HTTP/local FS sources (
Browse files Browse the repository at this point in the history
#485)"

This reverts commit f8ac705.
  • Loading branch information
relaxolotl committed Jul 19, 2021
1 parent 1ae6331 commit f1a6858
Show file tree
Hide file tree
Showing 5 changed files with 66 additions and 54 deletions.
1 change: 0 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
- Introduced the `max_download_timeout` config setting for source downloads. ([#482](https://github.com/getsentry/symbolicator/pull/482), [#489](https://github.com/getsentry/symbolicator/pull/489))
- Introduced the `streaming_timeout` config setting for source downloads. ([#489](https://github.com/getsentry/symbolicator/pull/489))
- Introduced the `connect_timeout` config setting for source downloads. ([#491](https://github.com/getsentry/symbolicator/pull/491))
- GCS, S3, HTTP, and local filesystem sources: Attempt to retry failed downloads at least once. ([#485](https://github.com/getsentry/symbolicator/pull/485))
- Refresh symcaches when a new `BcSymbolMap` becomes available. ([#493](https://github.com/getsentry/symbolicator/pull/493))

### Tools
Expand Down
17 changes: 10 additions & 7 deletions crates/symbolicator/src/services/download/gcs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ use super::locations::SourceLocation;
use super::{content_length_timeout, DownloadError, DownloadStatus, RemoteDif, RemoteDifUri};
use crate::sources::{FileType, GcsSourceConfig, GcsSourceKey};
use crate::types::ObjectId;
use crate::utils::futures as future_utils;

/// An LRU cache for GCS OAuth tokens.
type GcsTokenCache = lru::LruCache<Arc<GcsSourceKey>, Arc<GcsToken>>;
Expand Down Expand Up @@ -224,13 +225,15 @@ impl GcsDownloader {
.extend(&[&bucket, "o", &key]);

let source = RemoteDif::from(file_source);
let request = self
.client
.get(url.clone())
.header("authorization", format!("Bearer {}", token.access_token))
.send();
let request = tokio::time::timeout(self.connect_timeout, request);
let request = super::measure_download_time(source.source_metric_key(), request);
let request = future_utils::retry(|| {
let request = self
.client
.get(url.clone())
.header("authorization", format!("Bearer {}", token.access_token))
.send();
let request = tokio::time::timeout(self.connect_timeout, request);
super::measure_download_time(source.source_metric_key(), request)
});

match request.await {
Ok(Ok(response)) => {
Expand Down
22 changes: 13 additions & 9 deletions crates/symbolicator/src/services/download/http.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ use super::{
};
use crate::sources::{FileType, HttpSourceConfig};
use crate::types::ObjectId;
use crate::utils::futures as future_utils;

/// The HTTP-specific [`RemoteDif`].
#[derive(Debug, Clone)]
Expand Down Expand Up @@ -75,19 +76,22 @@ impl HttpDownloader {
Ok(x) => x,
Err(_) => return Ok(DownloadStatus::NotFound),
};
let headers = file_source.source.headers.clone();
let source = RemoteDif::from(file_source);

log::debug!("Fetching debug file from {}", download_url);
let mut builder = self.client.get(download_url.clone());
let request = future_utils::retry(|| {
let mut builder = self.client.get(download_url.clone());

for (key, value) in file_source.source.headers.iter() {
if let Ok(key) = header::HeaderName::from_bytes(key.as_bytes()) {
builder = builder.header(key, value.as_str());
for (key, value) in headers.iter() {
if let Ok(key) = header::HeaderName::from_bytes(key.as_bytes()) {
builder = builder.header(key, value.as_str());
}
}
}
let source = RemoteDif::from(file_source);
let request = builder.header(header::USER_AGENT, USER_AGENT).send();
let request = tokio::time::timeout(self.connect_timeout, request);
let request = super::measure_download_time(source.source_metric_key(), request);
let request = builder.header(header::USER_AGENT, USER_AGENT).send();
let request = tokio::time::timeout(self.connect_timeout, request);
super::measure_download_time(source.source_metric_key(), request)
});

match request.await {
Ok(Ok(response)) => {
Expand Down
44 changes: 7 additions & 37 deletions crates/symbolicator/src/services/download/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use thiserror::Error;
use tokio::fs::File;
use tokio::io::AsyncWriteExt;

use crate::utils::futures::{self as future_utils, m, measure};
use crate::utils::futures::{m, measure};
use crate::utils::paths::get_directory_paths;

mod filesystem;
Expand Down Expand Up @@ -111,42 +111,12 @@ impl DownloadService {
source: RemoteDif,
destination: PathBuf,
) -> Result<DownloadStatus, DownloadError> {
let result = future_utils::retry(|| async {
let destination = destination.clone();
match &source {
RemoteDif::Sentry(inner) => {
self.sentry
.download_source(inner.clone(), destination)
.await
}
RemoteDif::Http(inner) => {
self.http.download_source(inner.clone(), destination).await
}
RemoteDif::S3(inner) => self.s3.download_source(inner.clone(), destination).await,
RemoteDif::Gcs(inner) => self.gcs.download_source(inner.clone(), destination).await,
RemoteDif::Filesystem(inner) => {
self.fs.download_source(inner.clone(), destination).await
}
}
});

match result.await {
Ok(DownloadStatus::NotFound) => {
log::debug!(
"Did not fetch debug file from {:?}: {:?}",
source,
DownloadStatus::NotFound
);
Ok(DownloadStatus::NotFound)
}
Ok(status) => {
log::debug!("Fetched debug file from {:?}: {:?}", source, status);
Ok(status)
}
Err(err) => {
log::debug!("Failed to fetch debug file from {:?}: {}", source, err);
Err(err)
}
match source {
RemoteDif::Sentry(inner) => self.sentry.download_source(inner, destination).await,
RemoteDif::Http(inner) => self.http.download_source(inner, destination).await,
RemoteDif::S3(inner) => self.s3.download_source(inner, destination).await,
RemoteDif::Gcs(inner) => self.gcs.download_source(inner, destination).await,
RemoteDif::Filesystem(inner) => self.fs.download_source(inner, destination).await,
}
}

Expand Down
36 changes: 36 additions & 0 deletions crates/symbolicator/src/services/download/sentry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,42 @@ impl SentryDownloader {
&self,
file_source: SentryRemoteDif,
destination: PathBuf,
) -> Result<DownloadStatus, DownloadError> {
let retries = future_utils::retry(|| {
self.download_source_once(file_source.clone(), destination.clone())
});
match retries.await {
Ok(DownloadStatus::NotFound) => {
log::debug!(
"Did not fetch debug file from {:?}: {:?}",
file_source.url(),
DownloadStatus::NotFound
);
Ok(DownloadStatus::NotFound)
}
Ok(status) => {
log::debug!(
"Fetched debug file from {:?}: {:?}",
file_source.url(),
status
);
Ok(status)
}
Err(err) => {
log::debug!(
"Failed to fetch debug file from {:?}: {}",
file_source.url(),
err
);
Err(err)
}
}
}

async fn download_source_once(
&self,
file_source: SentryRemoteDif,
destination: PathBuf,
) -> Result<DownloadStatus, DownloadError> {
let request = self
.client
Expand Down

0 comments on commit f1a6858

Please sign in to comment.