You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by "merlimat (via GitHub)" <gi...@apache.org> on 2026/05/05 01:24:14 UTC
[PR] [improve][client] PIP-468: Make Backoff jitter configurable [pulsar]
merlimat opened a new pull request, #25669:
URL: https://github.com/apache/pulsar/pull/25669
### Motivation
The existing `Backoff` class always subtracts up to 10% jitter from the next delay, never adds, and skips jitter on the first invocation entirely (the result is clamped back up to `initialDelay`). When many clients restart together, the first reconnect attempt fires at the exact configured initial delay — defeating the point of jitter. The jitter percentage was also hard-coded.
### Modifications
- `Backoff` takes a configurable `jitterPercent` (default 10).
- Jitter is applied symmetrically as `±jitterPercent/2`: the underlying delay is multiplied by a uniform random factor in `[1 − jitterPercent/200, 1 + jitterPercent/200)`.
- Jitter is applied on every call, including the first. The previous `Math.max(initial, …)` clamp is removed; the returned value may now be slightly below `initialDelay` or slightly above `maxBackoff`.
- `BackoffPolicy` (v5 client API) gains a `jitterPercent` field with a `withJitter(double)` helper. Existing factory methods seed the default jitter so callers are unaffected.
The new `BackoffPolicy.jitterPercent` is currently API-shape only: the connection-backoff path is still TODO'd in `PulsarClientBuilderV5`, and the redelivery-backoff path goes through `MultiplierRedeliveryBackoff` which has no jitter support. Wiring is left for a follow-up.
### Verifying this change
This change added tests and can be verified as follows:
- New unit tests in `BackoffTest`:
- `jitterIsAppliedSymmetricallyOnFirstCall` — across 500 resets verifies values both above and below the base, all within `[base · 0.9, base · 1.1]` for `jitterPercent=20`.
- `jitterPercentZeroDisablesJitter` — verifies the disable path returns the exact base.
- `negativeJitterIsRejected` — validation.
- Existing tests updated to set `jitterPercent(0)` for deterministic exponential-progression and mandatory-stop checks.
### Does this pull request potentially affect one of the following parts:
- [x] The public API
`org.apache.pulsar.common.util.Backoff` and `org.apache.pulsar.client.api.v5.config.BackoffPolicy` gain new optional fields/builder methods. Existing callers are unaffected (defaults preserve the 10% magnitude, though the direction now spans `±5%` instead of `−10%`).
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [improve][client] PIP-468: Make Backoff jitter configurable [pulsar]
Posted by "merlimat (via GitHub)" <gi...@apache.org>.
merlimat merged PR #25669:
URL: https://github.com/apache/pulsar/pull/25669
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [improve][client] PIP-468: Make Backoff jitter configurable [pulsar]
Posted by "merlimat (via GitHub)" <gi...@apache.org>.
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [improve][client] PIP-468: Make Backoff jitter configurable [pulsar]
Posted by "lhotari (via GitHub)" <gi...@apache.org>.
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org