codelipenghui commented on a change in pull request #10182:
URL: https://github.com/apache/pulsar/pull/10182#discussion_r611273642
##########
File path:
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java
##########
@@ -1777,7 +1778,16 @@ protected void handleNewTxn(CommandNewTxn command) {
log.debug("Receive new txn request {} to transaction meta store {}
from {}.",
requestId, tcId, remoteAddress);
}
-
service.pulsar().getTransactionMetadataStoreService().newTransaction(tcId,
command.getTxnTtlSeconds())
+ TransactionMetadataStoreService transactionMetadataStoreService =
+ service.pulsar().getTransactionMetadataStoreService();
+ if (transactionMetadataStoreService == null) {
+ ServiceUnitNotReadyException ex =
+ new ServiceUnitNotReadyException("Transaction manager is
not started or not enabled");
Review comment:
return ServiceUnitNotReadyException here is not reasonable here. We have
defined `NotAllowedError` and `TransactionCoordinatorNotFound` in the
PulsarApi.proto. If the broker disabled the TC, we should return
NotAllowedError to the client, so that the client should not to retry to open a
transaction. And if in some cases the transactionMetadataStoreService is
null(In principle this shouldn't happen), we can return
`TransactionCoordinatorNotFound` to the client.
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
