-
Notifications
You must be signed in to change notification settings - Fork 68
fix: [Geneva Exporter] Add retries for ingestion info retrieval #442
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 3 commits
fd9eac7
e171095
a54322d
7d295df
7c963e3
2131aed
b7fe253
92bc8da
437eb36
49fbb6d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -93,6 +93,8 @@ pub(crate) enum GenevaConfigClientError { | |
#[allow(dead_code)] | ||
pub(crate) type Result<T> = std::result::Result<T, GenevaConfigClientError>; | ||
|
||
const MAX_GCS_RETRIES: usize = 5; | ||
|
||
/// Configuration for the Geneva Config Client. | ||
/// | ||
/// # Fields | ||
|
@@ -370,10 +372,23 @@ impl GenevaConfigClient { | |
} | ||
} | ||
} | ||
// Cache miss or expired token, fetch fresh data | ||
// Perform actual fetch before acquiring write lock to minimize lock contention | ||
let (fresh_ingestion_gateway_info, fresh_moniker_info) = | ||
self.fetch_ingestion_info().await?; | ||
// Cache miss or expired token, fetch fresh data with retry logic (up to 5 retries on retriable errors) | ||
let (fresh_ingestion_gateway_info, fresh_moniker_info) = { | ||
let mut attempt = 0; | ||
loop { | ||
match self.fetch_ingestion_info().await { | ||
Ok(v) => break v, | ||
Err(e) => { | ||
if attempt < MAX_GCS_RETRIES && is_retriable_error(&e) { | ||
attempt += 1; | ||
continue; | ||
} else { | ||
|
||
return Err(e); | ||
} | ||
} | ||
} | ||
} | ||
}; | ||
|
||
let token_expiry = | ||
Self::parse_token_expiry(&fresh_ingestion_gateway_info.auth_token_expiry_time) | ||
|
@@ -533,6 +548,16 @@ fn extract_endpoint_from_token(token: &str) -> Result<String> { | |
Ok(endpoint) | ||
} | ||
|
||
fn is_retriable_error(err: &GenevaConfigClientError) -> bool { | ||
match err { | ||
GenevaConfigClientError::Http(e) => e.is_timeout() || e.is_connect(), | ||
GenevaConfigClientError::RequestFailed { status, .. } => { | ||
(*status >= 500 && *status < 600) || *status == 429 | ||
} | ||
_ => false, | ||
} | ||
} | ||
|
||
#[cfg(feature = "self_signed_certs")] | ||
fn configure_tls_connector( | ||
mut builder: native_tls::TlsConnectorBuilder, | ||
|
@@ -558,3 +583,46 @@ fn configure_tls_connector( | |
.max_protocol_version(Some(Protocol::Tlsv12)); | ||
builder | ||
} | ||
|
||
#[cfg(test)] | ||
mod retry_tests { | ||
use super::*; | ||
|
||
#[test] | ||
fn test_is_retriable_request_failed_statuses() { | ||
// 500 range | ||
for status in [500u16, 502, 503, 504, 599] { | ||
let err = GenevaConfigClientError::RequestFailed { | ||
status, | ||
message: String::new(), | ||
}; | ||
assert!( | ||
super::is_retriable_error(&err), | ||
"status {status} should be retriable" | ||
); | ||
} | ||
// 429 | ||
let err_429 = GenevaConfigClientError::RequestFailed { | ||
status: 429, | ||
message: String::new(), | ||
}; | ||
assert!( | ||
super::is_retriable_error(&err_429), | ||
"status 429 should be retriable" | ||
); | ||
} | ||
|
||
#[test] | ||
fn test_is_not_retriable_request_failed_statuses() { | ||
for status in [400u16, 401, 403, 404, 422] { | ||
let err = GenevaConfigClientError::RequestFailed { | ||
status, | ||
message: String::new(), | ||
}; | ||
assert!( | ||
!super::is_retriable_error(&err), | ||
"status {status} should NOT be retriable" | ||
); | ||
} | ||
} | ||
} |
Uh oh!
There was an error while loading. Please reload this page.