minor refactor

compute_tools/src/extension_server.rs

@@ -118,12 +118,15 @@ pub async fn get_available_extensions(
     let index_path = RemotePath::new(Path::new(&index_path)).context("error forming path")?;
     info!("download ext_index.json from: {:?}", &index_path);
 
-    let mut download = better_download(remote_storage, &index_path).await?;
+    // TODO: this download is ~3KB. but it takes like 400ms. why? later
+    // downloads which are much larger take much less time
+    let mut download = remote_storage.download(&index_path).await?;
     let mut ext_idx_buffer = Vec::new();
     download
         .download_stream
         .read_to_end(&mut ext_idx_buffer)
         .await?;
+    info!("ext_index downloaded");
 
     #[derive(Debug, serde::Deserialize)]
     struct Index {
@@ -180,7 +183,7 @@ pub async fn download_extension(
     pgbin: &str,
 ) -> Result<u64> {
     info!("Download extension {:?} from {:?}", ext_name, ext_path);
-    let mut download = better_download(remote_storage, ext_path).await?;
+    let mut download = remote_storage.download(ext_path).await?;
     let mut download_buffer = Vec::new();
     download
         .download_stream

libs/remote_storage/src/lib.rs

@@ -24,7 +24,6 @@ use tokio::io;
 use toml_edit::Item;
 use tracing::info;
 
-pub use self::s3_bucket::better_download;
 pub use self::{local_fs::LocalFs, s3_bucket::S3Bucket, simulate_failures::UnreliableWrapper};
 
 /// How many different timelines can be processed simultaneously when synchronizing layers with the remote storage.

libs/remote_storage/src/s3_bucket.rs

@@ -31,8 +31,7 @@ use tracing::debug;
 
 use super::StorageMetadata;
 use crate::{
-    Download, DownloadError, GenericRemoteStorage, RemotePath, RemoteStorage, S3Config,
-    REMOTE_STORAGE_PREFIX_SEPARATOR,
+    Download, DownloadError, RemotePath, RemoteStorage, S3Config, REMOTE_STORAGE_PREFIX_SEPARATOR,
 };
 
 const MAX_DELETE_OBJECTS_REQUEST_SIZE: usize = 1000;
@@ -133,38 +132,6 @@ struct GetObjectRequest {
     range: Option<String>,
 }
 
-use crate::GenericRemoteStorage::AwsS3;
-// the regular download function adds a "/" to the start of file names in the
-// case of prefix="None", which breaks everything. Thus, the following function is necessary
-pub async fn better_download(
-    bucket: &GenericRemoteStorage,
-    from: &RemotePath,
-) -> Result<Download, DownloadError> {
-    if let AwsS3(bucket) = bucket {
-        // this is more expected behavior.
-        // prefix="" should result in a trailing slash
-        // wheras prefix=None should **NOT** result in a trailing slash
-        let query_key = match &bucket.prefix_in_bucket {
-            Some(_) => bucket.relative_path_to_s3_object(from),
-            None => from
-                .get_path()
-                .to_str()
-                .expect("bad object name")
-                .to_string(),
-        };
-
-        bucket
-            .download_object(GetObjectRequest {
-                bucket: bucket.bucket_name.clone(),
-                key: query_key,
-                range: None,
-            })
-            .await
-    } else {
-        panic!("this isn't supposed to happen");
-    }
-}
-
 impl S3Bucket {
     /// Creates the S3 storage, errors if incorrect AWS S3 configuration provided.
     pub fn new(aws_config: &S3Config) -> anyhow::Result<Self> {