[fix](fe) Redirect forward-to-master statements to the real master after failover - #67310
[fix](fe) Redirect forward-to-master statements to the real master after failover#67310bosswnx wants to merge 1 commit into
Conversation
…ter failover After a master FE failover, a non-master FE with lagging journal replay keeps forwarding statements to the old (degraded but still serving) master, and clients receive 'The statement has been forwarded to master FE(...) and failed to execute because Master FE is not ready' for the whole meta_delay_toleration_second window. FORWARD_WITH_SYNC statements additionally hang in JournalObservable.waitOn() for up to query_timeout * 1.2. - FrontendServiceImpl.forward() now rejects forwarded statements up front with a structured NOT_MASTER result (notMaster + best-effort masterAddress hint) when the receiving FE is not the master. The statement is NOT executed, so the sender can safely retry it. - MasterOpExecutor validates the hint (rejecting hints pointing back to the failed target or to itself), falls back to bdbje leader lookup, then to probing alive followers via a lightweight isMasterProbe request, and retries the statement once against the discovered master. - A NOT_MASTER result skips the journal replay wait, so rejections surface to the client immediately instead of hanging for the journal-wait timeout. - Redirect is scoped to MasterOpExecutor only (supportNotMasterRedirect); generic FEOpExecutor calls (config propagation, cross-FE query kill) keep their exact-target semantics. Transport-failure retry semantics are unchanged: only an explicit pre-execution NOT_MASTER rejection is retried.
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
run buildall |
|
Hi maintainers — one piece of context that may help with the decision: I noticed #67309 is also addressing #67297, with a different approach. I cross-checked both fixes against my reproduction environment and am leaving the conclusions here for reference. Background: the deterministic reproduction in #67297 (4-FE docker + iptables injection) exercises the failure mode where the old master is OOM-recovered and degraded but alive (process up, MySQL/thrift ports still serving, LB health checks still passing), while the observer's stuck journal replay keeps 1. The primary scenario of #67297 never triggers the retry code in #67309 The retry in #67309 lives inside 2. The rediscovery source is the same cache it is trying to fix In the sub-case where a transport exception does occur (old master fully dead), #67309 re-reads 3. Retrying non-idempotent statements on a transport exception risks double execution A transport failure is semantically ambiguous — the statement may already have executed on the target, with only the response lost. The retry in #67309 runs before the existing 4. Minor observations
These conclusions come from my own reproduction environment, offered for the maintainers' reference. The two approaches differ in direction; if the community prefers the other one, I'm happy to adjust or merge efforts accordingly. |
Cloud UT Coverage ReportIncrement line coverage Increment coverage report
|
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
|
run compile |
|
/review |
What problem does this PR solve?
Issue Number: close #67297
Problem Summary:
After a master FE failover (the old master lost leadership, e.g. due to OOM/GC pause, but the process stayed alive and kept serving its MySQL/thrift ports), a non-master FE (observer/follower) with a lagging journal replay keeps forwarding statements to the old master because:
Env.masterInfo, which is only refreshed by replaying theOP_MASTER_INFO_CHANGEjournal (or by loading an image at startup). There is no active master re-discovery on the forward path.StmtExecutorand throws "The statement has been forwarded to master FE(...) and failed to execute because Master FE is not ready" — an error the sender cannot recover from.FORWARD_WITH_SYNCstatements, the old master's error response still carries amaxJournalId, so the sender blocks inJournalObservable.waitOn()(up toquery_timeout * 1.2, default ~18 min) before surfacing anything.The issue above contains a deterministic 4-FE docker reproduction with iptables-based fault injection, which this PR fixes.
Fix:
FrontendServiceImpl.forward()): reject a forwarded statement up front with a structuredNOT_MASTERresult (notMaster+ best-effortmasterAddresshint) when the receiving FE is not the master. The statement is not executed, which makes a sender-side retry safe even for non-idempotent statements. A lightweightisMasterProbeshortcut is added for master discovery.MasterOpExecutor): onNOT_MASTER, validate the hint (rejecting hints that point back to the failed target or to itself — a degraded old master may keepmasterInfo = itself), then fall back to a bdbje leader lookup (getHaProtocol().getLeader(), independent of journal replay), then to probing alive followers viaisMasterProbe, and retry the statement once against the discovered master.NOT_MASTERresult skipsJournalObservable.waitOn()so rejections surface to the client immediately instead of hanging for the journal-wait timeout. Successful results keep the existing wait semantics (read-your-writes unchanged).MasterOpExecutor(supportNotMasterRedirect()); genericFEOpExecutorcalls (all-FE config propagation, cross-FE query kill) that intentionally target a specific non-master FE keep their semantics. Transport-failure retry semantics are unchanged — only an explicit pre-executionNOT_MASTERrejection is retried, never an ambiguous timeout.Compatibility: the new thrift fields are all
optional, so mixed old/new deployments behave as before (an old sender ignores the new fields; a new sender falling back to the old error path is no worse than the status quo).maxJournalIdof the rejection is0, whose journal wait is a no-op even for old senders.Release note
Fix an availability issue where, after a master FE failover, non-master FEs kept forwarding statements to the old degraded master for up to
meta_delay_toleration_second(default 300s), failing with "The statement has been forwarded to master FE(...) and failed to execute because Master FE is not ready", andFORWARD_WITH_SYNCstatements hung in journal-sync wait for up to 18 minutes. Forwarded statements are now rejected up front by a non-master receiver and retried once against the re-discovered master.Check List (For Author)
Test
Behavior changed:
A non-master FE that receives a forwarded statement now rejects it immediately with a structured
NOT_MASTERresult instead of executing it deep intoStmtExecutor;MasterOpExecutorthen retries once against the re-discovered master.FORWARD_WITH_SYNCstatements no longer hang on the journal wait when the forward target is not the master. GenericFEOpExecutorbehavior (config propagation, query kill, transport-failure retry) is unchanged.Does this need documentation?
Check List (For Reviewer who merge this PR)
Unit tests
MasterOpExecutorNotMasterTest(5 cases, all passing):masterInfo = itself)NOT_MASTERresult detectionManual test (deterministic reproduction from the issue)
4-FE docker cluster (fe1=initial master, fe2/fe3=followers, fe4=observer), fault injection via host-side
nsenter+iptables:masterInfostays = fe1docker killfe1 (old master "OOM" death); fe3 elected new masterBefore this PR (official
apache/doris:fe-3.0.8image), same injection:After this PR (patched build, same injection):
Observer log shows the redirect working as designed: