Repository navigation
[fix](fe) Persist routine load job before scheduling - #67681
Conversation
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
### What problem does this PR solve? Issue Number: None Related PR: None Problem Summary: Routine load creation published a mutable job to the scheduler before serializing its create journal. The scheduler could change the job from NEED_SCHEDULE to RUNNING before serialization, while runtime task membership was not persisted. Replaying that record restored a RUNNING job without tasks. Persist the initial NEED_SCHEDULE job before publishing it so the create journal always contains a stable, reconstructable state. ### Release note Routine load creation now persists job metadata before the job becomes schedulable. ### Check List (For Author) - Test: No unit test executed per local review request; targeted FE Checkstyle passed. Added Kafka and Kinesis create-journal ordering coverage. - Behavior changed: Yes. A new routine load job becomes visible to the scheduler only after its create journal is durable. - Does this need documentation: No.
2d989f6 to
e76a778
Compare
|
run buildall |
|
/review |
sollhui
left a comment
There was a problem hiding this comment.
LGTM. No actionable issues found in e76a778.
- Goal and scope: the minimal ordering change prevents the scheduler from mutating the new job before its create record is serialized.
- Concurrency and locking: checked the creation thread, scheduler, and batch journal writer. Both direct and queued EditLog paths wait for the journal write to complete. The existing manager lock scope is preserved, with no additional journal writes or new lock nesting.
- Persistence, failure, and lifecycle: the job is not published if logging fails. A failure after persistence but before publication is recoverable through the existing create-record replay, which also registers the transaction callback. No new ownership or cleanup path is introduced.
- Parallel paths and compatibility: Kafka and Kinesis share this creation path. No serialization format, replay-state normalization, configuration, or FE/BE protocol changes are introduced; historical-record recovery remains outside this fix.
- Tests: the parameterized test deterministically exercises the old scheduling window, checks the serialized initial state and absence of premature publication/callback registration, and verifies task creation after publication. This is targeted ordering coverage, not an end-to-end failover or journal-failure test; no regression output files are changed.
- Performance and observability: no additional scanning, allocation, or I/O is added to production code; existing logging remains sufficient for this focused change. Partition visibility, delete bitmaps, and BE memory accounting are unaffected.
Validation: source review only; I did not compile or run tests locally. FE UT and COMPILE CI are still pending at review time.
There was a problem hiding this comment.
Request changes: one concurrency/lifecycle issue remains at the new publication boundary.
Critical checkpoint conclusions:
- Goal and scope: journaling before scheduler visibility correctly prevents the original mutable-create snapshot race, and the production edit is otherwise minimal.
- Concurrency and lifecycle: the accepted inline finding shows that scheduler visibility still begins before transaction callback registration; the reordered journal wait makes an immediately cancellable job reachable at that partial-publication point. No other lock-order, deadlock, or static-lifecycle issue was found.
- Persistence and failover: direct and batched EditLog paths finish serialization/durability before returning; create replay and image reconstruction cover the durable-before-memory crash interval. No format or FE/BE protocol change is introduced. Historical malformed
RUNNINGrecords remain a real but explicitly scoped, non-regressing follow-up; safe recovery must also cover image load and cloud progress retry semantics. - Parallel paths: Kafka, Kinesis, multi-table, and cloud creation delegate through this manager path; no distinct parallel-path defect survived review.
- Tests: the new parameterized test is a deterministic oracle for the original base ordering for Kafka and Kinesis, but it does not cover the accepted post-journal/pre-callback interval; add a latch-based case with the fix. CheckStyle is green; FE UT and COMPILE CI are still pending. Per the review-runner instruction, no local build or test was run.
- Configuration, observability, performance, and data correctness: no configuration, metrics, data-write, storage-version, delete-bitmap, memory-accounting, or hot-path regression is introduced. Existing logging is adequate for this focused path.
- User focus: no additional focus was provided; the full PR was reviewed.
TPC-H: Total hot run time: 16757 ms |
TPC-DS: Total hot run time: 81482 ms |
ClickBench: Total hot run time: 14.61 s |
|
run NonConcurrent |
FE Regression Coverage ReportIncrement line coverage |
|
PR approved by anyone and no changes requested. |
|
PR approved by at least one committer and no changes requested. |
### What problem does this PR solve? Issue Number: None Related PR: None Problem Summary: Routine load creation published a mutable job to the scheduler before serializing its create journal. The scheduler could change the job from `NEED_SCHEDULE` to `RUNNING` before serialization, while runtime task membership was not persisted. Replaying such a create record restored a `RUNNING` job with no tasks. With stable source topology, the job could remain stuck without consuming data. This PR makes the create journal durable before publishing the job to the scheduler. The scheduler cannot observe or mutate the job while its initial `NEED_SCHEDULE` state is being serialized. Before: ```text publish job -> scheduler may change it to RUNNING -> serialize create journal ``` After: ```text serialize create journal in NEED_SCHEDULE -> publish job -> scheduler creates tasks ``` The change does not modify `RoutineLoadJob.read()` or normalize states during replay. There are no deployed historical Kinesis create records that require compatibility recovery, and existing Kafka replay behavior remains unchanged. #### Separate cloud Kafka follow-up found during review While evaluating a broader replay-time `RUNNING` to `NEED_SCHEDULE` conversion, we found an independent ordering issue in cloud Kafka automatic-partition recovery. `isKafkaPartitionsChanged()` can update `currentKafkaPartitions` before `updateCloudProgress()` successfully fetches the authoritative offset from Meta Service. If that fetch fails, a later scheduler round can see stable topology and skip the Meta Service retry. A recovery path that makes the job schedulable at that point could create a task from stale FE metadata, repeat already committed data, and regress stored routine-load progress. This PR does not add that replay-time conversion and therefore does not change or expose the cloud Kafka path. The cloud issue will be handled separately by making authoritative Meta Service progress synchronization a prerequisite for task creation and preserving the retry requirement across scheduler rounds.
### What problem does this PR solve? Issue Number: None Related PR: apache#66634 Problem Summary: Merge the latest master into the routine load definition persistence branch. Master added RoutineLoadJobPersistenceTest in apache#67681 with a class-level static Env mock created in @beforeeach, while this PR added a test class with the same name whose tests each open their own static Env mock. Two static mocks of one class can not coexist on a thread, so the add/add conflict can not be resolved by concatenating the two classes. Keep master's fixture and test unchanged, and let the PR tests stub the catalog on the shared Env mock instead of opening a second one. Also stub Env.getIndexPolicyMgr(), because master (apache#67918) now resolves the analyzer name of MATCH ... USING ANALYZER through the index policy manager during load property analysis. ### Release note None ### Check List (For Author) - Test: Unit Test - ./run-fe-ut.sh --run RoutineLoadJobPersistenceTest, KafkaRoutineLoadJobTest, KinesisRoutineLoadJobTest, AlterRoutineLoadOperationLogTest, RoutineLoadManagerTest: 59 tests, 0 failures, 0 errors - fe-core checkstyle: 0 violations - Behavior changed: No - Does this need documentation: No Co-Authored-By: Claude <[email protected]>
### What problem does this PR solve? Issue Number: None Related PR: None Problem Summary: Routine load creation published a mutable job to the scheduler before serializing its create journal. The scheduler could change the job from `NEED_SCHEDULE` to `RUNNING` before serialization, while runtime task membership was not persisted. Replaying such a create record restored a `RUNNING` job with no tasks. With stable source topology, the job could remain stuck without consuming data. This PR makes the create journal durable before publishing the job to the scheduler. The scheduler cannot observe or mutate the job while its initial `NEED_SCHEDULE` state is being serialized. Before: ```text publish job -> scheduler may change it to RUNNING -> serialize create journal ``` After: ```text serialize create journal in NEED_SCHEDULE -> publish job -> scheduler creates tasks ``` The change does not modify `RoutineLoadJob.read()` or normalize states during replay. There are no deployed historical Kinesis create records that require compatibility recovery, and existing Kafka replay behavior remains unchanged. #### Separate cloud Kafka follow-up found during review While evaluating a broader replay-time `RUNNING` to `NEED_SCHEDULE` conversion, we found an independent ordering issue in cloud Kafka automatic-partition recovery. `isKafkaPartitionsChanged()` can update `currentKafkaPartitions` before `updateCloudProgress()` successfully fetches the authoritative offset from Meta Service. If that fetch fails, a later scheduler round can see stable topology and skip the Meta Service retry. A recovery path that makes the job schedulable at that point could create a task from stale FE metadata, repeat already committed data, and regress stored routine-load progress. This PR does not add that replay-time conversion and therefore does not change or expose the cloud Kafka path. The cloud issue will be handled separately by making authoritative Meta Service progress synchronization a prerequisite for task creation and preserving the retry requirement across scheduler rounds.
…- #68560 (#68773) ### What problem does this PR solve? Issue Number: None Related PR: #67681, #68560 Problem Summary: Manual backport of #67681 to branch-4.1, replacing the closed automatic backport #68560. Persist the routine load create journal before publishing the job to the scheduler. This keeps the initial NEED_SCHEDULE state stable during serialization and prevents replay from restoring a RUNNING job without tasks. Adapt RoutineLoadJobPersistenceTest to branch-4.1: use the Kafka classes in their existing routineload package and remove unsupported Kinesis references. The Kafka test retains assertions for journal-before-publication ordering and successful scheduling after publication. This addresses the test compilation failure in TeamCity build 1062738. ### Release note Fix routine load jobs potentially becoming stuck after replay due to scheduling before the create journal is persisted. ### Check List (For Author) - Test - [ ] Regression test - [x] Unit Test - [ ] Manual test - Unit test adapted for branch-4.1. Local compilation and tests were not run, as requested. `git diff --check` passed. - Behavior changed: - [ ] No. - [x] Yes. The create journal is persisted before the job becomes visible to the scheduler. - Does this need documentation? - [x] No. - [ ] Yes. ### Check List (For Reviewer who merge this PR) - [ ] Confirm the release note - [ ] Confirm test cases - [ ] Confirm document - [ ] Add branch pick label
…- #68560 (#68773) ### What problem does this PR solve? Issue Number: None Related PR: #67681, #68560 Problem Summary: Manual backport of #67681 to branch-4.1, replacing the closed automatic backport #68560. Persist the routine load create journal before publishing the job to the scheduler. This keeps the initial NEED_SCHEDULE state stable during serialization and prevents replay from restoring a RUNNING job without tasks. Adapt RoutineLoadJobPersistenceTest to branch-4.1: use the Kafka classes in their existing routineload package and remove unsupported Kinesis references. The Kafka test retains assertions for journal-before-publication ordering and successful scheduling after publication. This addresses the test compilation failure in TeamCity build 1062738. ### Release note Fix routine load jobs potentially becoming stuck after replay due to scheduling before the create journal is persisted. ### Check List (For Author) - Test - [ ] Regression test - [x] Unit Test - [ ] Manual test - Unit test adapted for branch-4.1. Local compilation and tests were not run, as requested. `git diff --check` passed. - Behavior changed: - [ ] No. - [x] Yes. The create journal is persisted before the job becomes visible to the scheduler. - Does this need documentation? - [x] No. - [ ] Yes. ### Check List (For Reviewer who merge this PR) - [ ] Confirm the release note - [ ] Confirm test cases - [ ] Confirm document - [ ] Add branch pick label
What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary:
Routine load creation published a mutable job to the scheduler before serializing its create journal. The scheduler could change the job from
NEED_SCHEDULEtoRUNNINGbefore serialization, while runtime task membership was not persisted. Replaying such a create record restored aRUNNINGjob with no tasks. With stable source topology, the job could remain stuck without consuming data.This PR makes the create journal durable before publishing the job to the scheduler. The scheduler cannot observe or mutate the job while its initial
NEED_SCHEDULEstate is being serialized.Before:
After:
The change does not modify
RoutineLoadJob.read()or normalize states during replay. There are no deployed historical Kinesis create records that require compatibility recovery, and existing Kafka replay behavior remains unchanged.Separate cloud Kafka follow-up found during review
While evaluating a broader replay-time
RUNNINGtoNEED_SCHEDULEconversion, we found an independent ordering issue in cloud Kafka automatic-partition recovery.isKafkaPartitionsChanged()can updatecurrentKafkaPartitionsbeforeupdateCloudProgress()successfully fetches the authoritative offset from Meta Service. If that fetch fails, a later scheduler round can see stable topology and skip the Meta Service retry. A recovery path that makes the job schedulable at that point could create a task from stale FE metadata, repeat already committed data, and regress stored routine-load progress.This PR does not add that replay-time conversion and therefore does not change or expose the cloud Kafka path. The cloud issue will be handled separately by making authoritative Meta Service progress synchronization a prerequisite for task creation and preserving the retry requirement across scheduler rounds.
Release note
Routine load creation now persists job metadata before the job becomes visible to the scheduler.
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)