Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -1751,7 +1751,7 @@ project(':storage') {
task processMessages(type:JavaExec) {
mainClass = "org.apache.kafka.message.MessageGenerator"
classpath = configurations.generator
args = [ "-p", " org.apache.kafka.server.log.remote.metadata.storage.generated",
args = [ "-p", "org.apache.kafka.server.log.remote.metadata.storage.generated",
"-o", "src/generated/java/org/apache/kafka/server/log/remote/metadata/storage/generated",
"-i", "src/main/resources/message",
"-m", "MessageDataGenerator", "JsonConverterGenerator",
Expand Down
4 changes: 3 additions & 1 deletion checkstyle/checkstyle.xml
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,9 @@
</module>

<!-- code quality -->
<module name="MethodLength"/>
<module name="MethodLength">
<property name="max" value="170" />
</module>
Comment on lines 115 to 117

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because AbstractRequest.doParseRequest() and AbstractResponse.parseResponse() now exceed 150 lines.

<module name="ParameterNumber">
<!-- default is 8 -->
<property name="max" value="13"/>
Expand Down
3 changes: 3 additions & 0 deletions checkstyle/import-control-metadata.xml
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@
<allow pkg="org.apache.kafka.common.message" />
<allow pkg="org.apache.kafka.common.metadata" />
</subpackage>
<subpackage name="errors">
<allow pkg="org.apache.kafka.common.metadata" />
</subpackage>
</subpackage>

<subpackage name="controller">
Expand Down
30 changes: 30 additions & 0 deletions clients/src/main/java/org/apache/kafka/common/Uuid.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,12 @@
package org.apache.kafka.common;

import java.nio.ByteBuffer;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Base64;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Set;

/**
Expand Down Expand Up @@ -169,4 +171,32 @@ public int compareTo(Uuid other) {
return 0;
}
}

/**
* Convert a list of Uuid to an array of Uuid.
*
* @param list The input list
* @return The output array
*/
public static Uuid[] toArray(List<Uuid> list) {
if (list == null) return null;
Uuid[] array = new Uuid[list.size()];
for (int i = 0; i < list.size(); i++) {
array[i] = list.get(i);
}
return array;
}

/**
* Convert an array of Uuids to a list of Uuid.
*
* @param array The input array
* @return The output list
*/
public static List<Uuid> toList(Uuid[] array) {
if (array == null) return null;
List<Uuid> list = new ArrayList<>(array.length);
list.addAll(Arrays.asList(array));
return list;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,8 @@ public enum ApiKeys {
CONSUMER_GROUP_DESCRIBE(ApiMessageType.CONSUMER_GROUP_DESCRIBE),
CONTROLLER_REGISTRATION(ApiMessageType.CONTROLLER_REGISTRATION),
GET_TELEMETRY_SUBSCRIPTIONS(ApiMessageType.GET_TELEMETRY_SUBSCRIPTIONS),
PUSH_TELEMETRY(ApiMessageType.PUSH_TELEMETRY);
PUSH_TELEMETRY(ApiMessageType.PUSH_TELEMETRY),
ASSIGN_REPLICAS_TO_DIRS(ApiMessageType.ASSIGN_REPLICAS_TO_DIRS);

private static final Map<ApiMessageType.ListenerType, EnumSet<ApiKeys>> APIS_BY_LISTENER =
new EnumMap<>(ApiMessageType.ListenerType.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,8 @@ private static AbstractRequest doParseRequest(ApiKeys apiKey, short apiVersion,
return GetTelemetrySubscriptionsRequest.parse(buffer, apiVersion);
case PUSH_TELEMETRY:
return PushTelemetryRequest.parse(buffer, apiVersion);
case ASSIGN_REPLICAS_TO_DIRS:
return AssignReplicasToDirsRequest.parse(buffer, apiVersion);
default:
throw new AssertionError(String.format("ApiKey %s is not currently handled in `parseRequest`, the " +
"code should be updated to do so.", apiKey));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,8 @@ public static AbstractResponse parseResponse(ApiKeys apiKey, ByteBuffer response
return GetTelemetrySubscriptionsResponse.parse(responseBuffer, version);
case PUSH_TELEMETRY:
return PushTelemetryResponse.parse(responseBuffer, version);
case ASSIGN_REPLICAS_TO_DIRS:
return AssignReplicasToDirsResponse.parse(responseBuffer, version);
default:
throw new AssertionError(String.format("ApiKey %s is not currently handled in `parseResponse`, the " +
"code should be updated to do so.", apiKey));
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
/*
* 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.common.requests;

import org.apache.kafka.common.message.AssignReplicasToDirsRequestData;
import org.apache.kafka.common.message.AssignReplicasToDirsResponseData;
import org.apache.kafka.common.protocol.ApiKeys;
import org.apache.kafka.common.protocol.ByteBufferAccessor;
import org.apache.kafka.common.protocol.Errors;

import java.nio.ByteBuffer;

public class AssignReplicasToDirsRequest extends AbstractRequest {

public static class Builder extends AbstractRequest.Builder<AssignReplicasToDirsRequest> {

private final AssignReplicasToDirsRequestData data;

public Builder(AssignReplicasToDirsRequestData data) {
super(ApiKeys.ASSIGN_REPLICAS_TO_DIRS);
this.data = data;
}

@Override
public AssignReplicasToDirsRequest build(short version) {
return new AssignReplicasToDirsRequest(data, version);
}

@Override
public String toString() {
return data.toString();
}
}

private final AssignReplicasToDirsRequestData data;

public AssignReplicasToDirsRequest(AssignReplicasToDirsRequestData data, short version) {
super(ApiKeys.ASSIGN_REPLICAS_TO_DIRS, version);
this.data = data;
}

@Override
public AbstractResponse getErrorResponse(int throttleTimeMs, Throwable e) {
return new AssignReplicasToDirsResponse(
new AssignReplicasToDirsResponseData()
.setThrottleTimeMs(throttleTimeMs)
.setErrorCode(Errors.forException(e).code())
);
}

@Override
public AssignReplicasToDirsRequestData data() {
return data;
}

public static AssignReplicasToDirsRequest parse(ByteBuffer buffer, short version) {
return new AssignReplicasToDirsRequest(new AssignReplicasToDirsRequestData(
new ByteBufferAccessor(buffer), version), version);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
/*
* 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.common.requests;

import org.apache.kafka.common.message.AssignReplicasToDirsResponseData;
import org.apache.kafka.common.protocol.ApiKeys;
import org.apache.kafka.common.protocol.ByteBufferAccessor;
import org.apache.kafka.common.protocol.Errors;

import java.nio.ByteBuffer;

import java.util.Collections;
import java.util.Map;

public class AssignReplicasToDirsResponse extends AbstractResponse {

private final AssignReplicasToDirsResponseData data;

public AssignReplicasToDirsResponse(AssignReplicasToDirsResponseData data) {
super(ApiKeys.ASSIGN_REPLICAS_TO_DIRS);
this.data = data;
}

@Override
public AssignReplicasToDirsResponseData data() {
return data;
}

@Override
public Map<Errors, Integer> errorCounts() {
return Collections.singletonMap(Errors.forCode(data.errorCode()), 1);
}

@Override
public int throttleTimeMs() {
return data.throttleTimeMs();
}

@Override
public void maybeSetThrottleTimeMs(int throttleTimeMs) {
data.setThrottleTimeMs(throttleTimeMs);
}

public static AssignReplicasToDirsResponse parse(ByteBuffer buffer, short version) {
return new AssignReplicasToDirsResponse(new AssignReplicasToDirsResponseData(
new ByteBufferAccessor(buffer), version));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// 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.

{
"apiKey": 73,
"type": "request",
"listeners": ["controller"],
"name": "AssignReplicasToDirsRequest",
"validVersions": "0",
"flexibleVersions": "0+",
"fields": [
{ "name": "BrokerId", "type": "int32", "versions": "0+", "entityType": "brokerId",
"about": "The ID of the requesting broker" },
{ "name": "BrokerEpoch", "type": "int64", "versions": "0+", "default": "-1",
"about": "The epoch of the requesting broker" },
{ "name": "Directories", "type": "[]DirectoryData", "versions": "0+", "fields": [
{ "name": "Id", "type": "uuid", "versions": "0+", "about": "The ID of the directory" },
{ "name": "Topics", "type": "[]TopicData", "versions": "0+", "fields": [
{ "name": "TopicId", "type": "uuid", "versions": "0+",
"about": "The ID of the assigned topic" },
{ "name": "Partitions", "type": "[]PartitionData", "versions": "0+", "fields": [
{ "name": "PartitionIndex", "type": "int32", "versions": "0+",
"about": "The partition index" }
]}
]}
]}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
// 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.

{
"apiKey": 73,
"type": "response",
"name": "AssignReplicasToDirsResponse",
"validVersions": "0",
"flexibleVersions": "0+",
"fields": [
{ "name": "ThrottleTimeMs", "type": "int32", "versions": "0+",
"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": "ErrorCode", "type": "int16", "versions": "0+",
"about": "The top level response error code" },
{ "name": "Directories", "type": "[]DirectoryData", "versions": "0+", "fields": [
{ "name": "Id", "type": "uuid", "versions": "0+", "about": "The ID of the directory" },
{ "name": "Topics", "type": "[]TopicData", "versions": "0+", "fields": [
{ "name": "TopicId", "type": "uuid", "versions": "0+",
"about": "The ID of the assigned topic" },
{ "name": "Partitions", "type": "[]PartitionData", "versions": "0+", "fields": [
{ "name": "PartitionIndex", "type": "int32", "versions": "0+",
"about": "The partition index" },
{ "name": "ErrorCode", "type": "int16", "versions": "0+",
"about": "The partition level error code" }
]}
]}
]}
]
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
"type": "request",
"listeners": ["controller"],
"name": "BrokerHeartbeatRequest",
"validVersions": "0",
"validVersions": "0-1",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For my understanding, is there a benefit to updating the API version when you are adding a tagged (optional) field only?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this is more confusing now after the discussion around whether to burn the tag for Replicas or not. So this is a good question.

KIP-482 does say part of the motivation for tagged fields is to add them without bumping the record version:

Finally, sometimes, we want to add extra information to a message without requiring a version bump for everyone who has to read that message. This is particularly important for metadata like consumer offsets.

I'd rather we don't have two different definitions for same record version. That can lead to unnecessary confusion. It also feels weird to bump the version for PartitionRecord but not for PartitionChangeRecord. I also don't see any downside with bumping the version.

I wonder what @cmccabe 's take is on this.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about this over the weekend and I am happy to approve the pull request in its current state and then circle back to this at a later point in time. I think that it won't break anything whichever way you decide to go forward.

"flexibleVersions": "0+",
"fields": [
{ "name": "BrokerId", "type": "int32", "versions": "0+", "entityType": "brokerId",
Expand All @@ -30,6 +30,8 @@
{ "name": "WantFence", "type": "bool", "versions": "0+",
"about": "True if the broker wants to be fenced, false otherwise." },
{ "name": "WantShutDown", "type": "bool", "versions": "0+",
"about": "True if the broker wants to be shut down, false otherwise." }
"about": "True if the broker wants to be shut down, false otherwise." },
{ "name": "OfflineLogDirs", "type": "[]uuid", "versions": "1+", "taggedVersions": "1+", "tag": "0",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall this "tag": "0" be "tag": 0?

"about": "Log directories that failed and went offline." }
]
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
"apiKey": 63,
"type": "response",
"name": "BrokerHeartbeatResponse",
"validVersions": "0",
"validVersions": "0-1",
"flexibleVersions": "0+",
"fields": [
{ "name": "ThrottleTimeMs", "type": "int32", "versions": "0+",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,16 @@
// limitations under the License.

// Version 1 adds Zk broker epoch to the request if the broker is migrating from Zk mode to KRaft mode.
//

// Version 2 adds the PreviousBrokerEpoch for the KIP-966

// Version 3 adds LogDirs for KIP-858
{
"apiKey":62,
"type": "request",
"listeners": ["controller"],
"name": "BrokerRegistrationRequest",
"validVersions": "0-2",
"validVersions": "0-3",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the reasoning behind doing two bumps of this API version within the same Kafka release? 3.6 is currently on "validVersions": "0-3", so it looks like Kafka 3.7.0 will introduce both versions 2 and 3 of this API.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @aiven-anton. The api protocol may change more than once within the same release. The RPC version is bumped for each change. Refer to MetadataVersion and its documentation.

/**
* This class contains the different Kafka versions.
* Right now, we use them for upgrades - users can configure the version of the API brokers will use to communicate between themselves.
* This is only for inter-broker communications - when communicating with clients, the client decides on the API version.
* <br>
* Note that the ID we initialize for each version is important.
* We consider a version newer than another if it is lower in the enum list (to avoid depending on lexicographic order)
* <br>
* Since the api protocol may change more than once within the same release and to facilitate people deploying code from
* trunk, we have the concept of internal versions (first introduced during the 0.10.0 development cycle). For example,
* the first time we introduce a version change in a release, say 0.10.0, we will add a config value "0.10.0-IV0" and a
* corresponding enum constant IBP_0_10_0-IV0. We will also add a config value "0.10.0" that will be mapped to the
* latest internal version object, which is IBP_0_10_0-IV0. When we change the protocol a second time while developing
* 0.10.0, we will add a new config value "0.10.0-IV1" and a corresponding enum constant IBP_0_10_0-IV1. We will change
* the config value "0.10.0" to map to the latest internal version IBP_0_10_0-IV1. The config value of
* "0.10.0-IV0" is still mapped to IBP_0_10_0-IV0. This way, if people are deploying from trunk, they can use
* "0.10.0-IV0" and "0.10.0-IV1" to upgrade one internal version at a time. For most people who just want to use
* released version, they can use "0.10.0" when upgrading to the 0.10.0 release.
*/
public enum MetadataVersion {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@soarez Ah, thanks for pointing this out 🙏

"flexibleVersions": "0+",
"fields": [
{ "name": "BrokerId", "type": "int32", "versions": "0+", "entityType": "brokerId",
Expand Down Expand Up @@ -57,6 +59,8 @@
{ "name": "IsMigratingZkBroker", "type": "bool", "versions": "1+", "default": "false",
"about": "If the required configurations for ZK migration are present, this value is set to true" },
{ "name": "PreviousBrokerEpoch", "type": "int64", "versions": "2+", "default": "-1",
"about": "The epoch before a clean shutdown." }
"about": "The epoch before a clean shutdown." },
{ "name": "LogDirs", "type": "[]uuid", "versions": "3+",
"about": "Log directories configured in this broker which are available." }
]
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
"apiKey": 62,
"type": "response",
"name": "BrokerRegistrationResponse",
"validVersions": "0-2",
"validVersions": "0-3",
"flexibleVersions": "0+",
"fields": [
{ "name": "ThrottleTimeMs", "type": "int32", "versions": "0+",
Expand Down
Loading