-
Notifications
You must be signed in to change notification settings - Fork 454
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
[m3msg] Increase default writer initial backoff #3820
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3820 +/- ##
========================================
- Coverage 57.1% 56.8% -0.4%
========================================
Files 552 552
Lines 63177 63081 -96
========================================
- Hits 36115 35860 -255
- Misses 23876 24020 +144
- Partials 3186 3201 +15
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report at Codecov.
|
src/x/retry/config.go
Outdated
@@ -29,6 +29,7 @@ import ( | |||
// Configuration configures options for retry attempts. | |||
type Configuration struct { | |||
// Initial retry backoff. | |||
// Defaults to 5s if unspecified. |
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.
Nit: no need for this comment, eventually it would go out of sync with the actual value.
m3msg producer retry options is one of the most confusing pieces of config around, thanks for the PR! |
What this PR does / why we need it:
Increases initial retry backoff from 1s to 5s for all M3Msg message writers.
Special notes for your reviewer:
Increasing initial backoff to 5s reduced M3Msg retries on all of our workloads to almost 0. Under normal conditions retries should be close to 0. On certain workloads we also saw CPU usage reduction in M3 Aggregator.
Motivation to set a new default is mainly because with current setting all clusters will experience more retries than needed.
The following metrics can be used to inspect if retries are happening:
Does this PR introduce a user-facing and/or backwards incompatible change?:
Does this PR require updating code package or user-facing documentation?: