CalvinConfluent commented on code in PR #15657:
URL: https://github.com/apache/kafka/pull/15657#discussion_r1558097863
##########
core/src/main/scala/kafka/server/KafkaApis.scala:
##########
@@ -2715,6 +2721,10 @@ class KafkaApis(val requestChannel: RequestChannel,
} else if (!authHelper.authorize(request.context, READ, GROUP,
txnOffsetCommitRequest.data.groupId)) {
sendResponse(txnOffsetCommitRequest.getErrorResponse(Errors.GROUP_AUTHORIZATION_FAILED.exception))
CompletableFuture.completedFuture[Unit](())
+ } else if (!metadataCache.metadataVersion().isTransactionV2Enabled &&
txnOffsetCommitRequest.isTransactionV2Requested) {
+ // If the client requests to use transaction V2 but server side does not
supports it, return unsupported version.
Review Comment:
> TV is downgraded -- in this case, we can still handle the old request and
we should do so.
If I understand correctly, we should continue to serve the V2 requests but
let the downstream code return errors. In the txnOffsetCommitRequest example,
when the broker is on TV 1 and with TV 2 enabled image, if the broker receives
a new version txnOffsetCommitRequest, it should continue and verify the
transaction. If the offset partition has not been added to the transaction, we
should return the error. Also, when the client receives this error, it should
refresh the API versions.
##########
core/src/main/scala/kafka/server/KafkaApis.scala:
##########
@@ -2715,6 +2721,10 @@ class KafkaApis(val requestChannel: RequestChannel,
} else if (!authHelper.authorize(request.context, READ, GROUP,
txnOffsetCommitRequest.data.groupId)) {
sendResponse(txnOffsetCommitRequest.getErrorResponse(Errors.GROUP_AUTHORIZATION_FAILED.exception))
CompletableFuture.completedFuture[Unit](())
+ } else if (!metadataCache.metadataVersion().isTransactionV2Enabled &&
txnOffsetCommitRequest.isTransactionV2Requested) {
+ // If the client requests to use transaction V2 but server side does not
supports it, return unsupported version.
Review Comment:
> TV is downgraded -- in this case, we can still handle the old request and
we should do so.
If I understand correctly, we should continue to serve the V2 requests but
let the downstream code return errors. In the txnOffsetCommitRequest example,
when the broker is on TV 1 and with TV 2 enabled image, if the broker receives
a new version txnOffsetCommitRequest, it should continue and verify the
transaction. If the offset partition has not been added to the transaction, we
should return the error. Also, when the client receives this error, it should
refresh the API versions.
--
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]
