Inconsistent validation of exponential backoff

[aahlenst]

setMultiplier() in ExponentialBackoffPolicy allows a minimum multiplier of 1.0. However, the corresponding builder method in RestTemplateBuilder insists on a minimum multiplier greater than 1.0. There's even a test for it.

That's confusing. While it makes sense to require that the multiplier is greater than 1.0, aligning the setter with the builder might break existing usages.

[artembilan]

aligning the setter with the builder might break existing usages.

And since we agree that existing usage is wrong, it has to be fixed.
However we cannot make a breaking change like that in the patch version, but what we can is to show a warning when the value in that setter is <= 1.0.
This way end-user will know that something is off and respective part of his/her project has to be fixed.
And it will be fixed on our side next version - 2.1 or 3.0.

Does this sound like a plan?

[aahlenst]

I looked at the entire ExponentialBackoffPolicy. The pattern used by all setters is not to validate the arguments and to throw an exception if the argument is invalid but to adjust it silently. That pattern is used throughout the package. Deviating from it in a single setter feels wrong. An alternative would be to change RetryTemplateBuilder. However, the builder works differently. It validates the values and does not coerce them into the acceptable range. So maybe leaving it as it is is the more sensible approach.

[artembilan]

I think the pattern to "silently adjust" is wrong.
Many would be confused to not see the value they have configured, even if that is there for years already.
I believe it is better to say what is wrong and why. This way the library and end-user would be on the same page for values awareness.
Therefore I like the RetryTemplateBuilder approach more.

So, if you are OK with warnings in those all setters for the current versions, then feel free to contribute.
Otherwise the behavior change to make code flow consistent will be done in the later and probably major version.

[ivamly]

Hi, @artembilan !

If this is still relevant, I’d like to take this issue and work on it. Please let me know.

Thanks!

[kssumin]

Hi, I'd like to work on this issue. I understand the problem:

• setMultiplier() in ExponentialBackoffPolicy allows a minimum multiplier of 1.0
• RetryTemplateBuilder.multiplier() requires a minimum multiplier > 1.0

As mentioned in the discussion, I'll implement warnings for invalid values in setters for the current version. The approach would be:

1. Add warning logs when setter values are invalid (e.g., multiplier <= 1.0)
2. Keep the current behavior to avoid breaking changes
3. Ensure consistency across all setters in the package

May I proceed with this implementation?