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