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: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to