Denovo1998 commented on code in PR #25278:
URL: https://github.com/apache/pulsar/pull/25278#discussion_r2881267870
##########
pulsar-common/src/main/java/org/apache/pulsar/common/util/Backoff.java:
##########
@@ -19,108 +19,194 @@
package org.apache.pulsar.common.util;
import java.time.Clock;
+import java.time.Duration;
+import java.time.Instant;
import java.util.Random;
-import java.util.concurrent.TimeUnit;
-import lombok.Data;
+import lombok.Getter;
-// All variables are in TimeUnit millis by default
-@Data
+/**
+ * Exponential backoff with mandatory stop.
+ *
+ * <p>Delays start at {@code initialDelay} and double on every call to {@link
#next()}, up to
+ * {@code maxBackoff}. A random jitter of up to 10% is subtracted from each
value to avoid
+ * thundering-herd retries.
+ *
+ * <p>If a {@code mandatoryStop} duration is configured, the backoff tracks
wall-clock time from the
+ * first {@link #next()} call. Once the elapsed time plus the next delay would
exceed the mandatory
+ * stop, the delay is truncated so that the total does not exceed it, and
{@link #isMandatoryStopMade()}
+ * returns {@code true}. After the mandatory stop, backoff continues to grow
normally.
+ *
+ * <p>Use {@link #reset()} to restart the sequence from the initial delay.
+ *
+ * <pre>{@code
+ * Backoff backoff = Backoff.builder()
+ * .initialDelay(Duration.ofMillis(100))
+ * .maxBackoff(Duration.ofMinutes(1))
+ * .mandatoryStop(Duration.ofSeconds(30))
+ * .build();
+ *
+ * Duration delay = backoff.next();
+ * }</pre>
+ */
public class Backoff {
- public static final long DEFAULT_INTERVAL_IN_NANOSECONDS =
TimeUnit.MILLISECONDS.toNanos(100);
- public static final long MAX_BACKOFF_INTERVAL_NANOSECONDS =
TimeUnit.SECONDS.toNanos(30);
- private final long initial;
- private final long max;
- private final Clock clock;
- private long next;
- private long mandatoryStop;
+ private static final Duration DEFAULT_INITIAL_DELAY =
Duration.ofMillis(100);
+ private static final Duration DEFAULT_MAX_BACKOFF_INTERVAL =
Duration.ofSeconds(30);
+ private static final Random random = new Random();
- private long firstBackoffTimeInMillis;
- private boolean mandatoryStopMade = false;
+ @Getter
+ private final Duration initial;
+ @Getter
+ private final Duration max;
+ @Getter
+ private final Duration mandatoryStop;
+ private final Clock clock;
- private static final Random random = new Random();
+ private Duration next;
+ @Getter
+ private Instant firstBackoffTime;
+ @Getter
+ private boolean mandatoryStopMade;
- Backoff(long initial, TimeUnit unitInitial, long max, TimeUnit unitMax,
long mandatoryStop,
- TimeUnit unitMandatoryStop, Clock clock) {
- this.initial = unitInitial.toMillis(initial);
- this.max = unitMax.toMillis(max);
- if (initial == 0 && max == 0 && mandatoryStop == 0) {
+ private Backoff(Duration initial, Duration max, Duration mandatoryStop,
Clock clock) {
+ this.initial = initial;
+ this.max = max;
+ this.mandatoryStop = mandatoryStop;
+ this.next = initial;
+ this.clock = clock;
+ this.firstBackoffTime = Instant.EPOCH;
+ if (initial.isZero() && max.isZero() && mandatoryStop.isZero()) {
this.mandatoryStopMade = true;
}
- this.next = this.initial;
- this.mandatoryStop = unitMandatoryStop.toMillis(mandatoryStop);
- this.clock = clock;
- this.firstBackoffTimeInMillis = 0;
}
- public Backoff(long initial, TimeUnit unitInitial, long max, TimeUnit
unitMax, long mandatoryStop,
- TimeUnit unitMandatoryStop) {
- this(initial, unitInitial, max, unitMax, mandatoryStop,
unitMandatoryStop, Clock.systemDefaultZone());
+ /**
+ * Creates a new {@link Builder} with default settings.
+ *
+ * @return a new builder instance
+ */
+ public static Builder builder() {
+ return new Builder();
}
- public long next() {
- long current = this.next;
- if (current < max) {
- this.next = Math.min(this.next * 2, this.max);
+ /**
+ * Returns the next backoff delay, advancing the internal state.
+ *
+ * <p>The returned duration is never less than the initial delay and never
more than the max
+ * backoff. A random jitter of up to 10% is subtracted to spread out
concurrent retries.
+ *
+ * @return the delay to wait before the next retry attempt
+ */
+ public Duration next() {
+ Duration current = this.next;
+ if (current.compareTo(max) < 0) {
+ Duration doubled = this.next.multipliedBy(2);
+ this.next = doubled.compareTo(this.max) < 0 ? doubled : this.max;
}
// Check for mandatory stop
if (!mandatoryStopMade) {
- long now = clock.millis();
- long timeElapsedSinceFirstBackoff = 0;
- if (initial == current) {
- firstBackoffTimeInMillis = now;
+ Instant now = clock.instant();
+ Duration timeElapsedSinceFirstBackoff = Duration.ZERO;
+ if (initial.equals(current)) {
+ firstBackoffTime = now;
} else {
- timeElapsedSinceFirstBackoff = now - firstBackoffTimeInMillis;
+ timeElapsedSinceFirstBackoff =
Duration.between(firstBackoffTime, now);
}
Review Comment:
+1. I think I can mention a PR about this issue.
--
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]
