From 260b0df922d4c38902887c9dc6a90094413a1bed Mon Sep 17 00:00:00 2001 From: William Smith Date: Fri, 20 Sep 2024 22:30:37 -0400 Subject: [PATCH] [formal snapshots] Fix retry logic in GCS and improve logging (#19470) ## Description Describe the changes or additions included in this PR. ## Test plan How did you test the new or updated feature? --- ## Release notes Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required. For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates. - [ ] Protocol: - [ ] Nodes (Validators and Full nodes): - [ ] Indexer: - [ ] JSON-RPC: - [ ] GraphQL: - [ ] CLI: - [ ] Rust SDK: - [ ] REST API: --- crates/sui-config/src/object_storage_config.rs | 10 +++++++--- crates/sui-snapshot/src/reader.rs | 2 +- crates/sui-storage/src/object_store/http/gcs.rs | 2 +- crates/sui-storage/src/object_store/mod.rs | 11 +++++++++-- 4 files changed, 18 insertions(+), 7 deletions(-) diff --git a/crates/sui-config/src/object_storage_config.rs b/crates/sui-config/src/object_storage_config.rs index 7bdf75defe4e3..1c1a38b735b44 100644 --- a/crates/sui-config/src/object_storage_config.rs +++ b/crates/sui-config/src/object_storage_config.rs @@ -182,6 +182,11 @@ impl ObjectStoreConfig { if let Some(account) = &self.google_service_account { builder = builder.with_service_account_path(account); } + + let mut client_options = ClientOptions::new() + .with_timeout_disabled() + .with_connect_timeout_disabled() + .with_pool_idle_timeout(std::time::Duration::from_secs(300)); if let Some(google_project_id) = &self.google_project_id { let x_project_header = HeaderName::from_static("x-goog-user-project"); let iam_req_header = HeaderName::from_static("userproject"); @@ -189,10 +194,9 @@ impl ObjectStoreConfig { let mut headers = HeaderMap::new(); headers.insert(x_project_header, HeaderValue::from_str(google_project_id)?); headers.insert(iam_req_header, HeaderValue::from_str(google_project_id)?); - - builder = - builder.with_client_options(ClientOptions::new().with_default_headers(headers)); + client_options = client_options.with_default_headers(headers); } + builder = builder.with_client_options(client_options); Ok(Arc::new(LimitStore::new( builder.build().context("Invalid gcs config")?, diff --git a/crates/sui-snapshot/src/reader.rs b/crates/sui-snapshot/src/reader.rs index 8e15b9e4c208b..bdb4cd35f989c 100644 --- a/crates/sui-snapshot/src/reader.rs +++ b/crates/sui-snapshot/src/reader.rs @@ -522,7 +522,7 @@ pub async fn download_bytes( part_num: &u32, max_timeout_secs: Option, ) -> (Bytes, [u8; 32]) { - let max_timeout = Duration::from_secs(max_timeout_secs.unwrap_or(30)); + let max_timeout = Duration::from_secs(max_timeout_secs.unwrap_or(60)); let mut timeout = Duration::from_secs(2); timeout += timeout / 2; timeout = std::cmp::min(max_timeout, timeout); diff --git a/crates/sui-storage/src/object_store/http/gcs.rs b/crates/sui-storage/src/object_store/http/gcs.rs index 09e8c587004da..372167e372b87 100644 --- a/crates/sui-storage/src/object_store/http/gcs.rs +++ b/crates/sui-storage/src/object_store/http/gcs.rs @@ -22,7 +22,7 @@ struct GoogleCloudStorageClient { impl GoogleCloudStorageClient { pub fn new(bucket: &str) -> Result { - let mut builder = ClientBuilder::new(); + let mut builder = ClientBuilder::new().pool_idle_timeout(None); builder = builder.user_agent(DEFAULT_USER_AGENT); let client = builder.https_only(false).build()?; let bucket_name_encoded = percent_encode(bucket.as_bytes(), NON_ALPHANUMERIC).to_string(); diff --git a/crates/sui-storage/src/object_store/mod.rs b/crates/sui-storage/src/object_store/mod.rs index 1ebfa595af66c..e92f4000abbc1 100644 --- a/crates/sui-storage/src/object_store/mod.rs +++ b/crates/sui-storage/src/object_store/mod.rs @@ -36,10 +36,17 @@ as_ref_get_ext_impl!(Box); impl ObjectStoreGetExt for Arc { async fn get_bytes(&self, src: &Path) -> Result { self.get(src) - .await? + .await + .map_err(|e| anyhow!("Failed to get file {} with error: {:?}", src, e))? .bytes() .await - .map_err(|e| anyhow!("Failed to get file: {} with error: {}", src, e.to_string())) + .map_err(|e| { + anyhow!( + "Failed to collect GET result for file {} into bytes with error: {:?}", + src, + e + ) + }) } }