lianetm commented on code in PR #15415:
URL: https://github.com/apache/kafka/pull/15415#discussion_r1499920005
##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/MemberState.java:
##########
@@ -132,11 +132,20 @@ public enum MemberState {
PREPARE_LEAVING.previousValidStates = Arrays.asList(JOINING, STABLE,
RECONCILING,
ACKNOWLEDGING, UNSUBSCRIBED, FENCED);
- LEAVING.previousValidStates = Arrays.asList(PREPARE_LEAVING);
+ // Transition from prepare leaving to leaving is the expected one in
all close operations
+ // except for when the poll timer expires (ex. leave group due to
unsubscribe or consumer
+ // close, where member triggers callbacks first while it continues
sending heartbeat
+ // (PREPARE_LEAVE state) and then sends the heartbeat to leave
(LEAVING state).
+ // All other transitions directly to LEAVING are expected when the
member leaves due to
+ // expired poll timer. In that case, the member sends the heartbeat to
leave first, and
+ // then invokes callbacks to release assignment while STALE, not
sending any more
+ // heartbeats while STALE because it has been already removed from the
group on the broker.
+ LEAVING.previousValidStates = Arrays.asList(PREPARE_LEAVING, JOINING,
RECONCILING,
Review Comment:
I would say we shouldn't transition through prepare_leaving because it
represents a member that is still part of the group, which is not the case in
this situation where the first thing is to send the leave group and then
everything else. This means that the prepare-leaving continues to send HB for
instance, and needs to be considered in all the interactions that could happen
while HBeating. Of course we could attempt to reuse the state, but would
probably be confusing being preparing to leave when we already sent the leave
group, and would complicate the now clear decision to HB while prepare leaving
and not HB while stale. The fundamental difference in sequence of the 2 kind of
leave makes it tricky to reuse the whole path: regular leave need to prepare to
leave and then leave. The expiration leave needs to leave first, and then go to
a state where it needs to do different things than the prepare leave: it needs
to stop HB, release assignment (this is the only similar to prepare
leaving), wait for timer reset, and then rejoin. All different needs than the
prepare, except for the release assignment.
Most of these transitions existed before btw (as previous to STALE), they
just moved places because we're reusing the LEAVING now. So in that sense the
change is not introducing lots to the state machine, just moving transitions
to reuse the existing LEAVING. With the change
[8440b3a](https://github.com/apache/kafka/pull/15415/commits/8440b3a405522037f1d7387289c0f2d9271c76cf)
for a unified leave I think it's better, and I did remove the comment since
the intention is clearer with the leave group change now I believe. Please take
a look and let me know if it aligns better
--
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
