lianetm commented on code in PR #15311: URL: https://github.com/apache/kafka/pull/15311#discussion_r1476728141
########## clients/src/main/java/org/apache/kafka/clients/consumer/internals/RequestState.java: ########## @@ -106,6 +106,13 @@ public void onSendAttempt(final long currentTimeMs) { this.lastSentMs = currentTimeMs; } + /** + * Update the lastReceivedTime in milliseconds, indicating that a response has been received. + */ + public void updateLastReceivedTime(final long lastReceivedMs) { + this.lastReceivedMs = lastReceivedMs; + } + Review Comment: This is not a problem in all managers because they end up calling `onSuccessfulAttempt` or `onFailedAttempt` in all cases, with clear paths for success, fail, or retry with backoff. The issue was with the heartbeat Mgr because it has the particularity that it wants to "retry" immediately on some errors, so it does not need the `onFailedAttempt` backoff strategy. Ex. fence (should just rejoin right after), not_coordinator (should discover the new one and retry when found). Then it does has some errors where it wants to backoff, like the coord_load_in_progress. There, btw, was the answer to your 2nd point. The `onSuccessfulAttempt` and `onFailedAttempt` do update the last received, but it's more than that (they set values for backoff and attempts), so not exactly what's needed in this HB case, that we want to record the response time, and then apply backoff strategy on some errors (still errors, so not a successful attempt either) Makes sense? -- 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