artemlivshits commented on code in PR #16840:
URL: https://github.com/apache/kafka/pull/16840#discussion_r1725940379
##########
clients/src/main/java/org/apache/kafka/clients/producer/internals/TransactionManager.java:
##########
@@ -980,6 +1002,28 @@ void handleCoordinatorReady() {
null;
this.coordinatorSupportsBumpingEpoch = initProducerIdVersion != null &&
initProducerIdVersion.maxVersion() >= 3;
+
+ if (nodeApiVersions == null) return;
+
+ if (nodeApiVersions.finalizedFeatures() != null) {
+ /*
+ To enable the transaction V2, it requires:
+ 1. transaction.version finalized version >= 2
+ 2. The ProduceRequest max version >
ProducerRequest.LAST_BEFORE_TRANSACTION_V2_VERSION
Review Comment:
Yeah, if we enable v2 before all brokers support new produce requests, that
would break the protocol, but this check doesn't do it correctly anyway because
it's just a coordinator and we may produce to different brokers.
##########
clients/src/main/java/org/apache/kafka/clients/producer/internals/TransactionManager.java:
##########
@@ -395,6 +403,8 @@ public synchronized void maybeAddPartition(TopicPartition
topicPartition) {
" to transaction while in state " + currentState);
} else if (isPartitionAdded(topicPartition) ||
isPartitionPendingAdd(topicPartition)) {
return;
+ } else if (coordinatorSupportsTransactionV2) {
+ txnPartitionMap.getOrCreate(topicPartition);
Review Comment:
I think this should be fine, what would be the problem? With transactional
protocol v2 we don't really need to keep track on the client of what partitions
were added, so maybe we shouldn't even add it to `partitionsInTransaction` set,
just make sure that we don't block sends for partitions that are not present
there. Also, just because we've got a failure from a produce request doesn't
mean we didn't add a partition, on failure we don't know for sure.
--
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]