-
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?
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #442 +/- ##
=====================================
Coverage 53.2% 53.2%
=====================================
Files 70 70
Lines 10648 10718 +70
=====================================
+ Hits 5665 5711 +46
- Misses 4983 5007 +24 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This PR doesn't fix that issue. It's only working on add retries for the ingestion info calls not the actual upload calls. We should either create a dedicate issue for this PR or use a different phrase such as "Towards" or "Partially Fixes" so that merging this PR doesn't automatically close the issue. |
Err(e) => { | ||
if attempt < MAX_GCS_RETRIES && is_retriable_error(&e) { | ||
attempt += 1; | ||
continue; | ||
} else { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to respect the Retry-After
header if present, before sending the request again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. I wasn't actually aware that such rate limiting option exists in HTTP, Added now.
Agree, done. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds robust retry logic to the Geneva Config Service client to improve resilience against transient network issues and server overload. The changes implement intelligent error handling with appropriate backoff strategies.
Key changes:
- Adds retry mechanism with up to 3 attempts for Geneva Config Service API calls
- Implements smart backoff strategy using server-provided Retry-After headers or fixed delays
- Introduces new error type for rate limiting scenarios with retry information
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
File | Description |
---|---|
client.rs | Implements retry logic, error handling, and backoff strategy for config service calls |
Cargo.toml | Adds tokio dependency for sleep functionality in retry implementation |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
towards: #309
Summary
This PR adds retry logic to the Geneva Config Service client to improve resilience against transient network issues and server overload. The changes implement intelligent error handling with appropriate backoff strategies.
Key changes:
Merge requirement checklist
CHANGELOG.md
files updated for non-trivial, user-facing changes