chia7712 commented on code in PR #16421:
URL: https://github.com/apache/kafka/pull/16421#discussion_r1669192153
##########
clients/src/main/java/org/apache/kafka/common/requests/BrokerRegistrationRequest.java:
##########
@@ -45,7 +46,21 @@ public short oldestAllowedVersion() {
@Override
public BrokerRegistrationRequest build(short version) {
- return new BrokerRegistrationRequest(data, version);
+ if (version < 4) {
+ // Workaround for KAFKA-17011: for BrokerRegistrationRequest
versions older than 4,
+ // exclude support version ranges that begin with 0.
+ BrokerRegistrationRequestData newData = data.duplicate();
+ for (Iterator<BrokerRegistrationRequestData.Feature> iter =
newData.features().iterator();
Review Comment:
I guess there is no perf issue, so maybe we can simplify it by lambda code:
```java
newData.features().removeIf(feature -> feature.minSupportedVersion() == 0);
```
##########
core/src/test/scala/unit/kafka/server/ApiVersionsResponseIntegrationTest.scala:
##########
@@ -0,0 +1,83 @@
+/**
+ * Licensed 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.ApiVersionsResponseData.SupportedFeatureKeyCollection
+import org.apache.kafka.common.requests.ApiVersionsRequest
+import org.apache.kafka.common.requests.ApiVersionsResponse
+import org.apache.kafka.coordinator.group.GroupCoordinatorConfig
+import org.apache.kafka.server.config.ServerConfigs
+import org.junit.jupiter.api.Assertions.{assertEquals, assertNotNull,
assertNull}
+import org.junit.jupiter.params.ParameterizedTest
+import org.junit.jupiter.params.provider.ValueSource
+
+import java.util.Properties
+
+class ApiVersionsResponseIntegrationTest extends BaseRequestTest {
+ override def brokerCount: Int = 1
+
+ override def brokerPropertyOverrides(properties: Properties): Unit = {
+ properties.put(ServerConfigs.CONTROLLED_SHUTDOWN_ENABLE_CONFIG, "false")
+
properties.put(GroupCoordinatorConfig.OFFSETS_TOPIC_REPLICATION_FACTOR_CONFIG,
"1")
+ properties.put(GroupCoordinatorConfig.OFFSETS_TOPIC_PARTITIONS_CONFIG, "1")
+ }
+
+ private def sendApiVersionsRequest(version: Short): ApiVersionsResponse = {
+ val request = new ApiVersionsRequest.Builder().build(version)
+ connectAndReceive[ApiVersionsResponse](request)
+ }
+
+ @ParameterizedTest
+ @ValueSource(strings = Array("zk", "kraft"))
+ def testSendV3ApiVersionsRequest(quorum: String): Unit = {
+ val response = sendApiVersionsRequest(3)
+ assertFeatureDoesNotExist("group.version",
response.data().supportedFeatures())
+ if (quorum.equals("kraft")) {
+ assertFeatureHasMinVersion("metadata.version",
response.data().supportedFeatures(), 1)
+ } else {
+ assertEquals(0, response.data().supportedFeatures().size())
+ }
+ }
+
+ @ParameterizedTest
+ @ValueSource(strings = Array("zk", "kraft"))
+ def testSendV4ApiVersionsRequest(quorum: String): Unit = {
+ val response = sendApiVersionsRequest(4)
+ if (quorum.equals("kraft")) {
+ assertFeatureHasMinVersion("group.version",
response.data().supportedFeatures(), 0)
Review Comment:
`group.version` is already reverted by #16482
##########
clients/src/main/resources/common/message/ApiVersionsResponse.json:
##########
@@ -45,7 +47,7 @@
"about": "The duration in milliseconds for which the request was
throttled due to a quota violation, or zero if the request did not violate any
quota." },
{ "name": "SupportedFeatures", "type": "[]SupportedFeatureKey",
"ignorable": true,
"versions": "3+", "tag": 0, "taggedVersions": "3+",
- "about": "Features supported by the broker.",
+ "about": "Features supported by the broker. Note: in v0-v3, features
with MinSupportedVersion = 0 must be left out.",
Review Comment:
`MinSupportedVersion` -> `MinVersion`
--
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]
