[fix](cloud) Release warm-up destination on initialization failure - #67924
Merged
gavinchou merged 1 commit intoSep 17, 2026
Merged
Conversation
### What problem does this PR solve? Problem Summary: A cloud warm-up job registers its destination compute group before initializing tablet batches. If initialization throws, the outer `run()` handler only logs the exception: the job stays `PENDING` and keeps the destination registration. Other ONCE/PERIODIC jobs targeting that group cannot start. A later successful retry can recover the original job, but repeated initialization failures can block the group indefinitely because the warm-up timeout only applies to `RUNNING` jobs. Catch initialization failures before transitioning to `RUNNING` and reuse `cancel(..., false)` to persist the error and release the destination registration. ONCE jobs become `CANCELLED`; PERIODIC jobs remain `PENDING` and retry at their existing interval. Initialization has not submitted work to BEs, so this path does not send cleanup RPCs. Successful initialization retains the destination registration as before. ### Release note Release the destination compute group when cloud warm-up initialization fails, allowing subsequent warm-up jobs to proceed. Report the initialization error and preserve periodic retry scheduling. ### Check List (For Author) - Test: Unit Test. All 30 tests passed across `CloudWarmUpJobTest` (11), `CacheHotspotManagerSchedulerTest` (4), and `cloud.cache.CacheHotspotManagerTest` (15). New tests cover ONCE/PERIODIC initialization failure using the real destination registration map, persisted failure state, periodic retry, and continued mutual exclusion after successful initialization. Before the fix, both failure-injection cases failed at the subsequent-job registration assertion; both successful-initialization cases passed. - Validation commands: `./build.sh --fe -j100` passed, including Checkstyle; `./run-fe-ut.sh --run 'org.apache.doris.cloud.CloudWarmUpJobTest,org.apache.doris.cloud.CacheHotspotManagerSchedulerTest,org.apache.doris.cloud.cache.CacheHotspotManagerTest'` passed; `git diff --check` passed. No live cloud-cluster SQL regression was run. - Behavior changed: Yes. Failed ONCE initialization cancels the job; failed PERIODIC initialization releases the destination registration and waits for its next interval. - Does this need documentation: No. ### Check List (For Reviewer who merge this PR) - [ ] Confirm the release note - [ ] Confirm test cases - [ ] Confirm document - [ ] Add branch pick label
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
/review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Automated review result: no blocking issues or distinct inline findings at 0b7a7a4df86714031b866b882841c11655abe436. The review completed its main risk scan, full production/test passes, and dedicated initialization-lifecycle scan; all Round 1 reviewers converged with no new valuable findings.
Critical checkpoint conclusions:
- Goal and proof: The change closes the PENDING-state destination-registration leak. The new failure test uses the real registration map to prove ownership exists before the injected exception and is released afterward; the success control proves normal initialization retains ownership. Against the pre-fix path, the post-failure registration assertion would fail.
- Scope and clarity: The production change is localized to pre-RUNNING initialization and reuses the existing cancellation/persistence path. The test additions are focused on failure, success, serialization, and periodic retry behavior.
- Concurrency: Scheduler workers call synchronized
run(), while SQL/cluster-change cancellation calls synchronizedcancel()on the same reentrant job monitor. Destination ownership uses atomicputIfAbsentand conditional(cluster, jobId)removal, so this job cannot release another job's registration. The scheduler's active-job entry is removed infinally; no lock-order or deadlock issue was found. - Lifecycle and parallel paths: ONCE and legacy-null jobs become
CANCELLED; PERIODIC jobs return toPENDINGand retain the existing start-to-start retry interval; EVENT_DRIVEN jobs never own the destination registration. Initialization may issue read-only BE capacity/hotspot discovery RPCs, but no BE warm-upSET_JOBis sent until the later RUNNING pass, so skippingCLEAR_JOBon this failure path is correct. TABLE/TABLES behavior is unchanged. - Configuration and compatibility: No configuration, function symbol, storage format, protocol field, or serialized-field format changes are introduced.
- Persistence and failover: The failure state, error, and timestamps are journaled before local deregistration. Replay restores PENDING/CANCELLED state and periodic repeat detection while the transient destination map starts empty, so failover does not preserve the leaked registration. EditLog failure remains FE-fatal by subsystem contract.
- Error handling and observability: The warning includes the job ID and stack, the error is persisted for SHOW output, and existing start/finish metrics are updated. No recoverable cleanup-side exception or silent failure path was substantiated.
- Tests and results: Static inspection confirms deterministic ONCE/PERIODIC failure and success coverage, no warm-up cleanup RPC, persisted fields, interval eligibility, and retry mutual exclusion. Per the review contract, I did not run builds or tests; the author's reported 30 targeted tests and FE build were not independently rerun. Current GitHub evidence independently shows CheckStyle passing. No live cloud-cluster end-to-end test was reported.
- Data, transactions, and FE-BE propagation: No data-write, transaction, visibility, or new FE-BE variable path is involved.
- Performance: The added cancellation/journal/deregistration work is exceptional-path only; no normal-path performance regression was identified.
- Additional user focus: None was provided; the full PR scope was reviewed.
Overall opinion: the patch is small, coherent, and adequately unit-tested for the reported defect. Static review approves the change; no inline comments are needed.
Contributor
Author
|
run buildall |
Contributor
TPC-H: Total hot run time: 16990 ms |
Contributor
TPC-DS: Total hot run time: 83150 ms |
Contributor
ClickBench: Total hot run time: 14.9 s |
Contributor
FE Regression Coverage ReportIncrement line coverage |
Contributor
Author
|
run feut |
gavinchou
approved these changes
Sep 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
Problem Summary:
A cloud warm-up job registers its destination compute group before initializing tablet batches. If initialization throws, the outer
run()handler only logs the exception: the job staysPENDINGand keeps the destination registration. Other ONCE/PERIODIC jobs targeting that group cannot start. A later successful retry can recover the original job, but repeated initialization failures can block the group indefinitely because the warm-up timeout only applies toRUNNINGjobs.Catch initialization failures before transitioning to
RUNNINGand reusecancel(..., false)to persist the error and release the destination registration. ONCE jobs becomeCANCELLED; PERIODIC jobs remainPENDINGand retry at their existing interval. Initialization has not submitted work to BEs, so this path does not send cleanup RPCs. Successful initialization retains the destination registration as before.Release note
Release the destination compute group when cloud warm-up initialization fails, allowing subsequent warm-up jobs to proceed. Report the initialization error and preserve periodic retry scheduling.
Check List (For Author)
CloudWarmUpJobTest(11),CacheHotspotManagerSchedulerTest(4), andcloud.cache.CacheHotspotManagerTest(15). New tests cover ONCE/PERIODIC initialization failure using the real destination registration map, persisted failure state, periodic retry, and continued mutual exclusion after successful initialization. Before the fix, both failure-injection cases failed at the subsequent-job registration assertion; both successful-initialization cases passed../build.sh --fe -j100passed, including Checkstyle;./run-fe-ut.sh --run 'org.apache.doris.cloud.CloudWarmUpJobTest,org.apache.doris.cloud.CacheHotspotManagerSchedulerTest,org.apache.doris.cloud.cache.CacheHotspotManagerTest'passed;git diff --checkpassed. No live cloud-cluster SQL regression was run.Check List (For Reviewer who merge this PR)