m1a2st commented on code in PR #19426: URL: https://github.com/apache/kafka/pull/19426#discussion_r2040680388
########## core/src/main/scala/kafka/server/DefaultApiVersionManager.scala: ########## @@ -0,0 +1,84 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package kafka.server + +import org.apache.kafka.common.message.ApiMessageType.ListenerType +import org.apache.kafka.common.protocol.ApiKeys +import org.apache.kafka.common.requests.ApiVersionsResponse +import org.apache.kafka.metadata.MetadataCache +import org.apache.kafka.server.{ApiVersionManager, BrokerFeatures, ClientMetricsManager} +import org.apache.kafka.server.common.FinalizedFeatures + +import java.util + +/** + * The default ApiVersionManager that supports forwarding and has metadata cache, used in brokers. + * The enabled APis are determined by the broker listener type and the controller APIs. + * + * @param listenerType the listener type + * @param forwardingManager the forwarding manager, Review Comment: These is a unnecessary comma at last of sentence ########## core/src/main/scala/kafka/server/DefaultApiVersionManager.scala: ########## @@ -0,0 +1,84 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package kafka.server + +import org.apache.kafka.common.message.ApiMessageType.ListenerType +import org.apache.kafka.common.protocol.ApiKeys +import org.apache.kafka.common.requests.ApiVersionsResponse +import org.apache.kafka.metadata.MetadataCache +import org.apache.kafka.server.{ApiVersionManager, BrokerFeatures, ClientMetricsManager} +import org.apache.kafka.server.common.FinalizedFeatures + +import java.util + +/** + * The default ApiVersionManager that supports forwarding and has metadata cache, used in brokers. + * The enabled APis are determined by the broker listener type and the controller APIs. Review Comment: typo: APi -> API ########## server/src/main/java/org/apache/kafka/server/SimpleApiVersionManager.java: ########## @@ -0,0 +1,108 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.kafka.server; + +import org.apache.kafka.common.feature.Features; +import org.apache.kafka.common.feature.SupportedVersionRange; +import org.apache.kafka.common.message.ApiMessageType; +import org.apache.kafka.common.message.ApiVersionsResponseData; +import org.apache.kafka.common.protocol.ApiKeys; +import org.apache.kafka.common.requests.ApiVersionsResponse; +import org.apache.kafka.server.common.FinalizedFeatures; + +import java.util.Set; +import java.util.function.Supplier; + +/** + * A simple ApiVersionManager used in controllers. It does not support forwarding and does not have metadata cache. + * Its enabled APIs are determined by the listener type, its finalized features are dynamically determined by the controller. + */ +public class SimpleApiVersionManager implements ApiVersionManager { + + private final ApiMessageType.ListenerType listenerType; + private final Set<ApiKeys> enabledApis; + private final Features<SupportedVersionRange> brokerFeatures; + private final boolean enableUnstableLastVersion; + private final Supplier<FinalizedFeatures> featuresProvider; + private final ApiVersionsResponseData.ApiVersionCollection apiVersions; + + /** + * SimpleApiVersionManager constructor + * @param listenerType the listener type + * @param enabledApis the enabled apis, which are computed by the listener type + * @param brokerFeatures the broker features + * @param enableUnstableLastVersion whether to enable unstable last version, see + * {@link org.apache.kafka.server.config.ServerConfigs#UNSTABLE_API_VERSIONS_ENABLE_CONFIG} + * @param featuresProvider a provider to the finalized features supported + */ + public SimpleApiVersionManager(ApiMessageType.ListenerType listenerType, + Set<ApiKeys> enabledApis, + Features<SupportedVersionRange> brokerFeatures, + boolean enableUnstableLastVersion, + Supplier<FinalizedFeatures> featuresProvider) { + this.listenerType = listenerType; + this.enabledApis = enabledApis; + this.brokerFeatures = brokerFeatures; + this.enableUnstableLastVersion = enableUnstableLastVersion; + this.featuresProvider = featuresProvider; + this.apiVersions = ApiVersionsResponse.collectApis(listenerType, enabledApis, enableUnstableLastVersion); + } + + public SimpleApiVersionManager(ApiMessageType.ListenerType listenerType, + boolean enableUnstableLastVersion, + Supplier<FinalizedFeatures> featuresProvider) { + this(listenerType, + ApiKeys.apisForListener(listenerType), + BrokerFeatures.defaultSupportedFeatures(enableUnstableLastVersion), + enableUnstableLastVersion, + featuresProvider); + } + + @Override + public boolean enableUnstableLastVersion() { + return enableUnstableLastVersion; + } + + @Override + public ApiMessageType.ListenerType listenerType() { + return listenerType; + } + + @Override + public Set<ApiKeys> enabledApis() { + return enabledApis; + } + + @Override + public ApiVersionsResponse apiVersionResponse(int throttleTimeMs, boolean alterFeatureLevel0) { + FinalizedFeatures currentFeatures = features(); + return new ApiVersionsResponse.Builder() + .setThrottleTimeMs(throttleTimeMs) + .setApiVersions(apiVersions) + .setSupportedFeatures(brokerFeatures) + .setFinalizedFeatures(currentFeatures.finalizedFeatures()) + .setFinalizedFeaturesEpoch(currentFeatures.finalizedFeaturesEpoch()) + .setZkMigrationEnabled(false) Review Comment: ditto, The `zkMigrationEnabled` setting defaults to `false`, and the ZooKeeper migration feature has been removed in version 4.0. Given this, I believe we can remove `setZkMigrationEnabled(false)` ########## core/src/main/scala/kafka/server/DefaultApiVersionManager.scala: ########## @@ -0,0 +1,84 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package kafka.server + +import org.apache.kafka.common.message.ApiMessageType.ListenerType +import org.apache.kafka.common.protocol.ApiKeys +import org.apache.kafka.common.requests.ApiVersionsResponse +import org.apache.kafka.metadata.MetadataCache +import org.apache.kafka.server.{ApiVersionManager, BrokerFeatures, ClientMetricsManager} +import org.apache.kafka.server.common.FinalizedFeatures + +import java.util + +/** + * The default ApiVersionManager that supports forwarding and has metadata cache, used in brokers. + * The enabled APis are determined by the broker listener type and the controller APIs. + * + * @param listenerType the listener type + * @param forwardingManager the forwarding manager, + * @param brokerFeatures the broker features + * @param metadataCache the metadata cache, used to get the finalized features and the metadata version + * @param enableUnstableLastVersion whether to enable unstable last version, see [[KafkaConfig.unstableApiVersionsEnabled]] + * @param clientMetricsManager the client metrics manager, helps to determine whether client telemetry is enabled + */ +class DefaultApiVersionManager( + val listenerType: ListenerType, + forwardingManager: ForwardingManager, + brokerFeatures: BrokerFeatures, + metadataCache: MetadataCache, + val enableUnstableLastVersion: Boolean, + val clientMetricsManager: Option[ClientMetricsManager] = None +) extends ApiVersionManager { + + val enabledApis: util.Set[ApiKeys] = ApiKeys.apisForListener(listenerType) + + override def apiVersionResponse( + throttleTimeMs: Int, + alterFeatureLevel0: Boolean + ): ApiVersionsResponse = { + val finalizedFeatures = metadataCache.features() + val controllerApiVersions = forwardingManager.controllerApiVersions + val clientTelemetryEnabled = clientMetricsManager match { + case Some(manager) => manager.isTelemetryReceiverConfigured + case None => false + } + val apiVersions = if (controllerApiVersions.isDefined) { + ApiVersionsResponse.controllerApiVersions( + controllerApiVersions.get, + listenerType, + enableUnstableLastVersion, + clientTelemetryEnabled) + } else { + ApiVersionsResponse.brokerApiVersions( + listenerType, + enableUnstableLastVersion, + clientTelemetryEnabled) + } + new ApiVersionsResponse.Builder(). + setThrottleTimeMs(throttleTimeMs). + setApiVersions(apiVersions). + setSupportedFeatures(brokerFeatures.supportedFeatures). + setFinalizedFeatures(finalizedFeatures.finalizedFeatures()). + setFinalizedFeaturesEpoch(finalizedFeatures.finalizedFeaturesEpoch()). + setZkMigrationEnabled(false). Review Comment: The `zkMigrationEnabled` setting defaults to `false`, and the ZooKeeper migration feature has been removed in version 4.0. Given this, I believe we can remove `setZkMigrationEnabled(false)` -- 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
