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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
