AndrewJSchofield commented on code in PR #24000:
URL: https://github.com/apache/kafka/pull/24000#discussion_r4421632983
##########
server-common/src/main/java/org/apache/kafka/server/common/MetadataVersion.java:
##########
@@ -264,8 +264,8 @@ public boolean isControllerUnregistrationSupported() {
return this.isAtLeast(MetadataVersion.IBP_4_4_IV2);
}
- public boolean isRequestHeaderV3Supported() {
- return this.isAtLeast(MetadataVersion.IBP_4_5_IV0);
+ public short requestHeaderVersion() {
Review Comment:
I think this is perhaps named too broadly. If `MetadataVersion` is at least
`IBP_4_5_IV0`, then all requests except `SaslAuthenticate` will be RV3.
However, if it's lower, then it's nuanced. My suggestion is that you should
call this `shareGroupStateRequestHeaderVersion()`. When you extend this to
other RPCs, then the logic might be subtly different. In particular, I think
that `MetadataVersion.MINIMUM_VERSION.requestHeaderVersion()` is problematic,
because that's `IBP_3_3_IV2` which is ancient. Some RPCs were RV1 back then I
think.
--
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]
