jolshan commented on code in PR #14774:
URL: https://github.com/apache/kafka/pull/14774#discussion_r1414708109
##########
core/src/main/scala/kafka/server/KafkaApis.scala:
##########
@@ -708,23 +708,40 @@ class KafkaApis(val requestChannel: RequestChannel,
}
}
- if (authorizedRequestInfo.isEmpty)
- sendResponseCallback(Map.empty)
- else {
- val internalTopicsAllowed = request.header.clientId ==
AdminUtils.ADMIN_CLIENT_ID
+ val internalTopicsAllowed = request.header.clientId ==
AdminUtils.ADMIN_CLIENT_ID
+ val transactionVerificationEntries = new
ReplicaManager.TransactionVerificationEntries
- // call the replica manager to append messages to the replicas
+ def postVerificationCallback(newRequestLocal: RequestLocal)
Review Comment:
The problem I had was that I needed to pass the errors and the verification
guards into the postVerificationCallback.
The alternative is to create yet another method in ReplicaManager for the
transaction produce path. I was hoping to avoid many methods with 7+ arguments
that look similar but if we really want to move out of Kafka Apis, we can do
that.
I'm not sure I fully agree that the code was good before. I think it is
better to keep append records clean and not incorporate transaction
verification. That's why the transaction verification has the pre-verify (add
to the txn manager) and post verification (tidy the results and put them in a
form for the post verification callback).
If we want to move this code out of KafkaApis, I think it only makes sense
to make a separate method. The other stages need to remain generic for the
group coordinator usage.
--
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
