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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
