kevin-wu24 commented on code in PR #21122:
URL: https://github.com/apache/kafka/pull/21122#discussion_r2611529140
##########
metadata/src/main/java/org/apache/kafka/metadata/publisher/FeaturesPublisher.java:
##########
@@ -27,13 +27,15 @@
import org.slf4j.Logger;
-import static org.apache.kafka.server.common.MetadataVersion.MINIMUM_VERSION;
+import java.util.Map;
+import java.util.Optional;
public class FeaturesPublisher implements MetadataPublisher {
private final Logger log;
private final FaultHandler faultHandler;
- private volatile FinalizedFeatures finalizedFeatures =
FinalizedFeatures.fromKRaftVersion(MINIMUM_VERSION);
+ private volatile FinalizedFeatures finalizedFeatures = new
FinalizedFeatures(
Review Comment:
We should make `new FinalizedFeatures(Optional.empty()...)` a static
constant in `FinalizedFeatures`. Maybe something like
`UNKNOWN_FINALIZED_FEATURES`.
##########
core/src/main/scala/kafka/server/ControllerApis.scala:
##########
@@ -1059,7 +1059,7 @@ class ControllerApis(
def handleDescribeCluster(request: RequestChannel.Request):
CompletableFuture[Unit] = {
// Nearly all RPCs should check MetadataVersion inside the
QuorumController. However, this
// RPC is consulting a cache which lives outside the QC. So we check
MetadataVersion here.
- if
(!apiVersionManager.features.metadataVersion().isControllerRegistrationSupported)
{
+ if
(!apiVersionManager.features.metadataVersion().map(_.isControllerRegistrationSupported).orElse(false))
{
Review Comment:
It would be better if we could return a more accurate error message in the
case where `metadataVersion().isEmpty()`. Returning a message like "There is no
finalized MetadataVersion, so direct-to-controller communication is not
supported" is more accurate in this case.
--
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]
