hachikuji commented on a change in pull request #10129:
URL: https://github.com/apache/kafka/pull/10129#discussion_r578876834
##########
File path: raft/src/main/java/org/apache/kafka/raft/KafkaRaftClient.java
##########
@@ -1751,6 +1768,7 @@ private FetchRequestData buildFetchRequest() {
});
return request
.setMaxWaitMs(fetchMaxWaitMs)
+ .setClusterId(clusterId.toString())
Review comment:
One small detail which is probably ok. The clusterId field in the fetch
schema is not currently marked as ignorable. That should be ok since it is only
used in the raft implementation which can guarantee that we will have version
12 and above. On the other hand, I don't see any harm making the field
ignorable since we are accepting a null value anyway. Is it worth changing that?
##########
File path: clients/src/main/java/org/apache/kafka/common/protocol/Errors.java
##########
@@ -354,7 +355,8 @@
"Requested position is not greater than or equal to zero, and less
than the size of the snapshot.",
PositionOutOfRangeException::new),
UNKNOWN_TOPIC_ID(100, "This server does not host this topic ID.",
UnknownTopicIdException::new),
- DUPLICATE_BROKER_REGISTRATION(101, "This broker ID is already in use.",
DuplicateBrokerRegistrationException::new);
+ DUPLICATE_BROKER_REGISTRATION(101, "This broker ID is already in use.",
DuplicateBrokerRegistrationException::new),
+ INVALID_CLUSTER_ID(102, "The supplied cluster id is not valid.",
InvalidClusterIdException::new);
Review comment:
I couldn't find any existing `INVALID*` error code that seems to fit
this case. Usually "invalid" is reserved for cases where the field is
structurally invalid. For example, `INVALID_GROUP_ID` is used when the groupid
is empty. The closest similar case is `INVALID_PRODUCER_ID_MAPPING`.
We are going to add an INCONSISTENT_TOPIC_ID in
https://github.com/apache/kafka/pull/10143. Perhaps that is enough cover here?
The usage is similar: the request indicates an id which does not match the
local state.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
