Skip to content

Commit

Permalink
address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
kwannoel committed Jan 23, 2024
1 parent bc7f41d commit 7e27067
Show file tree
Hide file tree
Showing 5 changed files with 30 additions and 13 deletions.
4 changes: 2 additions & 2 deletions Makefile.toml
Original file line number Diff line number Diff line change
Expand Up @@ -1305,10 +1305,10 @@ command = "target/${BUILD_MODE_DIR}/risedev-dev"
args = ["${@}"]
description = "Clean data and start a full RisingWave dev cluster using risedev-dev"

[tasks.ci-kill-no-logs]
[tasks.ci-kill-no-dump-logs]
category = "RiseDev - CI"
dependencies = ["k", "check-logs", "wait-processes-exit"]
description = "Kill cluster, dump logs and check logs"
description = "Kill cluster and check logs, do not dump logs"

[tasks.ci-kill]
category = "RiseDev - CI"
Expand Down
2 changes: 1 addition & 1 deletion ci/scripts/run-backfill-tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ rename_logs_with_prefix() {
}

kill_cluster() {
cargo make ci-kill-no-logs
cargo make ci-kill-no-dump-logs
wait
}

Expand Down
29 changes: 22 additions & 7 deletions src/common/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -921,10 +921,23 @@ pub struct S3ObjectStoreConfig {
pub object_store_req_retry_max_delay_ms: u64,
#[serde(default = "default::object_store_config::s3::object_store_req_retry_max_attempts")]
pub object_store_req_retry_max_attempts: usize,
#[serde(default)]
pub developer: S3ObjectStoreDeveloperConfig,
}

/// The subsections `[storage.object_store.s3.developer]`.
#[derive(Clone, Debug, Serialize, Deserialize, DefaultFromSerde)]
pub struct S3ObjectStoreDeveloperConfig {
/// Whether to retry s3 sdk error from which no error metadata is provided.
#[serde(default = "default::object_store_config::s3::retry_unknown_service_error")]
#[serde(
default = "default::object_store_config::s3::developer::object_store_retry_unknown_service_error"
)]
pub retry_unknown_service_error: bool,
#[serde(default = "default::object_store_config::s3::retryable_service_error_codes")]
/// An array of error codes that should be retried.
/// e.g. `["SlowDown", "TooManyRequests"]`
#[serde(
default = "default::object_store_config::s3::developer::object_store_retryable_service_error_codes"
)]
pub retryable_service_error_codes: Vec<String>,
}

Expand Down Expand Up @@ -1528,12 +1541,14 @@ pub mod default {
DEFAULT_RETRY_MAX_ATTEMPTS
}

pub fn retry_unknown_service_error() -> bool {
false
}
pub mod developer {
pub fn object_store_retry_unknown_service_error() -> bool {
false
}

pub fn retryable_service_error_codes() -> Vec<String> {
vec!["SlowDown".into(), "TooManyRequests".into()]
pub fn object_store_retryable_service_error_codes() -> Vec<String> {
vec!["SlowDown".into(), "TooManyRequests".into()]
}
}
}
}
Expand Down
2 changes: 2 additions & 0 deletions src/config/example.toml
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,8 @@ object_store_nodelay = true
object_store_req_retry_interval_ms = 20
object_store_req_retry_max_delay_ms = 10000
object_store_req_retry_max_attempts = 8

[storage.object_store.s3.developer]
retry_unknown_service_error = false
retryable_service_error_codes = ["SlowDown", "TooManyRequests"]

Expand Down
6 changes: 3 additions & 3 deletions src/object_store/src/object/s3.rs
Original file line number Diff line number Diff line change
Expand Up @@ -944,8 +944,8 @@ struct RetryCondition {
impl RetryCondition {
fn new(config: &S3ObjectStoreConfig) -> Self {
Self {
retry_unknown_service_error: config.retry_unknown_service_error,
retryable_service_error_codes: config.retryable_service_error_codes.clone(),
retry_unknown_service_error: config.developer.retry_unknown_service_error,
retryable_service_error_codes: config.developer.retryable_service_error_codes.clone(),
}
}
}
Expand All @@ -969,7 +969,7 @@ impl tokio_retry::Condition<RetryError> for RetryCondition {
if self
.retryable_service_error_codes
.iter()
.any(|s| s.as_str() == code)
.any(|s| s.as_str().eq_ignore_ascii_case(code))
{
tracing::warn!(target: "retryable_service_error", "{e:?} occurs, retry S3 get_object request.");
return true;
Expand Down

0 comments on commit 7e27067

Please sign in to comment.