Copilot commented on code in PR #8042:
URL: https://github.com/apache/incubator-seata/pull/8042#discussion_r3042646451


##########
test-suite/seata-benchmark-cli/src/main/java/org/apache/seata/benchmark/executor/WorkloadGenerator.java:
##########
@@ -102,12 +102,7 @@ public void start() {
     private void executeTransaction() {
         try {
             TransactionRecord record = executor.execute();
-
-            if (record.isSuccess()) {
-                metrics.recordSuccess(record.getDuration());
-            } else {
-                metrics.recordFailure(record.getDuration());
-            }
+            metrics.recordTransaction(record.getStatus(), 
record.getDuration());
 

Review Comment:
   This switched to `recordTransaction(record.getStatus(), 
record.getDuration())`, but the exception path in the same method still records 
`recordFailure(0)`, which injects 0ms samples and doesn't populate the new 
per-status breakdown counters. Consider capturing elapsed time and calling 
`recordTransaction(STATUS_FAILED/STATUS_UNKNOWN, elapsedMs)` in the catch path 
to keep latency stats and status breakdown accurate.



##########
test-suite/seata-benchmark-cli/src/main/java/org/apache/seata/benchmark/executor/SagaModeExecutor.java:
##########
@@ -187,19 +194,15 @@ private TransactionRecord executeRealMode() {
             ExecutionStatus executionStatus = instance.getStatus();
             ExecutionStatus compensationStatus = 
instance.getCompensationStatus();
 
-            if (ExecutionStatus.SU.equals(executionStatus)) {
+            if (ExecutionStatus.SU.equals(compensationStatus)) {
+                status = STATUS_COMPENSATED;
+            } else if (ExecutionStatus.FA.equals(compensationStatus)) {
+                status = STATUS_COMPENSATION_FAILED;

Review Comment:
   `compensationStatus` can be `ExecutionStatus.UN` (unknown/retryable 
compensation state). With the current ordering, `UN` falls through and can end 
up classified as `STATUS_FAILED` (execution failed) instead of a 
compensation-failed/unknown end state. Consider explicitly handling 
`ExecutionStatus.UN` for `compensationStatus` and mapping it to 
`STATUS_COMPENSATION_FAILED` or `STATUS_UNKNOWN` so Saga end-state reporting 
stays accurate.
   ```suggestion
                   status = STATUS_COMPENSATION_FAILED;
               } else if (ExecutionStatus.UN.equals(compensationStatus)) {
                   status = STATUS_UNKNOWN;
   ```



-- 
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]

Reply via email to