chickenchickenlove commented on code in PR #23714:
URL: https://github.com/apache/kafka/pull/23714#discussion_r4190770261
##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/AbstractHeartbeatRequestManager.java:
##########
@@ -184,6 +184,11 @@ public NetworkClientDelegate.PollResult poll(long
currentTimeMs) {
return new
NetworkClientDelegate.PollResult(heartbeatRequestState.heartbeatIntervalMs(),
Collections.singletonList(leaveHeartbeat));
}
+ if (membershipManager().state() == MemberState.LEAVING &&
shouldSkipLeaveHeartbeat()) {
Review Comment:
@lianetm Thanks for your comments!
Yes, you're right for dynamic members using the consumer protocol. This was
broken in the regular close path too, independently of poll timer expiration.
The original code attempted to suppress the leave heartbeat by returning
false from `shouldSendLeaveHeartbeatNow()` for `REMAIN_IN_GROUP`. However, that
only disabled one side of the following OR condition:
```java
boolean heartbeatNow = shouldSendLeaveHeartbeatNow() ||
(membershipManager().shouldHeartbeatNow() &&
!heartbeatRequestState.requestInFlight());
if (!heartbeatRequestState.canSendRequest(currentTimeMs) && !heartbeatNow) {
return new
NetworkClientDelegate.PollResult(heartbeatRequestState.timeToNextHeartbeatMs(currentTimeMs));
}
```
Closing with `REMAIN_IN_GROUP` still transitioned the member to `LEAVING`
and set its epoch to `-1`. In that state, `shouldHeartbeatNow()` returned
`true`, so when no heartbeat was in flight, the second condition allowed a
leave heartbeat to be generated anyway. Even when heartbeatNow was false, the
normal `canSendRequest()` check could still allow it once the heartbeat
interval and backoff permitted.
However, `REMAIN_IN_GROUP` worked correctly for static members. they
intentionally send a heartbeat with epoch `-2`, which preserves their
membership until the session timeout expires. This issue affected dynamic
members, which should not send a leave heartbeat when closing with
`REMAIN_IN_GROUP`.
--
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]
