lhotari commented on code in PR #25669:
URL: https://github.com/apache/pulsar/pull/25669#discussion_r3186973149
##########
pulsar-client-api-v5/src/main/java/org/apache/pulsar/client/api/v5/config/BackoffPolicy.java:
##########
@@ -20,136 +20,84 @@
import java.time.Duration;
import java.util.Objects;
-import lombok.EqualsAndHashCode;
-import lombok.ToString;
/**
* Backoff configuration for broker reconnection attempts.
*
- * <p>The delay for attempt {@code n} is {@code min(initialInterval *
multiplier^(n-1), maxInterval)}.
+ * <p>The base delay for attempt {@code n} is {@code min(initialInterval *
multiplier^(n-1), maxInterval)}.
+ * A symmetric random jitter of {@code ±jitterPercent/2} is applied to each
delay (including the
+ * first one) to spread out concurrent retries.
*
- * <p>Use {@link #fixed(Duration, Duration)} or {@link #exponential(Duration,
Duration)} for
- * the common cases, or {@link #builder()} to configure all knobs explicitly.
+ * @param initialInterval the delay before the first reconnection attempt
+ * @param maxInterval the maximum delay between reconnection attempts
+ * @param multiplier the multiplier applied after each attempt
+ * @param jitterPercent the symmetric jitter percentage applied to each
delay; {@code 0} disables jitter
*/
-@EqualsAndHashCode
-@ToString
-public final class BackoffPolicy {
-
- private final Duration initialInterval;
- private final Duration maxInterval;
- private final double multiplier;
-
- private BackoffPolicy(Duration initialInterval, Duration maxInterval,
double multiplier) {
+public record BackoffPolicy(
+ Duration initialInterval,
+ Duration maxInterval,
+ double multiplier,
+ double jitterPercent
+) {
+ /** Default jitter percentage applied when not explicitly specified. */
+ public static final double DEFAULT_JITTER_PERCENT = 10.0;
+
+ public BackoffPolicy {
Objects.requireNonNull(initialInterval, "initialInterval must not be
null");
Objects.requireNonNull(maxInterval, "maxInterval must not be null");
if (multiplier < 1.0) {
throw new IllegalArgumentException("multiplier must be >= 1.0");
}
- this.initialInterval = initialInterval;
- this.maxInterval = maxInterval;
- this.multiplier = multiplier;
- }
-
- /**
- * @return the delay before the first reconnection attempt
- */
- public Duration initialInterval() {
- return initialInterval;
- }
-
- /**
- * @return the maximum delay between reconnection attempts
- */
- public Duration maxInterval() {
- return maxInterval;
- }
-
- /**
- * @return the multiplier applied after each attempt
- */
- public double multiplier() {
- return multiplier;
+ if (jitterPercent < 0) {
+ throw new IllegalArgumentException("jitterPercent must be >= 0");
+ }
Review Comment:
Should we limit jitterPercent to 100?
##########
pulsar-common/src/main/java/org/apache/pulsar/common/util/Backoff.java:
##########
@@ -205,6 +213,24 @@ public Builder mandatoryStop(Duration mandatoryStop) {
return this;
}
+ /**
+ * Sets the jitter percentage applied to each returned delay. The
actual jitter is symmetric:
+ * the returned value is multiplied by a uniform random factor in
+ * {@code [1 - jitterPercent/200, 1 + jitterPercent/200)}. Defaults to
10. Set to 0 to disable
+ * jitter.
+ *
+ * @param jitterPercent the jitter percentage, must be {@code >= 0}
+ * @return this builder
+ * @throws IllegalArgumentException if {@code jitterPercent} is
negative
+ */
+ public Builder jitterPercent(double jitterPercent) {
+ if (jitterPercent < 0) {
+ throw new IllegalArgumentException("jitterPercent must be >=
0");
+ }
Review Comment:
Should we limit jitterPercent to 100?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
