-
Notifications
You must be signed in to change notification settings - Fork 391
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
feat: optimize jitter factor calculation #3629
Conversation
🛠 PR Checks SummaryAll Automated Checks passed. ✅ Manual Checks (for Reviewers):
Read More🤖 This bot helps streamline PR reviews by verifying automated checks and providing guidance for contributors and reviewers. ✅ Automated Checks (for Contributors):🟢 Maintainers must be able to edit this pull request (more info) ☑️ Contributor Actions:
☑️ Reviewer Actions:
📚 Resources:Debug
|
Codecov ReportAttention: Patch coverage is
📢 Thoughts on this report? Let us know! |
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.
Clean 👌
#0: 931.627978ms
#1: 2.174257897s
#2: 3.757571214s
#3: 7.751274703s
#4: 15.326207363s
#5: 32.481973117s
#6: 1m5.236442173s
#7: 2m5.88950285s
#8: 4m7.178974447s
#9: 8m25.42811194s
#10: 10m1.165531942s
#11: 9m51.467254434s
#12: 10m1.91409499s
#13: 9m54.044129573s
#14: 9m54.655958142s
#15: 9m52.082631822s
#16: 10m5.809831228s
#17: 9m55.408970261s
#18: 10m0.820427954s
#19: 9m50.776101942s
#20: 9m54.162680995s
#21: 9m55.871490743s
#22: 9m50.257767184s
#23: 9m58.557184237s
#24: 10m6.159298462s
#25: 10m9.869970109s
#26: 10m9.026321693s
#27: 9m58.270476462s
#28: 9m52.066430181s
#29: 10m5.545603695s
@aeddi |
f4a0ed3
to
8f4daff
Compare
I added tests that cover pretty much all the cases I think. The only line not covered is when there's an error reading from |
LGTM |
This PR modifies the backoff calculation in two ways:
The jitter factor is now calculated based on a percentage of the interval and then added to the interval. Specifically, the jitter will be plus or minus 10% of the interval duration (so in the case of a 1s interval, the jitter will be a random duration between -0.1s and +0.1s, and the sum will therefore be a duration between 0.9s and 1.1s). The jitter is capped at a maximum of 10s (so in the case of a 5m interval, the jitter will be a random duration between -10s and +10s, and the sum will therefore be a duration between 4m50s and 5m10s).
The jitter factor is applied to the interval even when it has been capped at the
maxInterval
. Therefore, the following call calculateBackoff(10, time.Second, 10*time.Minute)
will return a duration between 9m50s and 10m10s instead of consistently returning 10m as before.Fixes #3621.