mumrah commented on code in PR #17147:
URL: https://github.com/apache/kafka/pull/17147#discussion_r1754786855
##########
metadata/src/test/java/org/apache/kafka/metadata/migration/KRaftMigrationDriverTest.java:
##########
@@ -232,6 +232,32 @@ CompletableFuture<Void>
enqueueMetadataChangeEventWithFuture(
return future;
}
+ @Test
+ public void testOnControllerChangeWhenUninitialized() throws
InterruptedException {
+ CountingMetadataPropagator metadataPropagator = new
CountingMetadataPropagator();
+ CapturingMigrationClient.newBuilder().build();
+ CapturingMigrationClient migrationClient =
CapturingMigrationClient.newBuilder().build();
+ MockFaultHandler faultHandler = new
MockFaultHandler("testBecomeLeaderUninitialized");
+ KRaftMigrationDriver.Builder builder = defaultTestBuilder()
+ .setZkMigrationClient(migrationClient)
+ .setPropagator(metadataPropagator)
+ .setFaultHandler(faultHandler);
+ try (KRaftMigrationDriver driver = builder.build()) {
+ // Fake a complete migration with ZK client
+ migrationClient.setMigrationRecoveryState(
+
ZkMigrationLeadershipState.EMPTY.withKRaftMetadataOffsetAndEpoch(100, 1));
+
+ // enqueue a poll event. once run, this will enqueue a recovery
event
+ driver.start();
Review Comment:
Good point. Let's consider the possibilities:
Case 1: If the PollEvent runs quickly, we'll see
* start(), enqueue PollEvent
* run PollEvent, enqueue RecoverMigrationStateFromZKEvent (from PollEvent)
* onControllerChange(), enqueue RecoverMigrationStateFromZKEvent and
KRaftLeaderEvent
* run RecoverMigrationStateFromZKEvent, state = INACTIVE
* run RecoverMigrationStateFromZKEvent, no-op
* run KRaftLeaderEvent, state is INACTIVE ✅
Case 2: If the RecoverMigrationStateFromZKEvent from PollEvent runs before
onControllerChange:
* start(), enqueue PollEvent
* run PollEvent, enqueue RecoverMigrationStateFromZKEvent (from PollEvent)
* run RecoverMigrationStateFromZKEvent, state = INACTIVE
* onControllerChange(), enqueue KRaftLeaderEvent
* run KRaftLeaderEvent, state is INACTIVE ✅
Only Case 1 tests the race condition and the fix.
If we reverse the operations, we'll have
Case 3:
* onControllerChange(), enqueue RecoverMigrationStateFromZKEvent and
KRaftLeaderEvent
* start(), enqueue PollEvent
* run RecoverMigrationStateFromZKEvent, state = INACTIVE
* run KRaftLeaderEvent, state is INACTIVE ✅
* run PollEvent, no-op
So yea, it makes sense to use Case 3 for the test since it contrives the
race condition.
--
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]
