Conversation
### What problem does this PR solve? Issue Number: N/A Related PR: apache#68302 Problem Summary: An initial FOLLOWER or OBSERVER transition can be interrupted by UNKNOWN before metadata post-processing, daemon startup, and metrics initialization run. The replayer can still publish readiness, and the retained INIT/UNKNOWN state handles UNKNOWN without completing initialization. This can release startup services and allow local queries against an incompletely initialized FE. Gate public readiness and read eligibility on completion of the first successful MASTER/FOLLOWER/OBSERVER initialization and FE type commit. Initialization waits use metadata readiness directly to avoid a circular wait. Keep the gate open for an already initialized FE entering UNKNOWN, preserving its metadata-based read policy. Include both readiness conditions in startup wait diagnostics. Exercise real replayer updates at the interruption boundary, successful retries, all non-master initialization steps, initialized UNKNOWN reads and metadata expiry, and ignore_meta_check. Separate listener creation from thread startup so the event interleaving can be tested deterministically. ### Release note Fix FE startup transitions that could expose a follower or observer before initialization completed when an UNKNOWN notification interrupted startup. ### Check List (For Author) - Test: Unit Test - ./run-fe-ut.sh --run org.apache.doris.catalog.EnvStateListenerTest,org.apache.doris.catalog.EnvTest,org.apache.doris.qe.StmtExecutorTest - All 34 tests passed; the 5 new regression cases failed with serving gates disabled. - cd fe && mvn checkstyle:check -pl fe-core - Behavior changed: Yes. Startup services and local reads require completed initialization; initialized UNKNOWN nodes retain the existing read policy. - Does this need documentation: No
morrySnow
requested review from
CalvinKirs,
dataroaring and
morningman
as code owners
September 21, 2026 09:12
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
/review |
Contributor
Author
|
run buildall |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Static review opinion: approve; no blocking or actionable issue found at 9f6a76192aeb2073b7a45172c110989c22c70e83.
Critical checkpoints:
- Goal and correctness: The change correctly separates replayer-owned metadata freshness from one-time serving readiness. Raw
isReady/canReadupdates cannot release startup services or local reads until the state listener finishes the first MASTER/FOLLOWER/OBSERVER initialization, commitsfeType, and publishes the volatile gate. - Scope and clarity: The implementation is focused on
Envreadiness semantics plus deterministic state-listener tests. The listener factory extraction preserves production meta-context and startup behavior. - Concurrency and lifecycle: I traced the BDB callback, transfer queue, state-listener thread, replayer thread, main
waitForReadythread, and readiness-dependent daemons. The volatile gate is written after initialization and role assignment; public readers acquire it before consulting the atomic metadata flags. Interrupted INIT/UNKNOWN to FOLLOWER/OBSERVER transitions remain fail-closed and retry correctly. After successful startup, UNKNOWN retains the existing read policy while replay error and metadata expiry still close readiness/readability. - Parallel paths and compatibility: MASTER, BDB, non-BDB, and Cloud serving startup all reach the same listener publication. Checkpoint/snapshot Env paths are not alternate serving-startup paths. No configuration, persistence, EditLog, transaction, storage-format, FE-BE protocol, or rolling-upgrade compatibility change is introduced.
- Conditions, errors, and performance: The queue-interruption condition has a concrete stale-transition purpose and leaves initialization uncommitted. No new silent error path, lock-order issue, or material performance concern was found; the extra volatile read is limited to readiness checks.
- Tests: The added cases cover INIT/UNKNOWN to FOLLOWER/OBSERVER interrupted by UNKNOWN, real replayer flag updates, retry/completion ordering, sampled initialization callbacks, initialized UNKNOWN behavior, freshness expiry,
ignore_meta_check, and listener cleanup. MASTER/Cloud/non-BDB behavior was verified statically through the shared listener commit. Per the review prompt, I did not run builds or tests; the author's reported 34 FE unit tests, checkstyle/build, regression suites, and manual restart remain author-provided validation rather than independent execution. - Observability: Startup wait logging now distinguishes metadata readiness from initialization publication, which is sufficient for this lifecycle change. No additional metric is needed.
- User focus: No additional user-provided focus points were supplied.
Round 1 converged across the main review, two complementary full-review agents, and a separate lifecycle risk review, all with NO_NEW_VALUABLE_FINDINGS. No inline comments are proposed.
morningman
approved these changes
Sep 21, 2026
Contributor
TPC-H: Total hot run time: 27836 ms |
Contributor
TPC-DS: Total hot run time: 152078 ms |
Contributor
ClickBench: Total hot run time: 24.03 s |
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?
Issue Number: N/A
Related PR: #68302
Problem Summary:
If UNKNOWN interrupts a FE's first FOLLOWER/OBSERVER transition, initialization returns early while the previous FE type is retained. The replayer can subsequently set the metadata readiness/readability flags to true before initialization has completed. This can release
waitForReady()and allow local reads while startup work is still incomplete. Clearing those flags once does not close the race, since the replayer can set them again.Add a process-local
startupInitializedgate toisReady()andcanRead(). The state listener opens the gate only after a successful MASTER/FOLLOWER/OBSERVER initialization and publication of the new FE type. UNKNOWN handling cannot open it. Initialization's own waits continue to use metadata readiness, avoiding a circular dependency.Once initialization has completed, the gate stays open so an initialized FE entering UNKNOWN retains its existing read policy and metadata freshness checks.
Release note
Fix a race that could let an FE report ready or serve local reads before startup initialization completed after an UNKNOWN state interruption.
Check List (For Author)
Validation:
./run-fe-ut.sh --run org.apache.doris.catalog.EnvStateListenerTest,org.apache.doris.catalog.EnvTest,org.apache.doris.qe.StmtExecutorTest: 34 tests passed.ignore_meta_check. With the two serving gates disabled, all five new cases fail; they pass with the fix.cd fe && mvn checkstyle:check -pl fe-coreandgit diff --check: passed../build.sh --fe: passed.test_select_constantandtest_numbersthroughrun-regression-test.sh: both suites passed.SHOW FRONTENDShealth,/metrics, andSELECT COUNT(*), SUM(number) FROM numbers("number"="1000")returning1000, 499500. UNKNOWN interruption interleavings are covered by unit tests; no multi-FE network-failure test was run.Check List (For Reviewer who merge this PR)