Skip to content
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

max randomization factor can be 1, refactor randomization function #2109

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jdcaperon
Copy link

@jdcaperon jdcaperon commented Feb 7, 2024

TLDR: Nice to have but not important, this could have impacts on code that expects values returned from an interval function to be greater than zero.

This PR proposes two changes:

  1. Allow randomization factor to be 1.0.
  2. Refactor randomize() in IntervalFunction to remove +1 and to specify its contract.

Randomization factor to be 1.0

Largely cosmetic, however it does have the consequence of allowing interval functions to now return 0. I don't know enough about the rest of the codebase outside of Retry to know if this will have any negative consequences. If this is accepted I would also propose a new constructor ofExponentialFullRandomBackoff that defaults to a value of 1.0 for randomization. Full random or full jitter is generally a desirable characteristic for backoff functions: https://aws.amazon.com/blogs/architecture/exponential-backoff-and-jitter/

Refactor randomize()

Randomize currently includes a + 1 which seems out of place. This +1 has no effect on large intervals since it extends the range by an insignificant amount and when the interval is 0 it forces the randomize function to produce values between 0 and 1.

My proposal is to refactor randomize so that it produces values between 0.0 <= x < 2 * current.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant