gavinchou commented on code in PR #37669:
URL: https://github.com/apache/doris/pull/37669#discussion_r1681393795
##########
cloud/src/meta-service/meta_service_txn.cpp:
##########
@@ -2544,6 +2559,100 @@ void
MetaServiceImpl::abort_sub_txn(::google::protobuf::RpcController* controlle
response->mutable_txn_info()->CopyFrom(txn_info);
}
+void
MetaServiceImpl::abort_txn_by_coordinate_be(::google::protobuf::RpcController*
controller,
Review Comment:
add an UT for it
##########
fe/fe-core/src/main/java/org/apache/doris/alter/SchemaChangeJobV2.java:
##########
@@ -792,10 +796,20 @@ private void cancelInternal() {
jobState = JobState.CANCELLED;
}
- // Check whether transactions of the given database which txnId is less
than 'watershedTxnId' are finished.
- protected boolean isPreviousLoadFinished() throws AnalysisException {
- return
Env.getCurrentGlobalTransactionMgr().isPreviousTransactionsFinished(
+ // Check whether transactions of the given database which txnId is less
than 'watershedTxnId' are finished
+ // and abort it if it is failed.
+ // If return true, all previous load is finish
+ protected boolean checkFailedPreviousLoadAndAbort() throws UserException {
+ List<TransactionState> unFinishedTxns =
Env.getCurrentGlobalTransactionMgr().getUnFinishedPreviousLoad(
watershedTxnId, dbId, Lists.newArrayList(tableId));
+ if (Config.enable_abort_txn_by_checking_conflict_txn) {
+ List<TransactionState> failedTxns =
GlobalTransactionMgr.checkFailedTxns(unFinishedTxns);
+ for (TransactionState txn : failedTxns) {
+ Env.getCurrentGlobalTransactionMgr()
+ .abortTransaction(txn.getDbId(),
txn.getTransactionId(), "Cancel by schema change");
+ }
+ }
+ return unFinishedTxns.isEmpty();
Review Comment:
Is `unFinishedTxns.empty() == true` if
`enable_abort_txn_by_checking_conflict_txn` is set to true?
##########
cloud/src/meta-service/meta_service.h:
##########
@@ -89,6 +89,11 @@ class MetaServiceImpl : public cloud::MetaService {
CheckTxnConflictResponse* response,
::google::protobuf::Closure* done) override;
+ void abort_txn_by_coordinate_be(::google::protobuf::RpcController*
controller,
Review Comment:
Naming `abort_txn_by_coordinate_be` should be more flexible,
we can abort txn with a coordinator no mater it a BE or FE.
Name it `abort_txn_with_coordinator` or even just reuse `abort_txn`
##########
gensrc/proto/cloud.proto:
##########
@@ -772,6 +772,17 @@ message GetCurrentMaxTxnResponse {
optional int64 current_max_txn_id = 2;
}
+message AbortTxnByCoordinateBeRequest {
Review Comment:
This is a coordinator, do not narrow it down with "BE" in it.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
