merlimat commented on code in PR #25669:
URL: https://github.com/apache/pulsar/pull/25669#discussion_r3191723569


##########
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:
   In theory it doesn't need to get capped at 100%, since you could fluctuate 
with >100%, though in practice it wouldn't make much sense for the backoff 
purpose.. let me add the check. 



-- 
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]

Reply via email to