-
Notifications
You must be signed in to change notification settings - Fork 911
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
Use generics in jitter #3717
Use generics in jitter #3717
Conversation
common/backoff/jitter.go
Outdated
return base + addon | ||
var base float64 | ||
var addon float64 | ||
switch i := any(input).(type) { |
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.
Do you know if the compiler is smart enough to compile this into separate functions that don't require a runtime type switch?
Ok, I tried it on compiler explorer and it looks like the answer is no: it makes two separate versions and still has a lot of logic around the type switch
Co-authored-by: David Reiss <[email protected]>
Co-authored-by: David Reiss <[email protected]>
@@ -198,7 +198,7 @@ func (r *TaskGeneratorImpl) GenerateWorkflowCloseTasks( | |||
} | |||
// We schedule the archival task for a random time in the near future to avoid sending a surge of tasks | |||
// to the archival system at the same time | |||
delay := backoff.JitDuration(r.config.ArchivalProcessorArchiveDelay(), archivalDelayJitterCoefficient) / 2 | |||
delay := backoff.Jitter(r.config.ArchivalProcessorArchiveDelay(), archivalDelayJitterCoefficient) / 2 |
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.
FullJitter?
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.
Intentionally skip this because of test verify the jitter. I will see if I can use the full jitter and have the test pass.
What changed?
Use generics in jitter
Why?
Use generics in jitter
How did you test it?
Potential risks
Is hotfix candidate?