dajac commented on code in PR #15311:
URL: https://github.com/apache/kafka/pull/15311#discussion_r1481205214


##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/RequestState.java:
##########
@@ -127,8 +127,16 @@ public void onSuccessfulAttempt(final long currentTimeMs) {
      * @param currentTimeMs Current time in milliseconds
      */
     public void onFailedAttempt(final long currentTimeMs) {
+        onFailedAttempt(currentTimeMs, false);
+    }
+
+    public void onFailedAttempt(final long currentTimeMs, final boolean 
skipBackoff) {
         this.lastReceivedMs = currentTimeMs;
-        this.backoffMs = exponentialBackoff.backoff(numAttempts);
+        if (skipBackoff) {
+            this.backoffMs = 0;
+        } else {
+            this.backoffMs = exponentialBackoff.backoff(numAttempts);
+        }
         this.numAttempts++;

Review Comment:
   Don't we need to also skip incrementing `numAttempts`? Conceptually, I would 
say so but I may have missed something.
   
   I was also wondering whether it would make sense to keep `onFailedAttempt` 
as it was before and to always call it on failures and to add a `resetBackoff` 
method that we could call in the places where we don't want to backoff. 
`resetBackoff` would set both `backoffMs` and `numAttempts` to zero. Another 
alternative would have been to call `onSuccessfulAttempt` for the cases where 
we don't want to retry with a backoff. Have you already considered those? 



-- 
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: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to