Re: [PR] KAFKA-16527; Implement request handling for updated KRaft RPCs [kafka]
chia7712 commented on code in PR #16235:
URL: https://github.com/apache/kafka/pull/16235#discussion_r1706356186
##
clients/src/main/resources/common/message/FetchRequest.json:
##
@@ -100,7 +102,9 @@
{ "name": "LogStartOffset", "type": "int64", "versions": "5+",
"default": "-1", "ignorable": true,
"about": "The earliest available offset of the follower replica.
The field is only used when the request is sent by the follower."},
{ "name": "PartitionMaxBytes", "type": "int32", "versions": "0+",
- "about": "The maximum bytes to fetch from this partition. See
KIP-74 for cases where this limit may not be honored." }
+ "about": "The maximum bytes to fetch from this partition. See
KIP-74 for cases where this limit may not be honored." },
+{ "name": "ReplicaDirectoryId", "type": "uuid", "versions": "17+",
"taggedVersions": "17+", "tag": 0,
Review Comment:
> We need to mark the field as ignorable. Same for the FetchSnapshotRequest.
yep, you are right. that is the better approach. I will update the jira
> Are you going to work on the PR @chia7712 ? If so, I can review it when it
is ready.
@m1a2st he will file PR after the e2e pass on our local.
--
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
Re: [PR] KAFKA-16527; Implement request handling for updated KRaft RPCs [kafka]
jolshan commented on code in PR #16235:
URL: https://github.com/apache/kafka/pull/16235#discussion_r1706208615
##
clients/src/main/resources/common/message/FetchRequest.json:
##
@@ -100,7 +102,9 @@
{ "name": "LogStartOffset", "type": "int64", "versions": "5+",
"default": "-1", "ignorable": true,
"about": "The earliest available offset of the follower replica.
The field is only used when the request is sent by the follower."},
{ "name": "PartitionMaxBytes", "type": "int32", "versions": "0+",
- "about": "The maximum bytes to fetch from this partition. See
KIP-74 for cases where this limit may not be honored." }
+ "about": "The maximum bytes to fetch from this partition. See
KIP-74 for cases where this limit may not be honored." },
+{ "name": "ReplicaDirectoryId", "type": "uuid", "versions": "17+",
"taggedVersions": "17+", "tag": 0,
Review Comment:
Error is in the title of this ticket:
https://issues.apache.org/jira/browse/KAFKA-17250.
How are you writing the RPC in non-test files? It seems like you always set
directory ID here?
https://github.com/apache/kafka/blob/5596f9e1d561dfef46a83fbd7264e6745ca538cc/raft/src/main/java/org/apache/kafka/raft/KafkaRaftClient.java#L2686
You cannot guarantee the receiver supports the same version as the sender.
(sorry I sent this before the response from Jose -- but yes, we are seeing
the same issue)
--
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
Re: [PR] KAFKA-16527; Implement request handling for updated KRaft RPCs [kafka]
jolshan commented on code in PR #16235:
URL: https://github.com/apache/kafka/pull/16235#discussion_r1706208615
##
clients/src/main/resources/common/message/FetchRequest.json:
##
@@ -100,7 +102,9 @@
{ "name": "LogStartOffset", "type": "int64", "versions": "5+",
"default": "-1", "ignorable": true,
"about": "The earliest available offset of the follower replica.
The field is only used when the request is sent by the follower."},
{ "name": "PartitionMaxBytes", "type": "int32", "versions": "0+",
- "about": "The maximum bytes to fetch from this partition. See
KIP-74 for cases where this limit may not be honored." }
+ "about": "The maximum bytes to fetch from this partition. See
KIP-74 for cases where this limit may not be honored." },
+{ "name": "ReplicaDirectoryId", "type": "uuid", "versions": "17+",
"taggedVersions": "17+", "tag": 0,
Review Comment:
Error is in the title of this ticket:
https://issues.apache.org/jira/browse/KAFKA-17250.
How are you writing the RPC in non-test files? It seems like you always set
directory ID here?
https://github.com/apache/kafka/blob/5596f9e1d561dfef46a83fbd7264e6745ca538cc/raft/src/main/java/org/apache/kafka/raft/KafkaRaftClient.java#L2686
You cannot guarantee the receiver supports the same version as the sender.
--
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
Re: [PR] KAFKA-16527; Implement request handling for updated KRaft RPCs [kafka]
jsancio commented on code in PR #16235:
URL: https://github.com/apache/kafka/pull/16235#discussion_r1706206849
##
clients/src/main/resources/common/message/FetchRequest.json:
##
@@ -100,7 +102,9 @@
{ "name": "LogStartOffset", "type": "int64", "versions": "5+",
"default": "-1", "ignorable": true,
"about": "The earliest available offset of the follower replica.
The field is only used when the request is sent by the follower."},
{ "name": "PartitionMaxBytes", "type": "int32", "versions": "0+",
- "about": "The maximum bytes to fetch from this partition. See
KIP-74 for cases where this limit may not be honored." }
+ "about": "The maximum bytes to fetch from this partition. See
KIP-74 for cases where this limit may not be honored." },
+{ "name": "ReplicaDirectoryId", "type": "uuid", "versions": "17+",
"taggedVersions": "17+", "tag": 0,
Review Comment:
I see the issue in the generator code:
```java
if (_version >= 17) {
if (!this.replicaDirectoryId.equals(Uuid.ZERO_UUID)) {
_numTaggedFields++;
}
} else {
if (!this.replicaDirectoryId.equals(Uuid.ZERO_UUID)) {
throw new UnsupportedVersionException("Attempted to
write a non-default replicaDirectoryId at version " + _version);
}
}
```
We need to mark the field as ignorable. Same for the `FetchSnapshotRequest`.
Are you going to work on the PR @chia7712 ? If so, I can review it when it
is ready.
--
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
Re: [PR] KAFKA-16527; Implement request handling for updated KRaft RPCs [kafka]
jsancio commented on code in PR #16235:
URL: https://github.com/apache/kafka/pull/16235#discussion_r1706197212
##
clients/src/main/resources/common/message/FetchRequest.json:
##
@@ -100,7 +102,9 @@
{ "name": "LogStartOffset", "type": "int64", "versions": "5+",
"default": "-1", "ignorable": true,
"about": "The earliest available offset of the follower replica.
The field is only used when the request is sent by the follower."},
{ "name": "PartitionMaxBytes", "type": "int32", "versions": "0+",
- "about": "The maximum bytes to fetch from this partition. See
KIP-74 for cases where this limit may not be honored." }
+ "about": "The maximum bytes to fetch from this partition. See
KIP-74 for cases where this limit may not be honored." },
+{ "name": "ReplicaDirectoryId", "type": "uuid", "versions": "17+",
"taggedVersions": "17+", "tag": 0,
Review Comment:
@chia7712 this is a tagged field so it should show as an unknown tagged
field on older client. KRaft only support Fetch with version 12+ and tag fields
(flexible version) was added in version 12. Are you not seeing that? We should
be exercising this in `KafkaRaftClientTest` and `RaftClientTestContext`. This
is how we test different version in the KRaft tests using
`RaftClientTestContxt.fetchRpcVersion`:
```java
private short fetchRpcVersion() {
if (kip853Rpc) {
return 17;
} else {
return 16;
}
}
```
What error are you getting @chia7712 ?
--
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
Re: [PR] KAFKA-16527; Implement request handling for updated KRaft RPCs [kafka]
jolshan commented on code in PR #16235:
URL: https://github.com/apache/kafka/pull/16235#discussion_r1706085027
##
clients/src/main/resources/common/message/FetchRequest.json:
##
@@ -100,7 +102,9 @@
{ "name": "LogStartOffset", "type": "int64", "versions": "5+",
"default": "-1", "ignorable": true,
"about": "The earliest available offset of the follower replica.
The field is only used when the request is sent by the follower."},
{ "name": "PartitionMaxBytes", "type": "int32", "versions": "0+",
- "about": "The maximum bytes to fetch from this partition. See
KIP-74 for cases where this limit may not be honored." }
+ "about": "The maximum bytes to fetch from this partition. See
KIP-74 for cases where this limit may not be honored." },
+{ "name": "ReplicaDirectoryId", "type": "uuid", "versions": "17+",
"taggedVersions": "17+", "tag": 0,
Review Comment:
Thanks @chia7712! If you open a PR for this I will take a look since I would
like these tests to be fixed.
--
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
Re: [PR] KAFKA-16527; Implement request handling for updated KRaft RPCs [kafka]
chia7712 commented on code in PR #16235:
URL: https://github.com/apache/kafka/pull/16235#discussion_r1706046846
##
clients/src/main/resources/common/message/FetchRequest.json:
##
@@ -100,7 +102,9 @@
{ "name": "LogStartOffset", "type": "int64", "versions": "5+",
"default": "-1", "ignorable": true,
"about": "The earliest available offset of the follower replica.
The field is only used when the request is sent by the follower."},
{ "name": "PartitionMaxBytes", "type": "int32", "versions": "0+",
- "about": "The maximum bytes to fetch from this partition. See
KIP-74 for cases where this limit may not be honored." }
+ "about": "The maximum bytes to fetch from this partition. See
KIP-74 for cases where this limit may not be honored." },
+{ "name": "ReplicaDirectoryId", "type": "uuid", "versions": "17+",
"taggedVersions": "17+", "tag": 0,
Review Comment:
I file https://issues.apache.org/jira/browse/KAFKA-17276 for it
--
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
Re: [PR] KAFKA-16527; Implement request handling for updated KRaft RPCs [kafka]
chia7712 commented on code in PR #16235:
URL: https://github.com/apache/kafka/pull/16235#discussion_r1706045071
##
clients/src/main/resources/common/message/FetchRequest.json:
##
@@ -100,7 +102,9 @@
{ "name": "LogStartOffset", "type": "int64", "versions": "5+",
"default": "-1", "ignorable": true,
"about": "The earliest available offset of the follower replica.
The field is only used when the request is sent by the follower."},
{ "name": "PartitionMaxBytes", "type": "int32", "versions": "0+",
- "about": "The maximum bytes to fetch from this partition. See
KIP-74 for cases where this limit may not be honored." }
+ "about": "The maximum bytes to fetch from this partition. See
KIP-74 for cases where this limit may not be honored." },
+{ "name": "ReplicaDirectoryId", "type": "uuid", "versions": "17+",
"taggedVersions": "17+", "tag": 0,
Review Comment:
> I guess that is slightly different. We should always be able to write a
request for a lower version.
yep, that is different. The case we discuss is "KafkaRaftClient" . IMHO, we
should update the request if the version is <17. for example:
```java
if (version < 17) {
fetchRequestData.topics()
.forEach(t -> t.partitions()
.forEach(p -> p.setReplicaDirectoryId(Uuid.ZERO_UUID)));
}
```
https://github.com/apache/kafka/blob/5596f9e1d561dfef46a83fbd7264e6745ca538cc/clients/src/main/java/org/apache/kafka/common/requests/FetchRequest.java#L142
--
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
Re: [PR] KAFKA-16527; Implement request handling for updated KRaft RPCs [kafka]
jolshan commented on code in PR #16235:
URL: https://github.com/apache/kafka/pull/16235#discussion_r1705955663
##
clients/src/main/resources/common/message/FetchRequest.json:
##
@@ -100,7 +102,9 @@
{ "name": "LogStartOffset", "type": "int64", "versions": "5+",
"default": "-1", "ignorable": true,
"about": "The earliest available offset of the follower replica.
The field is only used when the request is sent by the follower."},
{ "name": "PartitionMaxBytes", "type": "int32", "versions": "0+",
- "about": "The maximum bytes to fetch from this partition. See
KIP-74 for cases where this limit may not be honored." }
+ "about": "The maximum bytes to fetch from this partition. See
KIP-74 for cases where this limit may not be honored." },
+{ "name": "ReplicaDirectoryId", "type": "uuid", "versions": "17+",
"taggedVersions": "17+", "tag": 0,
Review Comment:
Anyway, I guess that is slightly different. We should always be able to
write a request for a lower version.
--
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
Re: [PR] KAFKA-16527; Implement request handling for updated KRaft RPCs [kafka]
jolshan commented on code in PR #16235:
URL: https://github.com/apache/kafka/pull/16235#discussion_r1705954077
##
clients/src/main/resources/common/message/FetchRequest.json:
##
@@ -100,7 +102,9 @@
{ "name": "LogStartOffset", "type": "int64", "versions": "5+",
"default": "-1", "ignorable": true,
"about": "The earliest available offset of the follower replica.
The field is only used when the request is sent by the follower."},
{ "name": "PartitionMaxBytes", "type": "int32", "versions": "0+",
- "about": "The maximum bytes to fetch from this partition. See
KIP-74 for cases where this limit may not be honored." }
+ "about": "The maximum bytes to fetch from this partition. See
KIP-74 for cases where this limit may not be honored." },
+{ "name": "ReplicaDirectoryId", "type": "uuid", "versions": "17+",
"taggedVersions": "17+", "tag": 0,
Review Comment:
I saw this ticket, but not a lot of detail:
https://issues.apache.org/jira/browse/KAFKA-17018
--
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
Re: [PR] KAFKA-16527; Implement request handling for updated KRaft RPCs [kafka]
chia7712 commented on code in PR #16235:
URL: https://github.com/apache/kafka/pull/16235#discussion_r1672760435
##
clients/src/main/resources/common/message/FetchRequest.json:
##
@@ -100,7 +102,9 @@
{ "name": "LogStartOffset", "type": "int64", "versions": "5+",
"default": "-1", "ignorable": true,
"about": "The earliest available offset of the follower replica.
The field is only used when the request is sent by the follower."},
{ "name": "PartitionMaxBytes", "type": "int32", "versions": "0+",
- "about": "The maximum bytes to fetch from this partition. See
KIP-74 for cases where this limit may not be honored." }
+ "about": "The maximum bytes to fetch from this partition. See
KIP-74 for cases where this limit may not be honored." },
+{ "name": "ReplicaDirectoryId", "type": "uuid", "versions": "17+",
"taggedVersions": "17+", "tag": 0,
Review Comment:
Pardon me, does this new field obstruct new broker from communicating to old
quorum controller? For example, the quorum controller is running with 3.6.2.
The broker based on this patch will not be able to generate `FetchRequest` for
the controller, because the version=15 can't accept non-default
`ReplicaDirectoryId`
--
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
Re: [PR] KAFKA-16527; Implement request handling for updated KRaft RPCs [kafka]
cmccabe commented on code in PR #16235:
URL: https://github.com/apache/kafka/pull/16235#discussion_r1653309009
##
raft/src/main/java/org/apache/kafka/raft/internals/ReplicaKey.java:
##
@@ -21,7 +21,9 @@
import java.util.Objects;
import java.util.Optional;
-public final class ReplicaKey {
+public final class ReplicaKey implements Comparable {
+public static final Uuid NO_DIRECTORY_ID = Uuid.ZERO_UUID;
+
private final int id;
private final Optional directoryId;
Review Comment:
I don't think having a special class would be better (in fact, I think it
would be strictly worse than just using `Optional`)
If you feel strongly about this, then I guess just leave it as Optional. I
find this messy (for one thing, you have to make sure to remember to transform
UUID_ZERO into Optional.empty in all the relevant places) but maybe I'm not
going to convince you here :)
##
server-common/src/main/java/org/apache/kafka/server/common/MetadataVersion.java:
##
@@ -408,7 +408,9 @@ public short partitionRecordVersion() {
}
public short fetchRequestVersion() {
-if (this.isAtLeast(IBP_3_7_IV4)) {
+if (this.isAtLeast(IBP_4_0_IV0)) {
Review Comment:
we will fix this in https://github.com/apache/kafka/pull/16347/commits
--
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
Re: [PR] KAFKA-16527; Implement request handling for updated KRaft RPCs [kafka]
jsancio commented on code in PR #16235: URL: https://github.com/apache/kafka/pull/16235#discussion_r1644751861 ## core/src/main/scala/kafka/server/KafkaServer.scala: ## @@ -440,6 +441,8 @@ class KafkaServer( threadNamePrefix, CompletableFuture.completedFuture(quorumVoters), QuorumConfig.parseBootstrapServers(config.quorumBootstrapServers), +// Endpoint information is only needed for controllers (voters). ZK brokers can never be controllers +Endpoints.empty(), Review Comment: Fixed the comment. -- 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
Re: [PR] KAFKA-16527; Implement request handling for updated KRaft RPCs [kafka]
jsancio commented on code in PR #16235: URL: https://github.com/apache/kafka/pull/16235#discussion_r1644748799 ## clients/src/main/resources/common/message/VoteRequest.json: ## @@ -18,30 +18,36 @@ "type": "request", "listeners": ["controller"], "name": "VoteRequest", - "validVersions": "0", + "validVersions": "0-1", Review Comment: Fixed. ## clients/src/main/resources/common/message/VoteResponse.json: ## @@ -17,29 +17,37 @@ "apiKey": 52, "type": "response", "name": "VoteResponse", - "validVersions": "0", + "validVersions": "0-1", Review Comment: Fixed. -- 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
Re: [PR] KAFKA-16527; Implement request handling for updated KRaft RPCs [kafka]
jsancio commented on code in PR #16235: URL: https://github.com/apache/kafka/pull/16235#discussion_r1644747026 ## clients/src/main/resources/common/message/FetchSnapshotRequest.json: ## @@ -18,7 +18,7 @@ "type": "request", "listeners": ["controller"], "name": "FetchSnapshotRequest", - "validVersions": "0", + "validVersions": "0-1", Review Comment: Fixed. ## clients/src/main/resources/common/message/FetchSnapshotResponse.json: ## @@ -17,7 +17,7 @@ "apiKey": 59, "type": "response", "name": "FetchSnapshotResponse", - "validVersions": "0", + "validVersions": "0-1", Review Comment: Fixed. -- 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
Re: [PR] KAFKA-16527; Implement request handling for updated KRaft RPCs [kafka]
jsancio commented on code in PR #16235: URL: https://github.com/apache/kafka/pull/16235#discussion_r1644744740 ## clients/src/main/resources/common/message/EndQuorumEpochResponse.json: ## @@ -17,25 +17,35 @@ "apiKey": 54, "type": "response", "name": "EndQuorumEpochResponse", - "validVersions": "0", - "flexibleVersions": "none", + "validVersions": "0-1", Review Comment: Fixed. -- 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
Re: [PR] KAFKA-16527; Implement request handling for updated KRaft RPCs [kafka]
jsancio commented on code in PR #16235: URL: https://github.com/apache/kafka/pull/16235#discussion_r1644744458 ## clients/src/main/resources/common/message/EndQuorumEpochRequest.json: ## @@ -18,26 +18,41 @@ "type": "request", "listeners": ["controller"], "name": "EndQuorumEpochRequest", - "validVersions": "0", - "flexibleVersions": "none", + "validVersions": "0-1", Review Comment: Fixed. -- 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
Re: [PR] KAFKA-16527; Implement request handling for updated KRaft RPCs [kafka]
jsancio commented on code in PR #16235: URL: https://github.com/apache/kafka/pull/16235#discussion_r1644728890 ## clients/src/main/resources/common/message/BeginQuorumEpochResponse.json: ## @@ -17,25 +17,35 @@ "apiKey": 53, "type": "response", "name": "BeginQuorumEpochResponse", - "validVersions": "0", - "flexibleVersions": "none", + "validVersions": "0-1", Review Comment: Fixed. -- 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
Re: [PR] KAFKA-16527; Implement request handling for updated KRaft RPCs [kafka]
jsancio commented on code in PR #16235: URL: https://github.com/apache/kafka/pull/16235#discussion_r1644723447 ## clients/src/main/resources/common/message/BeginQuorumEpochRequest.json: ## @@ -18,24 +18,37 @@ "type": "request", "listeners": ["controller"], "name": "BeginQuorumEpochRequest", - "validVersions": "0", - "flexibleVersions": "none", + "validVersions": "0-1", Review Comment: Done. I just realized that I missed one part of the request handling of this RPC. -- 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
Re: [PR] KAFKA-16527; Implement request handling for updated KRaft RPCs [kafka]
jsancio commented on code in PR #16235: URL: https://github.com/apache/kafka/pull/16235#discussion_r1644714171 ## clients/src/main/resources/common/message/FetchResponse.json: ## @@ -47,7 +47,7 @@ // Version 15 is the same as version 14 (KIP-903). // // Version 16 adds the 'NodeEndpoints' field (KIP-951). - "validVersions": "0-16", + "validVersions": "0-17", Review Comment: Fixed. -- 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
Re: [PR] KAFKA-16527; Implement request handling for updated KRaft RPCs [kafka]
showuon commented on code in PR #16235: URL: https://github.com/apache/kafka/pull/16235#discussion_r1644362575 ## clients/src/main/resources/common/message/FetchResponse.json: ## @@ -47,7 +47,7 @@ // Version 15 is the same as version 14 (KIP-903). // // Version 16 adds the 'NodeEndpoints' field (KIP-951). - "validVersions": "0-16", + "validVersions": "0-17", Review Comment: We need to add a comment to mention why we need to bump the version. ## clients/src/main/resources/common/message/BeginQuorumEpochResponse.json: ## @@ -17,25 +17,35 @@ "apiKey": 53, "type": "response", "name": "BeginQuorumEpochResponse", - "validVersions": "0", - "flexibleVersions": "none", + "validVersions": "0-1", Review Comment: Please add a comment to mention what changed in v.1. ## clients/src/main/resources/common/message/EndQuorumEpochResponse.json: ## @@ -17,25 +17,35 @@ "apiKey": 54, "type": "response", "name": "EndQuorumEpochResponse", - "validVersions": "0", - "flexibleVersions": "none", + "validVersions": "0-1", Review Comment: Please add a comment to mention what changed in v.1. ## clients/src/main/resources/common/message/FetchSnapshotResponse.json: ## @@ -17,7 +17,7 @@ "apiKey": 59, "type": "response", "name": "FetchSnapshotResponse", - "validVersions": "0", + "validVersions": "0-1", Review Comment: Please add a comment to mention what changed in v.1. ## core/src/main/scala/kafka/server/KafkaServer.scala: ## @@ -440,6 +441,8 @@ class KafkaServer( threadNamePrefix, CompletableFuture.completedFuture(quorumVoters), QuorumConfig.parseBootstrapServers(config.quorumBootstrapServers), +// Endpoint information is only needed for controllers (voters). ZK brokers can never be controllers +Endpoints.empty(), Review Comment: Should we make it clear that `ZK brokers can never be [KRaft] controllers`. ## clients/src/main/resources/common/message/BeginQuorumEpochRequest.json: ## @@ -18,24 +18,37 @@ "type": "request", "listeners": ["controller"], "name": "BeginQuorumEpochRequest", - "validVersions": "0", - "flexibleVersions": "none", + "validVersions": "0-1", Review Comment: Please add a comment to mention what changed in v.1. ## clients/src/main/resources/common/message/EndQuorumEpochRequest.json: ## @@ -18,26 +18,41 @@ "type": "request", "listeners": ["controller"], "name": "EndQuorumEpochRequest", - "validVersions": "0", - "flexibleVersions": "none", + "validVersions": "0-1", Review Comment: Please add a comment to mention what changed in v.1. ## clients/src/main/resources/common/message/FetchSnapshotRequest.json: ## @@ -18,7 +18,7 @@ "type": "request", "listeners": ["controller"], "name": "FetchSnapshotRequest", - "validVersions": "0", + "validVersions": "0-1", Review Comment: Please add a comment to mention what changed in v.1. ## clients/src/main/resources/common/message/VoteResponse.json: ## @@ -17,29 +17,37 @@ "apiKey": 52, "type": "response", "name": "VoteResponse", - "validVersions": "0", + "validVersions": "0-1", Review Comment: ditto ## clients/src/main/resources/common/message/VoteRequest.json: ## @@ -18,30 +18,36 @@ "type": "request", "listeners": ["controller"], "name": "VoteRequest", - "validVersions": "0", + "validVersions": "0-1", Review Comment: Please add a comment to mention what changed in v.1. -- 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
[PR] KAFKA-16527; Implement request handling for updated KRaft RPCs [kafka]
jsancio opened a new pull request, #16235: URL: https://github.com/apache/kafka/pull/16235 DRAFT ### Committer Checklist (excluded from commit message) - [ ] Verify design and implementation - [ ] Verify test coverage and CI build status - [ ] Verify documentation (including upgrade notes) -- 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
