Skip to content

[fix](fe) Redirect forward-to-master statements to the real master after failover - #67310

Open
bosswnx wants to merge 1 commit into
apache:masterfrom
bosswnx:fix/forward-to-stale-master-67297
Open

[fix](fe) Redirect forward-to-master statements to the real master after failover#67310
bosswnx wants to merge 1 commit into
apache:masterfrom
bosswnx:fix/forward-to-stale-master-67297

Conversation

@bosswnx

@bosswnx bosswnx commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

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:

  1. The forward target comes from Env.masterInfo, which is only refreshed by replaying the OP_MASTER_INFO_CHANGE journal (or by loading an image at startup). There is no active master re-discovery on the forward path.
  2. The forwarded thrift call succeeds at the transport level on the degraded old master, which runs the statement deep into StmtExecutor and 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.
  3. For FORWARD_WITH_SYNC statements, the old master's error response still carries a maxJournalId, so the sender blocks in JournalObservable.waitOn() (up to query_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:

  • Receiver side (FrontendServiceImpl.forward()): reject a forwarded statement 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, which makes a sender-side retry safe even for non-idempotent statements. A lightweight isMasterProbe shortcut is added for master discovery.
  • Sender side (MasterOpExecutor): on NOT_MASTER, validate the hint (rejecting hints that point back to the failed target or to itself — a degraded old master may keep masterInfo = itself), then fall back to a bdbje leader lookup (getHaProtocol().getLeader(), independent of journal replay), then to probing alive followers via isMasterProbe, and retry the statement once against the discovered master.
  • Journal wait: a NOT_MASTER result skips JournalObservable.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).
  • Scoping: redirect is enabled only for MasterOpExecutor (supportNotMasterRedirect()); generic FEOpExecutor calls (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-execution NOT_MASTER rejection 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). maxJournalId of the rejection is 0, 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", and FORWARD_WITH_SYNC statements 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

    • Regression test
    • Unit Test
    • Manual test (add detailed scripts or steps below)
      • This is a refactor/code format and no logic has been changed.
      • Previous test can cover this change.
      • No code files have been changed.
      • Other reason
  • Behavior changed:

    • No.
    • Yes.
      A non-master FE that receives a forwarded statement now rejects it immediately with a structured NOT_MASTER result instead of executing it deep into StmtExecutor; MasterOpExecutor then retries once against the re-discovered master. FORWARD_WITH_SYNC statements no longer hang on the journal wait when the forward target is not the master. Generic FEOpExecutor behavior (config propagation, query kill, transport-failure retry) is unchanged.
  • 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

Unit tests

MasterOpExecutorNotMasterTest (5 cases, all passing):

  • hint pointing to the failed target is rejected (prevents retry loop against a degraded old master whose masterInfo = itself)
  • hint pointing to this node is rejected
  • a valid hint pointing elsewhere is accepted
  • empty/invalid hints are rejected
  • NOT_MASTER result detection

Manual 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:

  1. freeze the observer's journal replay toward fe2/fe3 (block bdbje 9010) — its masterInfo stays = fe1
  2. docker kill fe1 (old master "OOM" death); fe3 elected new master
  3. restart fe1 and isolate its bdbje — alive-but-degraded old master (thrift 9020 still serving)
  4. execute statements via the observer (port 19033)

Before this PR (official apache/doris:fe-3.0.8 image), same injection:

$ mysql -h 127.0.0.1 -P 19033 -u root -e 'ADMIN SET FRONTEND CONFIG ("label_keep_max_second" = "259200")'
ERROR 1105 (HY000) at line 1: errCode = 2, detailMessage = The statement has been forwarded to master FE(172.20.80.2) and failed to execute because Master FE is not ready. You may need to check FE's status

$ mysql -h 127.0.0.1 -P 19033 -u root -e 'CREATE USER x IDENTIFIED BY 'p''
(hangs in JournalObservable.waitOn() for up to 18 min)

After this PR (patched build, same injection):

$ mysql -h 127.0.0.1 -P 19033 -u root -e 'ADMIN SET FRONTEND CONFIG ("label_keep_max_second" = "259200")'   -> OK (x3)
$ mysql -h 127.0.0.1 -P 19033 -u root -e 'CREATE USER verify_1 IDENTIFIED BY 'p''                            -> OK

Observer log shows the redirect working as designed:

[fe4] forward to master FE TNetworkAddress(hostname:172.20.80.2, port:9020)
[fe4] forward target 172.20.80.2:9020 is not master any more, retry against the new master TNetworkAddress(hostname:172.20.80.4, port:9020)
[fe3] finished to create user: 'verify_1'@'%'   <- statement actually executed on the real master
[fe2] replay add user verify_1                   <- journal sync to other nodes unaffected

…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.
@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?

@bosswnx

bosswnx commented Aug 29, 2026

Copy link
Copy Markdown
Contributor Author

run buildall

@bosswnx

bosswnx commented Aug 29, 2026

Copy link
Copy Markdown
Contributor Author

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 masterInfo pointing at that old master.

1. The primary scenario of #67297 never triggers the retry code in #67309

The retry in #67309 lives inside catch (TTransportException). But under the core failure mode above, forwarding to the degraded-but-alive old master succeeds at the transport level and returns the business error "The statement has been forwarded to master FE(...) and failed to execute because Master FE is not ready" — no TTransportException is thrown, so the rediscovery code never runs. This is the path my reproduction actually observes.

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 ctx.getEnv().getMasterHost()/getMasterRpcPort(). But masterInfo itself is only refreshed by journal replay of OP_MASTER_INFO_CHANGE — which is exactly the thing that is stuck in this bug. While replay is stuck, the re-read returns the same stale address, newAddr.equals(feAddr) holds, and no retry happens. To recover independently of journal replay, the discovery needs a different source (e.g. Env.getHaProtocol().getLeader() asking bdbje directly, or probing known FE thrift endpoints).

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 shouldNotRetry check, so DDL/DML can execute twice. Constraint 4 in the triage analysis under #67297 draws exactly this distinction. The retry in this PR only applies to an explicit NOT_MASTER rejection returned by the receiver before it executes the statement (safe to retry even for non-idempotent statements); ambiguous transport failures are deliberately not retried.

4. Minor observations

  • The retry is placed in FEOpExecutor, which is also used for calls that intentionally target specific non-master FEs (all-FE config propagation, cross-FE query kill); redirecting those to the master changes their semantics (constraint 3 in the triage analysis).
  • [BugFix] Fix Observer forwarding to stale master FE (#67297) #67309 also removes FEOpExecutor.getAuditStatisticsBackendIds(), which backs the audit-statistics feature recently added upstream — it looks unintentional and would regress that feature.
  • The FORWARD_WITH_SYNC hang (CREATE USER blocking in JournalObservable.waitOn() on the stale maxJournalId returned by the old master, for up to query_timeout * 1.2 ≈ 18 min) is not addressed; in this PR a NOT_MASTER result skips the journal wait.

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.

@hello-stephen

Copy link
Copy Markdown
Contributor

Cloud UT Coverage Report

Increment line coverage 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 78.06% (2053/2630)
Line Coverage 65.78% (37499/57006)
Region Coverage 52.94% (34939/65998)
Branch Coverage 56.42% (11234/19910)

@hello-stephen

Copy link
Copy Markdown
Contributor

BE UT Coverage Report

Increment line coverage 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 62.73% (29344/46781)
Line Coverage 47.70% (306935/643444)
Region Coverage 43.33% (247891/572144)
Branch Coverage 44.89% (115375/257039)

@bosswnx

bosswnx commented Aug 29, 2026

Copy link
Copy Markdown
Contributor Author

run compile

@bosswnx

bosswnx commented Aug 29, 2026

Copy link
Copy Markdown
Contributor Author

/review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Observer/follower keeps forwarding statements to old (degraded) master after master failover, clients receive errors for minutes

2 participants