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:
us...@infra.apache.org
