Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/1924#discussion_r99637679
--- Diff:
external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/KafkaSpoutRetryExponentialBackoff.java
---
@@ -27,13 +27,15 @@
import java.util.Comparator;
import java.util.HashSet;
import java.util.Iterator;
+import java.util.Objects;
import java.util.Set;
import java.util.TreeSet;
import java.util.concurrent.TimeUnit;
+import org.apache.storm.utils.Time;
/**
* Implementation of {@link KafkaSpoutRetryService} using the exponential
backoff formula. The time of the nextRetry is set as follows:
- * nextRetry = failCount == 1 ? currentTime + initialDelay : currentTime +
delayPeriod^(failCount-1) where failCount = 1, 2, 3, ...
+ * nextRetry = failCount == 1 ? currentTime + initialDelay : currentTime +
delayPeriod*2^(failCount-1) where failCount = 1, 2, 3, ...
* nextRetry = Min(nextRetry, currentTime + maxDelay)
--- End diff --
The current implementation will multiply the delay by delayPeriod for each
retry after the first. Let's say I set initialDelay to 1000ms and retryPeriod
to 1000ms. The first retry is then scheduled at 1 second after failure. The
second at 1 second after that. The third retry is scheduled 1000 seconds after
that. The usual exponential backoff function would multiply the backoff by 2
for each failure, not by the backoff itself. This is a geometric progression
where you can only set ratio, not scale factor, which isn't very useful (for
anything except tiny backoffs, it grows much too fast). If we want to have
backoffs with multipliers other than 2, we could add another parameter for
that? (in that case that parameter should be an int, not a time period IMO)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
