rajinisivaram commented on code in PR #22512:
URL: https://github.com/apache/kafka/pull/22512#discussion_r3412301788
##########
clients/src/main/resources/common/message/ApiVersionsRequest.json:
##########
@@ -27,7 +27,6 @@
// Version 5 introduces ClusterId and NodeId checking and
REBOOTSTRAP_REQUIRED error (KIP-1242).
"validVersions": "0-5",
"flexibleVersions": "3+",
- "latestVersionUnstable": true,
Review Comment:
This change is not related to the KIP?
##########
clients/src/main/java/org/apache/kafka/clients/NetworkClient.java:
##########
@@ -1140,8 +1140,11 @@ private void handleInitiateApiVersionRequests(long now) {
int nodeId = Integer.parseInt(node);
// In order to allow separate connections to coordinators,
the client uses large positive node ID values
// (Integer.MAX_VALUE - nodeId) for these connections
which do not match the target broker's actual node ID.
- // To avoid those, only check if the node ID is less than
half of Integer.MAX_VALUE.
- if (clusterId != null && nodeId >= 0 && nodeId <
Integer.MAX_VALUE / 2) {
+ // We can get the real node ID by subtracting from
Integer.MAX_VALUE.
+ if (nodeId > Integer.MAX_VALUE / 2) {
Review Comment:
There is an assumption here that node ids are less than `Integer.MAX_VALUE /
2`. But that is not a documented limitation for broker ids, right? Isn't it
possible that some deployments use larger broker ids, in which case, the larger
number is the actual broker id and the smaller one is the coordinator id?
--
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]
