-
Notifications
You must be signed in to change notification settings - Fork 1.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[BugFix] fix inconsistent transaction process if exception thrown #56153
Conversation
Signed-off-by: starrocks-xupeng <xupeng@starrocks.com>
transactionState.afterStateTransform(TransactionStatus.VISIBLE, txnOperated, ""); | ||
} catch (Throwable t) { | ||
LOG.warn("transaction after state transform failed: {}", transactionState, t); | ||
} | ||
} | ||
persistTxnStateInTxnLevelLock(transactionState); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The most risky bug in this code is:
Returning null
when txnOperated
is false may not be handled properly by the caller, leading to potential null pointer exceptions or logical errors downstream.
You can modify the code like this:
// Check for txnOperated before returning null
if (!txnOperated) {
LOG.error("Transaction operation failed. Returning default value or taking corrective action.");
// Handle the null return case more robustly here, e.g., return an empty VisibleStateWaiter.
return new VisibleStateWaiter(); // Example, adjust to actual required behavior
}
fe/fe-core/src/main/java/com/starrocks/lake/compaction/CompactionScheduler.java
Show resolved
Hide resolved
|
[Java-Extensions Incremental Coverage Report]✅ pass : 0 / 0 (0%) |
[FE Incremental Coverage Report]❌ fail : 16 / 38 (42.11%) file detail
|
[BE Incremental Coverage Report]✅ pass : 0 / 0 (0%) |
fe/fe-core/src/main/java/com/starrocks/lake/compaction/CompactionScheduler.java
Show resolved
Hide resolved
fe/fe-core/src/main/java/com/starrocks/transaction/DatabaseTransactionMgr.java
Show resolved
Hide resolved
@Mergifyio backport branch-3.4 |
@Mergifyio backport branch-3.3 |
✅ Backports have been created
|
✅ Backports have been created
|
…ckport StarRocks#56153) Signed-off-by: starrocks-xupeng <xupeng@starrocks.com>
…ckport StarRocks#56153) Signed-off-by: starrocks-xupeng <xupeng@starrocks.com>
#56242 |
#56243 |
Why I'm doing:
What I'm doing:
if step 1 succeeds, step 2 must not throw exception, avoid causing inconsistency between memory info(step 1) and disk info(step 3)
What type of PR is this:
Does this PR entail a change in behavior?
If yes, please specify the type of change:
Checklist:
Bugfix cherry-pick branch check: