kafka-jira mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [kafka] ncliang commented on a change in pull request #9765: KAFKA-10763: Fix incomplete cooperative rebalances preventing connector/task revocations
Date Fri, 18 Dec 2020 18:56:49 GMT

ncliang commented on a change in pull request #9765:
URL: https://github.com/apache/kafka/pull/9765#discussion_r546001555



##########
File path: connect/runtime/src/test/java/org/apache/kafka/connect/runtime/distributed/DistributedHerderTest.java
##########
@@ -566,6 +566,112 @@ public Boolean answer() throws Throwable {
         PowerMock.verifyAll();
     }
 
+    @Test
+    public void testRevoke() throws TimeoutException {
+        revokeAndReassign(false);
+    }
+
+    @Test
+    public void testIncompleteRebalanceBeforeRevoke() throws TimeoutException {
+        revokeAndReassign(true);
+    }
+
+    public void revokeAndReassign(boolean incompleteRebalance) throws TimeoutException {

Review comment:
       It's hard to tell if this actually reproduces the issue or not due to the heavy mocking
required. Is there a more direct way to reproduce? Maybe in `RebalanceSourceConnectorsIntegrationTest`
or similar? Even if the IT ends up being flaky, having that repro would boost confidence in
this fix.

##########
File path: connect/runtime/src/main/java/org/apache/kafka/connect/runtime/distributed/DistributedHerder.java
##########
@@ -1740,7 +1741,7 @@ public void onRevoked(String leader, Collection<String> connectors,
Collection<C
             // Note that since we don't reset the assignment, we don't revoke leadership
here. During a rebalance,
             // it is still important to have a leader that can write configs, offsets, etc.
 
-            if (rebalanceResolved) {
+            if (rebalanceResolved || currentProtocolVersion >= CONNECT_PROTOCOL_V1) {

Review comment:
       Maybe add a comment explaining why the additional check is needed.




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



Mime
View raw message