ExponentialRandomBackOffPolicy not always random

[bones418]

Hey all,

Great work on this project. Reaching out because I feel like the documentation is a bit unclear as to what happens in the following case:

@Backoff(delay = 2000, multiplier = 1.0, random = true))

You'd think you're introducing randomness here by setting random to true, but this is not the case. Refer to this line https://github.com/spring-projects/spring-retry/blob/v2.0.4/src/main/java/org/springframework/retry/backoff/ExponentialRandomBackOffPolicy.java#L79

next = (long) (next * (1 + r.nextFloat() * (getMultiplier() - 1)));

When multiplier is 1.0 that line of code basically boils down to next = next. This doesn't create randomness and allows a thundering herd problem to persist. At the very least, can the documentation be more clear that random should be used with multiplier values greater than 1.0. Even better, can multiplier = 1.0 and random = true just be flagged and raised as an invalid configuration?

Thanks!
John

[garyrussell]

Well, in order to randomize, you need a range of intervals (delay and maxDelay - or infinity when there is no max).

A multiplier of 1 turns any exponential back off into a fixed backoff.

I don't see a whole lot of point rejecting such a configuration, but feel free to submit a PR, but it could break existing users, so a log would be better.

[bones418]

Hmm. Documentation can be clearer though, yes? See https://github.com/spring-projects/spring-retry/blob/v2.0.4/src/main/java/org/springframework/retry/annotation/Backoff.java#L118

It specifically states that when multiplier is > 0 and random is set to true that it will introduce jitter. And this is not the case for multiplier = 1.0. And yes, if you put on your thinking cap and parse the rest of that paragraph, then you might realize that all values greater than 0.0 (other than 1.0) will introduce jitter. But I wonder how many folks have misinterpreted it.

[garyrussell]

Actually; it turns out that any multiplier <= 1.0 is coerced to 1.0.