lianetm commented on code in PR #15311:
URL: https://github.com/apache/kafka/pull/15311#discussion_r1480240755
##########
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:
Thanks for the feedback. Agree with @dajac's point that we might be carrying
on with a non-desired backoff by leaving it unchanged in this situation, so we
definitely need to be setting one, and as @AndrewJSchofield said, all errors
would update the last receive time and apply "some" backoff.
That leads to deciding which one, and my intention was 0 backoff in a subset
of errors. As I see it, there are some specific errors in HB where we don't
just retry the same request, but rather do a different one, so it makes sense
to skip backoff as an optimization, ex.
1. HB to rejoin as new member after fence error. We would send it right away
after the fence, skipping backoff
2. HB to a new coordinator after not_coordinator error. We would send it as
soon as the new coordinator is discovered, without applying any backoff.
Makes sense? I will update to make sure we always set a backoff, but with
custom 0 backoff for these cases as the initial intention was, if it 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]
