Copilot commented on code in PR #67661:
URL: https://github.com/apache/doris/pull/67661#discussion_r3956749816
##########
fe/fe-core/src/main/java/org/apache/doris/alter/RollupJobV2.java:
##########
@@ -708,8 +708,14 @@ protected boolean checkFailedPreviousLoadAndAbort() throws
UserException {
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");
+ try {
+ Env.getCurrentGlobalTransactionMgr()
+ .abortTransaction(txn.getDbId(),
txn.getTransactionId(), "Cancel by schema change");
Review Comment:
The abort reason string says \"Cancel by schema change\" inside rollup code.
This makes logs/metrics misleading when diagnosing rollup-related aborts.
Consider changing the message to explicitly reference rollup (e.g., \"Cancel by
rollup\" or \"Cancel by rollup job\"), or reuse a shared constant if one exists
for conflict-txn abort reasons.
##########
fe/fe-core/src/main/java/org/apache/doris/transaction/GlobalTransactionMgr.java:
##########
@@ -515,6 +515,10 @@ public static boolean
checkFailedTxnsByCoordinator(TransactionState txn) {
public static List<TransactionState>
checkFailedTxns(List<TransactionState> conflictTxns) {
List<TransactionState> failedTxns = new ArrayList<>();
for (TransactionState txn : conflictTxns) {
+ TransactionStatus status = txn.getTransactionStatus();
+ if (status == TransactionStatus.COMMITTED ||
status.isFinalStatus()) {
Review Comment:
`status == TransactionStatus.COMMITTED` is redundant if `COMMITTED` is (now
or in future) treated as a final status, and it also embeds knowledge about
status semantics here. To make this easier to maintain, consider consolidating
the predicate (e.g., a single helper like `isAbortCandidate(status)` or
extending `TransactionStatus` with a clearer method) so callers don't need to
reason about which statuses should be skipped.
--
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]
