[fix](fetype) Commit FE type only after transition completes

[morrySnow]

What problem does this PR solve?

Issue Number: N/A

Related PR: N/A

Problem Summary:

A non-master FE transition can be interrupted when a newer BDB state is queued while the listener waits for metadata readiness. The listener previously still committed the target FE type even though non-master initialization, including MetricRepo.init(), had not run. A repeated FOLLOWER or OBSERVER notification was then treated as redundant, leaving the FE in an incompletely initialized state.

This change makes transferToNonMaster() report whether the transition completed and updates feType only after all initialization finishes. An interrupted transition retains the previous committed state so the queued event is evaluated against the state that was actually initialized and can retry the transition.

Release note

Fix FE state transitions that could leave a follower or observer serving queries before non-master initialization completed.

Check List (For Author)

• Test: Unit Test
  • ./run-fe-ut.sh --run org.apache.doris.catalog.EnvStateListenerTest
  • cd fe and run mvn checkstyle:check -pl fe-core
• Behavior changed: Yes. Interrupted non-master transitions no longer publish an incomplete FE type.
• Does this need documentation: No

[hello-stephen]

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?

[morrySnow]

run buildall

[morrySnow]

/review

[github-actions]

Static review of PR 68302 at commit 6994426 found one blocking FE lifecycle issue; the inline finding below requests a readiness gate for the retained-state UNKNOWN path.

Review checkpoints:

• Goal and correctness: The repeated FOLLOWER/OBSERVER retry case is addressed, but an UNKNOWN notification can still expose an incompletely initialized non-master FE.
• Lifecycle and concurrency: The listener is synchronized and the queue/replayer paths were traced. The remaining defect is a real race between the replayer publishing readiness and the retained-state UNKNOWN no-op/equality handling.
• Locking, exceptions, persistence, compatibility, and configuration: No additional issue found; this change does not alter a protocol or persisted format.
• Parallel paths and observability: BDBStateChangeListener, startup waitForReady(), canRead-based query forwarding, and the UNKNOWN transition path were checked.
• Test coverage: The new unit test deterministically covers a repeated FOLLOWER event, but it does not cover INIT/UNKNOWN -> FOLLOWER/OBSERVER interrupted by UNKNOWN or a later retry.
• Validation status: Static-only review as required by the review prompt; no builds or tests were run.
• User focus: No additional focus points were provided.

Please address the inline issue and add the UNKNOWN interruption regression coverage before merging.

[hello-stephen]

TPC-H: Total hot run time: 27861 ms

machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit 6994426099e6301acdce42563ff91c95c1190ef8, data reload: false

------ Round 1 ----------------------------------
============================================
q1	17726	3833	3815	3815
q2	2224	392	315	315
q3	10020	1465	810	810
q4	4680	478	349	349
q5	7481	875	551	551
q6	177	167	134	134
q7	732	786	615	615
q8	9298	1507	1475	1475
q9	5510	4222	4178	4178
q10	6823	1335	992	992
q11	427	275	244	244
q12	630	429	298	298
q13	18120	2600	1994	1994
q14	268	259	235	235
q15	q16	739	719	648	648
q17	1812	1068	998	998
q18	6554	5604	5588	5588
q19	1329	1276	999	999
q20	479	380	268	268
q21	5759	3542	3043	3043
q22	432	366	312	312
Total cold run time: 101220 ms
Total hot run time: 27861 ms

----- Round 2, with runtime_filter_mode=off -----
============================================
q1	4591	4738	4430	4430
q2	721	579	549	549
q3	4728	5135	4527	4527
q4	2243	2363	1489	1489
q5	4530	4336	4621	4336
q6	242	182	125	125
q7	1827	1713	1511	1511
q8	2383	2020	2016	2016
q9	7253	7158	7196	7158
q10	3715	3606	3073	3073
q11	516	375	367	367
q12	702	704	506	506
q13	2276	2577	2013	2013
q14	277	286	243	243
q15	q16	656	688	599	599
q17	7278	6712	6676	6676
q18	11905	11069	11886	11069
q19	1083	1008	988	988
q20	2214	2196	1925	1925
q21	5031	4129	4274	4129
q22	499	444	413	413
Total cold run time: 64670 ms
Total hot run time: 58142 ms

[hello-stephen]

TPC-DS: Total hot run time: 152265 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 6994426099e6301acdce42563ff91c95c1190ef8, data reload: false

query5	4298	596	471	471
query6	434	211	202	202
query7	4831	546	291	291
query8	320	181	173	173
query9	8843	3964	3931	3931
query10	450	306	261	261
query11	5855	3548	3263	3263
query12	147	92	88	88
query13	1262	571	420	420
query14	6576	4538	4215	4215
query14_1	3966	3952	3955	3952
query15	202	194	183	183
query16	974	463	404	404
query17	907	679	548	548
query18	2442	465	334	334
query19	201	182	145	145
query20	84	84	80	80
query21	223	134	115	115
query22	12932	13007	12898	12898
query23	14023	12963	12468	12468
query23_1	12371	12401	12482	12401
query24	7276	1084	670	670
query24_1	687	654	753	654
query25	565	437	375	375
query26	1274	304	163	163
query27	2676	541	343	343
query28	4572	1934	1922	1922
query29	1660	739	528	528
query30	296	223	178	178
query31	903	764	642	642
query32	151	102	95	95
query33	527	316	259	259
query34	1195	1081	647	647
query35	726	763	638	638
query36	791	826	734	734
query37	154	112	95	95
query38	1836	1771	1670	1670
query39	701	659	653	653
query39_1	641	636	654	636
query40	224	130	124	124
query41	82	69	68	68
query42	99	98	98	98
query43	337	342	306	306
query44	1355	711	707	707
query45	189	184	172	172
query46	1069	1177	788	788
query47	1503	1465	1393	1393
query48	400	407	275	275
query49	588	401	289	289
query50	1005	339	254	254
query51	10526	10634	10549	10549
query52	88	89	72	72
query53	246	253	177	177
query54	243	207	195	195
query55	82	72	66	66
query56	218	207	205	205
query57	1402	1408	1348	1348
query58	285	249	250	249
query59	1973	2067	1866	1866
query60	275	240	218	218
query61	142	149	150	149
query62	396	314	272	272
query63	218	176	178	176
query64	2770	985	795	795
query65	3453	3408	3402	3402
query66	1808	414	308	308
query67	20507	19977	19829	19829
query68	3147	1535	919	919
query69	421	300	261	261
query70	898	823	795	795
query71	284	232	208	208
query72	2698	2540	2240	2240
query73	836	747	423	423
query74	4655	4527	4265	4265
query75	2325	2265	1923	1923
query76	2295	1093	753	753
query77	356	396	293	293
query78	9083	8965	8461	8461
query79	1420	1151	671	671
query80	1250	449	363	363
query81	618	326	278	278
query82	754	169	125	125
query83	311	218	190	190
query84	317	147	113	113
query85	875	470	377	377
query86	420	240	230	230
query87	1950	1965	1831	1831
query88	3625	2702	2713	2702
query89	360	287	245	245
query90	1916	182	175	175
query91	174	158	124	124
query92	101	88	89	88
query93	1563	1377	812	812
query94	716	342	292	292
query95	674	359	329	329
query96	1105	792	334	334
query97	2452	2422	2328	2328
query98	165	147	143	143
query99	722	726	634	634
Total cold run time: 237644 ms
Total hot run time: 152265 ms

[hello-stephen]

ClickBench: Total hot run time: 24.05 s

machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit 6994426099e6301acdce42563ff91c95c1190ef8, data reload: false

query1	0.01	0.00	0.00
query2	0.09	0.05	0.05
query3	0.26	0.14	0.13
query4	1.61	0.14	0.14
query5	0.24	0.21	0.22
query6	1.16	0.93	0.96
query7	0.04	0.01	0.01
query8	0.06	0.03	0.04
query9	0.39	0.36	0.34
query10	0.56	0.55	0.60
query11	0.19	0.14	0.14
query12	0.18	0.14	0.15
query13	0.46	0.48	0.48
query14	0.95	0.94	0.93
query15	0.60	0.61	0.60
query16	0.30	0.33	0.31
query17	1.10	1.08	1.10
query18	0.22	0.20	0.20
query19	2.03	1.92	1.94
query20	0.02	0.01	0.01
query21	15.44	0.23	0.13
query22	4.75	0.04	0.04
query23	16.12	0.30	0.13
query24	2.99	0.40	0.32
query25	0.11	0.05	0.05
query26	0.75	0.19	0.15
query27	0.04	0.04	0.03
query28	3.53	0.80	0.37
query29	12.48	4.15	3.26
query30	0.28	0.15	0.15
query31	2.77	0.56	0.31
query32	3.23	0.58	0.48
query33	3.16	3.18	3.28
query34	15.47	3.98	3.29
query35	3.22	3.18	3.26
query36	0.56	0.44	0.41
query37	0.08	0.07	0.07
query38	0.05	0.04	0.04
query39	0.04	0.03	0.03
query40	0.17	0.15	0.14
query41	0.08	0.04	0.04
query42	0.04	0.03	0.02
query43	0.04	0.03	0.04
Total cold run time: 95.87 s
Total hot run time: 24.05 s

[hello-stephen]

FE UT Coverage Report

Increment line coverage 50.00% (6/12) 🎉
Increment coverage report
Complete coverage report

[morningman]

Context guide for reviewers: what this PR fixes, in plain terms

TL;DR: this fixes a race in the FE role state machine. When a FOLLOWER/OBSERVER transition is interrupted, the old code still marked the FE as "already FOLLOWER", so the real initialization never got a second chance and the FE stayed half-initialized until a restart.

Background: how an FE changes its role

• The role is decided by the BDBJE replication group. On every BDBJE state change, BDBStateChangeListener pushes a new role (FOLLOWER / OBSERVER / UNKNOWN / MASTER) into typeTransferQueue.
• The stateListener daemon takes roles from the queue, runs the transition (transferToMaster() / transferToNonMaster()), and then sets feType = newType. feType is what the FE believes it currently is.
• If the incoming role equals the current feType, the listener treats it as a duplicate notification and ignores it.

What transferToNonMaster() does (INIT/UNKNOWN -> FOLLOWER/OBSERVER)

1. Start the replayer thread that replays edit logs from master.
2. Block in postProcessAfterMetadataReplayed(true) until metadata has caught up, i.e. until isReady becomes true (it is set by the replayer thread).
3. Only then run the actual non-master initialization: checkLowerCaseTableNames(), startNonMasterDaemonThreads() (load manager, label cleaner, tablet stat mgr, DNS cache, admission control, ...), MetricRepo.init(), statistics cache pre-heat, FollowerColumnSender.

Step 2 has an escape hatch: if another role notification is already waiting in the queue, it returns false immediately ("the state changed again, don't wait here forever, go handle the newer state"). That design is fine by itself.

The bug

When the escape hatch fired, transferToNonMaster() just returned and none of step 3 ran. But the caller (runOneCycle) had no way to know that and unconditionally did feType = newType. The FE now claims to be a FOLLOWER while none of the FOLLOWER initialization has happened.

Next the listener processes the queued notification that caused the interruption. If it is another FOLLOWER (BDBJE can deliver the same state more than once), the listener sees feType == newType and drops it as a duplicate. From then on every further FOLLOWER notification is ignored as well, so step 3 never runs.

Why it matters

isReady / canRead (the "can I serve" flags) are controlled independently by the replayer thread: as soon as metadata is caught up they flip to true, regardless of whether step 3 ran. So the FE accepts queries and looks healthy from the outside, but its metrics are empty, the non-master daemon threads are not running, and the statistics cache is never pre-heated. This state is hard to notice and does not heal itself.

The fix

• transferToNonMaster() now returns a boolean: true only when step 3 completed, false when interrupted.
• The listener commits feType only on true. On false it keeps the previous feType (e.g. INIT) and continues with the next queued notification.
• When the next FOLLOWER notification arrives, it is a legitimate INIT -> FOLLOWER transition and the whole initialization runs again. In other words, feType now means "the last state that was fully initialized", not "the latest state reported by BDB".

The test

EnvStateListenerTest mocks the wait in step 2 to always return false (simulating an interruption). It sends FOLLOWER once and checks that the transition was attempted, then sends FOLLOWER again and asserts the transition is attempted a second time instead of being dropped as a duplicate, with feType still INIT. Before this fix the second notification would have been ignored.

dismiss