rreddy-22 commented on code in PR #19429:
URL: https://github.com/apache/kafka/pull/19429#discussion_r2051195451
##########
clients/src/main/java/org/apache/kafka/clients/producer/internals/TransactionManager.java:
##########
@@ -273,17 +276,29 @@ public TransactionManager(final LogContext logContext,
this.retryBackoffMs = retryBackoffMs;
this.txnPartitionMap = new TxnPartitionMap(logContext);
this.apiVersions = apiVersions;
+ this.enable2PC = enable2PC;
}
void setPoisonStateOnInvalidTransition(boolean shouldPoisonState) {
shouldPoisonStateOnInvalidTransition.set(shouldPoisonState);
}
public synchronized TransactionalRequestResult initializeTransactions() {
- return initializeTransactions(ProducerIdAndEpoch.NONE);
+ return initializeTransactions(ProducerIdAndEpoch.NONE, false);
Review Comment:
Yes, I was also unsure about this. The main reason I decided to keep the
overload was that all the existing tests currently use it. I figured anyone who
doesn’t particularly care about keepPrepared shouldn’t have to deal with the
additional argument.
That said, I know that there is another school of thought that is not to
have surprise defaults and to make callers set parameters explicitly, which I
agree with.
I just didn't know how much change we wanted to make to the existing
internal method so lmk whichever works
--
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
