bosswnx opened a new pull request, #67310:
URL: https://github.com/apache/doris/pull/67310
### 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 <!-- At least one of them must be included. -->
- [ ] Regression test
- [x] Unit Test
- [x] 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 <!-- Add your reason? -->
- Behavior changed:
- [ ] No.
- [x] Yes. <!-- Explain the behavior change -->
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?
- [x] No.
- [ ] Yes. <!-- Add document PR link here. eg:
https://github.com/apache/doris-website/pull/1214 -->
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR should
merge into -->
#### 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
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
