lianetm commented on code in PR #14857:
URL: https://github.com/apache/kafka/pull/14857#discussion_r1409611754


##########
clients/src/test/java/org/apache/kafka/clients/consumer/internals/MembershipManagerImplTest.java:
##########
@@ -223,8 +232,65 @@ public void testFencingWhenStateIsReconciling() {
         verify(subscriptionState).assignFromSubscribed(Collections.emptySet());
     }
 
+    @Test
+    public void testFencingWhenStateIsPrepareLeaving() {
+        MembershipManagerImpl membershipManager = createMemberInStableState();
+
+        // Start leaving group, blocked waiting for commit of all consumed to 
complete.
+        CompletableFuture<Void>  commitResult = 
mockPrepareLeavingStuckCommitting();
+        membershipManager.leaveGroup();
+        assertEquals(MemberState.PREPARE_LEAVING, membershipManager.state());
+
+        // Get fenced while preparing to leave the group. Member should not 
try to rejoin and
+        // continue leaving the group as it was before getting fenced.
+        mockMemberHasAutoAssignedPartition();
+        membershipManager.transitionToFenced();
+        assertEquals(MemberState.PREPARE_LEAVING, membershipManager.state());
+        assertNotEquals(0, membershipManager.memberEpoch());
+
+        // When commit completes member should transition to LEAVE.
+        commitResult.complete(null);
+        assertEquals(MemberState.LEAVING, membershipManager.state());
+    }
+
+    @Test
+    public void testFencingWhenStateIsLeaving() {
+        MembershipManagerImpl membershipManager = createMemberInStableState();
+
+        // Start leaving group.
+        mockLeaveGroup();
+        membershipManager.leaveGroup();
+        assertEquals(MemberState.LEAVING, membershipManager.state());
+
+        // Get fenced while leaving. Member should not try to rejoin and 
continue leaving the
+        // group as it was before getting fenced.
+        mockMemberHasAutoAssignedPartition();
+        membershipManager.transitionToFenced();
+        assertEquals(MemberState.LEAVING, membershipManager.state());

Review Comment:
   I also closed this same gap for fatal errors, in the case where that last HB 
may be sent, and a fatal error may come in the response. At that point the 
member should transition to FATAL but without triggering the `onPartitionsLost` 
(it already left the group and released all it assignments). Not sure if the 
server could ever send a fatal error as a response to a last HB, but seems 
better to have the client handling it properly, no matter what the broker ends 
up doing. 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]

Reply via email to