You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by "JNSimba (via GitHub)" <gi...@apache.org> on 2026/04/13 02:33:45 UTC
[PR] [fix](streaming-job) Fix NPE in StreamingInsertJob.replayOnCommitted during EditLog replay [doris]
JNSimba opened a new pull request, #62416:
URL: https://github.com/apache/doris/pull/62416
### What problem does this PR solve?
Issue Number: close #DORIS-25067
Problem Summary:
Fix NPE in `StreamingInsertJob.replayOnCommitted()` during FE checkpoint EditLog replay.
When a streaming insert task is canceled while its transaction is being committed, `beforeCommitted()` silently returned without setting `txnCommitAttachment`. The transaction still committed with a null attachment written to EditLog. During checkpoint replay, `replayOnCommitted()` calls `Preconditions.checkNotNull(txnState.getTxnCommitAttachment())`, throwing NPE.
Two issues fixed in `beforeCommitted()`:
1. Throw `TransactionException` when task is canceled instead of silent `return`, preventing commit with null attachment. Consistent with `RoutineLoadJob.executeBeforeCheck()` pattern.
2. Fix write lock leak — replaced broken `shouldReleaseLock` (always `false`) with `passCheck` pattern: release lock on failure in `finally`, keep it for `onStreamTaskSuccess/Fail()` to release on success.
### Release note
None
### Check List (For Author)
- Test
- [ ] Regression test
- [ ] Unit Test
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- [ ] Previous test can cover this change.
- Behavior changed:
- [x] Yes. When a streaming insert task is canceled during transaction commit, the transaction will now fail with TransactionException instead of committing with null attachment.
- Does this need documentation?
- [x] No.
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](streaming-job) Fix NPE in StreamingInsertJob.replayOnCommitted during EditLog replay [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #62416:
URL: https://github.com/apache/doris/pull/62416#issuecomment-4233664485
# FE UT Coverage Report
Increment line coverage ` 0.00% (0/6)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/62416_a81cd823dd207a33e41c382dd4658c4e1ac11972/fe_increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/62416_a81cd823dd207a33e41c382dd4658c4e1ac11972/fe_report/index.html)
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](streaming-job) Fix NPE in StreamingInsertJob.replayOnCommitted during EditLog replay [doris]
Posted by "zclllyybb (via GitHub)" <gi...@apache.org>.
zclllyybb commented on PR #62416:
URL: https://github.com/apache/doris/pull/62416#issuecomment-4249758523
/review
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](streaming-job) Fix NPE in StreamingInsertJob.replayOnCommitted during EditLog replay [doris]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #62416:
URL: https://github.com/apache/doris/pull/62416#discussion_r3076822942
##########
fe/fe-core/src/main/java/org/apache/doris/job/extensions/insert/streaming/StreamingInsertJob.java:
##########
@@ -1059,8 +1058,9 @@ public void beforeCommitted(TransactionState txnState) throws TransactionExcepti
loadStatistic.getFileNumber(),
loadStatistic.getTotalFileSizeB(),
offsetJson));
+ passCheck = true;
} finally {
- if (shouldReleaseLock) {
+ if (!passCheck) {
writeUnlock();
}
Review Comment:
`beforeCommitted()` intentionally keeps the write lock when `passCheck=true`, relying on `onStreamTaskSuccess()` / `onStreamTaskFail()` to release it later. However, task cancellation paths can skip those callbacks entirely (e.g., `StreamingInsertTask.onSuccess()` returns early when `isCanceled` is true, and `AbstractStreamingTask.onFail()` returns early when canceled), which can leave the job's write lock held indefinitely if the task becomes canceled after `passCheck` is set. Consider ensuring the lock is always released on all task terminal paths (including cancel), or change the design to always unlock in `beforeCommitted()` and explicitly lock/unlock within `onStreamTaskSuccess/Fail()` as needed.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](streaming-job) Fix NPE in StreamingInsertJob.replayOnCommitted during EditLog replay [doris]
Posted by "JNSimba (via GitHub)" <gi...@apache.org>.
JNSimba commented on PR #62416:
URL: https://github.com/apache/doris/pull/62416#issuecomment-4233408740
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](streaming-job) Fix NPE in StreamingInsertJob.replayOnCommitted during EditLog replay [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #62416:
URL: https://github.com/apache/doris/pull/62416#issuecomment-4249740493
PR approved by at least one committer and no changes requested.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](streaming-job) Fix NPE in StreamingInsertJob.replayOnCommitted during EditLog replay [doris]
Posted by "JNSimba (via GitHub)" <gi...@apache.org>.
JNSimba merged PR #62416:
URL: https://github.com/apache/doris/pull/62416
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](streaming-job) Fix NPE in StreamingInsertJob.replayOnCommitted during EditLog replay [doris]
Posted by "Thearas (via GitHub)" <gi...@apache.org>.
Thearas commented on PR #62416:
URL: https://github.com/apache/doris/pull/62416#issuecomment-4233404802
Thank you for your contribution to Apache Doris.
Don't know what should be done next? See [How to process your PR](https://cwiki.apache.org/confluence/display/DORIS/How+to+process+your+PR).
Please clearly describe your PR:
1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
3. What features were added. Why was this function added?
4. Which code was refactored and why was this part of the code refactored?
5. Which functions were optimized and what is the difference before and after the optimization?
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](streaming-job) Fix NPE in StreamingInsertJob.replayOnCommitted during EditLog replay [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #62416:
URL: https://github.com/apache/doris/pull/62416#discussion_r3076825090
##########
fe/fe-core/src/main/java/org/apache/doris/job/extensions/insert/streaming/StreamingInsertJob.java:
##########
@@ -1021,13 +1021,13 @@ public long getId() {
@Override
public void beforeCommitted(TransactionState txnState) throws TransactionException {
- boolean shouldReleaseLock = false;
writeLock();
+ boolean passCheck = false;
try {
if (runningStreamTask.getIsCanceled().get()) {
- log.info("streaming insert job {} task {} is canceled, skip beforeCommitted",
- getJobId(), runningStreamTask.getTaskId());
- return;
+ throw new TransactionException("streaming insert job " + getJobId()
Review Comment:
This changes a commit-time callback in the exact replay-sensitive path that caused the original NPE, but the PR still adds no FE unit or regression coverage for the cancel-during-commit case. Since the failure only surfaced later during EditLog replay, please add a test that cancels the streaming task during commit and then verifies replay / FE restart does not hit `checkNotNull(txnCommitAttachment)` again.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](streaming-job) Fix NPE in StreamingInsertJob.replayOnCommitted during EditLog replay [doris]
Posted by "JNSimba (via GitHub)" <gi...@apache.org>.
JNSimba commented on PR #62416:
URL: https://github.com/apache/doris/pull/62416#issuecomment-4240925713
/review
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](streaming-job) Fix NPE in StreamingInsertJob.replayOnCommitted during EditLog replay [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #62416:
URL: https://github.com/apache/doris/pull/62416#discussion_r3084391423
##########
fe/fe-core/src/main/java/org/apache/doris/job/extensions/insert/streaming/StreamingInsertJob.java:
##########
@@ -1059,8 +1058,9 @@ public void beforeCommitted(TransactionState txnState) throws TransactionExcepti
loadStatistic.getFileNumber(),
loadStatistic.getTotalFileSizeB(),
offsetJson));
+ passCheck = true;
Review Comment:
`passCheck = true` only proves that `beforeCommitted()` itself succeeded. From this point the write lock is intentionally left held, but there is still no release path if the commit fails *after* `beforeStateTransform(COMMITTED)` and *before* `afterCommitted()` runs. In `DatabaseTransactionMgr.commitTransaction()` that window includes `unprotectedCommitTransaction(...)`, `persistTransactionState(...)`, and `enqueueTransactionState(...)` (`fe/fe-core/src/main/java/org/apache/doris/transaction/DatabaseTransactionMgr.java:820-840` and `893-909`). If one of those throws, `afterCommitted()` is skipped, `StreamingInsertJob.afterAborted()` is empty, and the job can remain permanently write-locked.
Please add cleanup for that post-check failure window as well; otherwise this still leaves a lock leak in exactly the callback path this PR is trying to harden.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [fix](streaming-job) Fix NPE in StreamingInsertJob.replayOnCommitted during EditLog replay [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #62416:
URL: https://github.com/apache/doris/pull/62416#issuecomment-4241040728
PR approved by anyone and no changes requested.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org