junrao commented on code in PR #18997: URL: https://github.com/apache/kafka/pull/18997#discussion_r1968646894
########## metadata/src/test/java/org/apache/kafka/controller/QuorumControllerTest.java: ########## @@ -373,7 +373,7 @@ public void testElrEnabledByDefault() throws Throwable { )). build() ) { - controlEnv.activeController(true); + controlEnv.activeController(); Review Comment: Thinking a bit more. It seems that this test exposes a real bug introduced in https://github.com/apache/kafka/pull/16848/files#diff-77dc2adb187fd078084644613cff2b53021c8a5fbcdcfa116515734609d1332a. In the following code, we pick up the finalized feature version (including MV) first and pass them to the registerBroker event. It's possible that the passed in MV is outdated, but when the registerBroker event is processed, the MV becomes update to date. This will fail a registerBroker request that shouldn't be failed. It seems that we should pick up the finalized feature version when the registerBroker event is processed. ``` public CompletableFuture<BrokerRegistrationReply> registerBroker( ControllerRequestContext context, BrokerRegistrationRequestData request ) { // populate finalized features map with latest known kraft version for validation Map<String, Short> controllerFeatures = new HashMap<>(featureControl.finalizedFeatures(Long.MAX_VALUE).featureMap()); controllerFeatures.put(KRaftVersion.FEATURE_NAME, raftClient.kraftVersion().featureLevel()); return appendWriteEvent("registerBroker", context.deadlineNs(), () -> clusterControl. registerBroker(request, offsetControl.nextWriteOffset(), new FinalizedControllerFeatures(controllerFeatures, Long.MAX_VALUE), context.requestHeader().requestApiVersion() >= 3), EnumSet.noneOf(ControllerOperationFlag.class)); } ``` Colin confirmed that this is an issue. In general, we should not be accessing featureControl.finalizedFeatures outside of the event queue thread since it introduces potential concurrency issues. -- 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