philipnee commented on code in PR #14873:
URL: https://github.com/apache/kafka/pull/14873#discussion_r1422983344


##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/HeartbeatRequestManager.java:
##########
@@ -201,13 +224,35 @@ public long maximumTimeToWait(long currentTimeMs) {
         return heartbeatNow ? 0L : 
heartbeatRequestState.nextHeartbeatMs(currentTimeMs);
     }
 
-    private NetworkClientDelegate.UnsentRequest makeHeartbeatRequest() {
+    /**
+     * When consumer polls, we need to reset the pollTimer.  If the poll timer 
has expired, we rejoin only when the
+     * member is in the {@link MemberState#UNSUBSCRIBED} state.
+     */
+    public void resetPollTimer() {
+        pollTimer.reset(maxPollIntervalMs);
+    }
+
+    private NetworkClientDelegate.UnsentRequest makeHeartbeatRequest(final 
long currentTimeMs,
+                                                                     final 
boolean ignoreResponse) {
+        NetworkClientDelegate.UnsentRequest request = 
makeHeartbeatRequest(ignoreResponse);
+        heartbeatRequestState.onSendAttempt(currentTimeMs);
+        membershipManager.onHeartbeatRequestSent();
+        return request;
+    }
+
+    private NetworkClientDelegate.UnsentRequest makeHeartbeatRequest(final 
boolean ignoreResponse) {
         NetworkClientDelegate.UnsentRequest request = new 
NetworkClientDelegate.UnsentRequest(
-            new 
ConsumerGroupHeartbeatRequest.Builder(this.heartbeatState.buildRequestData()),
-            coordinatorRequestManager.coordinator());
+                new 
ConsumerGroupHeartbeatRequest.Builder(this.heartbeatState.buildRequestData()),
+                coordinatorRequestManager.coordinator());
         return request.whenComplete((response, exception) -> {
             if (response != null) {
                 onResponse((ConsumerGroupHeartbeatResponse) 
response.responseBody(), request.handler().completionTimeMs());
+                // The response is only ignore when the member becomes staled. 
This is because the member needs to
+                // rejoin on the next poll regardless the server has responded 
to the heartbeat.
+                if (!ignoreResponse) {
+                    
membershipManager.onHeartbeatResponseReceived(((ConsumerGroupHeartbeatResponse) 
response.responseBody()).data());
+                    maybeSendGroupMetadataUpdateEvent();

Review Comment:
   For calling onResponse: We need to try our best to handle the error if there 
is one.   However, when leaving a group, we don't need to process the 
`onHeartbeatResponseReceived` because we don't need to reconcile.
   
   After leaving the group - The request manager should not keep heart beating. 
 For this specific PR, when the poll timer expires, it will return an empty 
response when the request manager gets polled.  See the first if block in 
poll().  And you are right, when the user poll the consumer, it resets the 
timer, and on the next request manager poll (they happen asynchronously), it 
should send a heartbeat to rejoin.
   
   i think if we commit after leaving the group - I think the commit will get 
rejected with an error.
   



-- 
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]

Reply via email to