Skip to content

[fix](fe) Persist routine load job before scheduling - #67681

Merged
sollhui merged 1 commit into
apache:masterfrom
0AyanamiRei:fix/routine-load-create-replay-reschedule
Sep 28, 2026
Merged

sollhui merged 1 commit into
apache:masterfrom
0AyanamiRei:fix/routine-load-create-replay-reschedule

Conversation

@0AyanamiRei

@0AyanamiRei 0AyanamiRei commented Sep 9, 2026 •

Copy link
Copy Markdown
Contributor

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:

publish job -> scheduler may change it to RUNNING -> serialize create journal

After:

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.

Release note

Routine load creation now persists job metadata before the job becomes visible to the scheduler.

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:
      • Other reason: Added Kafka and Kinesis create-journal ordering coverage, but did not compile or run it locally per review request. Targeted FE Checkstyle passed.
  • Behavior changed:

    • No.
    • Yes. A new routine load job becomes visible to the scheduler only after its create journal is durable.
  • Does this need documentation?

    • No.
    • Yes.

Check List (For Reviewer who merge this PR)

  • Confirm the release note
  • Confirm test cases
  • Confirm document
  • Add branch pick label

@hello-stephen

Copy link
Copy Markdown
Contributor

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See 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?

### 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.
@0AyanamiRei
0AyanamiRei force-pushed the fix/routine-load-create-replay-reschedule branch from 2d989f6 to e76a778 Compare September 9, 2026 01:44
@0AyanamiRei 0AyanamiRei changed the title [fix](fe) Reschedule routine load jobs after metadata replay [fix](fe) Persist routine load job before scheduling Sep 9, 2026
@sollhui

sollhui commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

run buildall

@sollhui

sollhui commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

/review

@sollhui sollhui left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 RUNNING records 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.

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-H: Total hot run time: 16757 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit e76a7785786a70ba24b15ce9b65756afd356b044, data reload: false

------ Round 1 ----------------------------------
============================================
q1	17588	3028	3012	3012
q2	2152	263	229	229
q3	10192	879	503	503
q4	4667	252	201	201
q5	7678	553	387	387
q6	138	113	94	94
q7	529	500	381	381
q8	9232	859	980	859
q9	3474	2415	2394	2394
q10	6498	831	691	691
q11	395	198	181	181
q12	611	260	194	194
q13	18135	1546	1140	1140
q14	168	148	135	135
q15	q16	437	393	366	366
q17	1405	852	802	802
q18	3086	2269	2235	2235
q19	1283	887	782	782
q20	364	274	203	203
q21	5596	1732	1859	1732
q22	321	268	236	236
Total cold run time: 93949 ms
Total hot run time: 16757 ms

----- Round 2, with runtime_filter_mode=off -----
============================================
q1	3369	3278	3259	3259
q2	496	393	391	391
q3	2228	2249	2164	2164
q4	1188	1167	899	899
q5	2164	2118	2088	2088
q6	160	117	84	84
q7	1054	902	843	843
q8	1575	1370	1391	1370
q9	3118	3071	3078	3071
q10	1848	1818	1631	1631
q11	352	262	257	257
q12	458	426	335	335
q13	1471	1542	1144	1144
q14	164	167	155	155
q15	q16	397	396	350	350
q17	3594	3299	3183	3183
q18	4773	4420	4734	4420
q19	846	821	888	821
q20	1023	988	818	818
q21	3842	3110	3295	3110
q22	400	338	321	321
Total cold run time: 34520 ms
Total hot run time: 30714 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-DS: Total hot run time: 81482 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpcds-tools
TPC-DS sf100 test result on commit e76a7785786a70ba24b15ce9b65756afd356b044, data reload: false

query5	4260	410	352	352
query6	382	133	122	122
query7	4946	415	230	230
query8	291	125	132	125
query9	8693	2866	2863	2863
query10	399	226	185	185
query11	5373	1054	922	922
query12	124	71	73	71
query13	1213	459	335	335
query14	6166	2222	2085	2085
query14_1	1965	1957	1949	1949
query15	176	126	117	117
query16	905	377	347	347
query17	779	435	349	349
query18	2322	323	234	234
query19	157	138	112	112
query20	71	69	71	69
query21	198	102	92	92
query22	5398	5320	5535	5320
query23	6773	6408	5949	5949
query23_1	5938	5861	6075	5861
query24	7311	1087	753	753
query24_1	755	790	763	763
query25	421	305	253	253
query26	1224	227	132	132
query27	2780	408	249	249
query28	4687	1508	1508	1508
query29	932	440	359	359
query30	251	156	140	140
query31	823	397	335	335
query32	135	74	80	74
query33	458	220	180	180
query34	979	824	476	476
query35	406	400	343	343
query36	581	593	517	517
query37	123	79	69	69
query38	1001	841	811	811
query39	487	474	482	474
query39_1	494	474	448	448
query40	201	98	80	80
query41	60	57	56	56
query42	75	74	74	74
query43	244	241	208	208
query44	1028	564	581	564
query45	112	111	109	109
query46	747	808	508	508
query47	742	738	706	706
query48	303	291	222	222
query49	531	233	188	188
query50	757	261	202	202
query51	8187	7996	7912	7912
query52	67	70	56	56
query53	188	196	150	150
query54	219	175	170	170
query55	74	66	58	58
query56	181	162	156	156
query57	692	649	720	649
query58	190	169	158	158
query59	1195	1220	1068	1068
query60	225	174	177	174
query61	109	122	109	109
query62	358	216	180	180
query63	179	145	134	134
query64	2764	659	585	585
query65	1610	1619	1606	1606
query66	1923	284	209	209
query67	9782	9737	9797	9737
query68	3015	1251	698	698
query69	330	226	198	198
query70	658	620	623	620
query71	250	180	164	164
query72	2213	1638	1466	1466
query73	636	565	338	338
query74	1988	1223	1116	1116
query75	1170	1100	959	959
query76	2358	746	532	532
query77	253	253	200	200
query78	3931	3795	3235	3235
query79	2755	824	572	572
query80	1594	308	268	268
query81	517	160	133	133
query82	631	124	102	102
query83	283	208	193	193
query84	291	111	89	89
query85	830	345	286	286
query86	472	177	169	169
query87	1026	973	889	889
query88	3021	2115	2093	2093
query89	281	195	174	174
query90	2168	132	135	132
query91	128	112	97	97
query92	94	71	72	71
query93	2494	1042	722	722
query94	623	235	213	213
query95	532	324	228	228
query96	796	543	256	256
query97	1039	1039	1054	1039
query98	177	134	135	134
query99	420	344	313	313
Total cold run time: 179499 ms
Total hot run time: 81482 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
ClickBench: Total hot run time: 14.61 s
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit e76a7785786a70ba24b15ce9b65756afd356b044, data reload: false

query1	0.01	0.00	0.00
query2	0.08	0.04	0.04
query3	0.24	0.11	0.09
query4	1.61	0.10	0.10
query5	0.17	0.15	0.16
query6	1.27	0.67	0.71
query7	0.03	0.01	0.00
query8	0.05	0.03	0.03
query9	0.29	0.21	0.22
query10	0.35	0.35	0.35
query11	0.16	0.11	0.12
query12	0.14	0.12	0.12
query13	0.30	0.31	0.30
query14	0.45	0.44	0.44
query15	0.36	0.34	0.36
query16	0.21	0.24	0.21
query17	0.66	0.72	0.70
query18	0.18	0.16	0.17
query19	1.21	1.11	1.20
query20	0.01	0.01	0.01
query21	15.44	0.15	0.13
query22	5.07	0.04	0.04
query23	16.17	0.25	0.10
query24	2.90	0.32	0.26
query25	0.11	0.04	0.04
query26	0.77	0.16	0.12
query27	0.04	0.03	0.04
query28	3.68	0.55	0.29
query29	12.44	3.22	2.59
query30	0.26	0.12	0.12
query31	2.75	0.37	0.17
query32	3.52	0.31	0.23
query33	1.33	1.38	1.43
query34	15.30	2.17	1.76
query35	1.74	1.72	1.74
query36	0.45	0.29	0.28
query37	0.06	0.04	0.04
query38	0.04	0.02	0.02
query39	0.03	0.02	0.03
query40	0.12	0.08	0.07
query41	0.07	0.02	0.02
query42	0.04	0.02	0.02
query43	0.04	0.03	0.02
Total cold run time: 90.15 s
Total hot run time: 14.61 s

@0AyanamiRei

Copy link
Copy Markdown
Contributor Author

run NonConcurrent

@hello-stephen

Copy link
Copy Markdown
Contributor

FE Regression Coverage Report

Increment line coverage 0.00% (0/3218) 🎉
Increment coverage report
Complete coverage report

@sollhui
sollhui merged commit 4ba8b27 into apache:master Sep 28, 2026
45 checks passed
@github-actions

Copy link
Copy Markdown
Contributor

PR approved by anyone and no changes requested.

@github-actions github-actions Bot added the approved Indicates a PR has been approved by one committer. label Sep 28, 2026
@github-actions

Copy link
Copy Markdown
Contributor

PR approved by at least one committer and no changes requested.

github-actions Bot pushed a commit that referenced this pull request Sep 28, 2026
### 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.
0AyanamiRei added a commit to 0AyanamiRei/doris that referenced this pull request Sep 28, 2026
### 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]>
yiguolei pushed a commit that referenced this pull request Oct 8, 2026
### 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.
yiguolei pushed a commit that referenced this pull request Oct 13, 2026
…- #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
yiguolei pushed a commit that referenced this pull request Oct 15, 2026
…- #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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by one committer. dev/4.1.6-merged reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants