Panic mode routes traffic to hosts that were excluded from the panic calculation (EDS `DRAINING`)
- Dominant language
- C++
- Stars
- 28.9k
- Forks
- 5.6k
- Avg merge
- 1d 22h
- Merged PRs (30d)
- 430
Description
## Title
Panic mode routes traffic to hosts that were excluded from the panic calculation (EDS `DRAINING`)
---
## Body
### Description
Envoy subtracts "excluded" hosts from the panic *threshold* calculation, but once panic engages it
selects from `host_set.hosts()` — the unfiltered membership — so those same excluded hosts do
receive traffic. For `EDS_STATUS_DRAINING` this means an endpoint the control plane explicitly
marked as draining receives **new** requests.
The two decisions contradict each other: if a host is not capacity for the purpose of deciding
whether we are in panic, it should not become capacity once we are in panic.
**Code path** (paths/lines from `main` at time of writing):
1. `LoadBalancerBase::isHostSetInPanic` — excluded hosts are removed from the denominator:
```cpp
const auto host_count = host_set.hosts().size() - host_set.excludedHosts().size();
```
`source/extensions/load_balancing_policies/common/load_balancer_impl.cc`
2. `ZoneAwareLoadBalancerBase::hostSourceToUse` — on panic, selects `AllHosts`:
```cpp
if (per_priority_panic_[hosts_source.priority_]) {
stats_.lb_healthy_panic_.inc();
if (fail_traffic_on_panic_) { return std::nullopt; }
else { hosts_source.source_type_ = HostsSource::SourceType::AllHosts; return hosts_source; }
}
```
3. `ZoneAwareLoadBalancerBase::hostSourceToHosts` — `AllHosts` is the raw membership, **not**
filtered by `excludedHosts()`:
```cpp
case HostsSource::SourceType::AllHosts:
return host_set.hosts();
```
4. `excludeBasedOnHealthFlag` includes draining (`source/common/upstream/upstream_impl.cc`):
```cpp
return host.healthFlagGet(Host::HealthFlag::PENDING_ACTIVE_HC) ||
host.healthFlagGet(Host::HealthFlag::EXCLUDED_VIA_IMMEDIATE_HC_FAIL) ||
host.healthFlagGet(Host::HealthFlag::EDS_STATUS_DRAINING);
```
`ClusterImplBase::partitionHostList` builds `healthy`/`degraded`/`excluded` as derived views while
`hosts()` keeps everything, so nothing downstream of `AllHosts` re-applies the exclusion.
Envoy's own documentation of the exclusion concept says these hosts are **not meant to be routed
to** (comment on `EXCLUDED_VIA_IMMEDIATE_HC_FAIL`, `envoy/upstream/upstream.h`):
> The host should be excluded from panic, spillover, etc. calculations because it was explicitly
> taken out of rotation via protocol signal and **is not meant to be routed to.**
### Repro
1. Cluster with two EDS endpoints, `common_lb_config.healthy_panic_threshold = 50%`, round robin.
2. Endpoint **A**: normal, but failing its active health check.
3. Endpoint **B**: `health_status: DRAINING` (and passing its health check).
4. Send requests with **no** host override (no `override_host_status` match, no session cookie).
Panic engages because A is the only endpoint in the denominator and it is unhealthy
(`0 / (2 - 1) = 0% < 50%`).
**Observed:** roughly half the requests are served by the **DRAINING** endpoint B, returning 200.
```
cluster..membership_total: 2
cluster..membership_excluded: 1
cluster..membership_healthy: 0
cluster..lb_healthy_panic: 12
cluster..upstream_cx_none_healthy: 0
```
(12 requests sent; 6 reached the draining endpoint.)
**Expected:** a DRAINING endpoint should not receive new load-balanced traffic. It stays reachable
through an explicit host override (`common_lb_config.override_host_status` including `DRAINING`),
which is the documented mechanism for letting existing sessions finish.
### This is not limited to "all endpoints down"
The repro above uses zero healthy endpoints for simplicity, but that is not required. With `L`
non-excluded endpoints and `H` healthy ones, panic engages whenever `H/L` is below the configured
threshold (and below the overprovisioning cap of ~71.4%). Healthy endpoints can therefore be
available and serving while a DRAINING endpoint is *also* receiving new traffic.
Example: 4 endpoints + 1 DRAINING, `healthy_panic_threshold = 75%`, 2 healthy and 2 unhealthy.
Healthy is 50%, so panic engages and `AllHosts` spreads new requests across all five — including
the draining one — despite two healthy endpoints being available to take the load.
### Why this matters
Panic's rationale is *"most hosts look unhealthy — the health signal may be wrong, so ignore health
and try everything rather than fail."* That reasoning holds for a failed health check, which is an
inference. It does not hold for `DRAINING`, which is an explicit statement from the control plane.
A draining endpoint is not draining by mistake.
Practical consequence: a proxy answers 200 by using an endpoint the operator has already removed,
so the fact that every remaining endpoint is down produces no 5xx and no alert. If a session
affinity mechanism is in play, new clients can additionally be pinned to an endpoint that is about
to be shut down.
### Design question (not asserting a single answer)
`excludeBasedOnHealthFlag` groups three flags with different meanings:
| Flag | Meaning | Use in panic? |
|---|---|---|
| `PENDING_ACTIVE_HC` | Not yet health checked — status *unknown* | Probably **yes**; excluding these could make panic useless at startup |
| `EXCLUDED_VIA_IMMEDIATE_HC_FAIL` | Explicitly taken out of rotation via protocol signal | Probably **no** |
| `EDS_STATUS_DRAINING` | Explicit administrative removal | Probably **no** |
So "panic should skip `excludedHosts()`" may be too blunt; the narrower change is to skip only the
hosts that were *administratively* removed. Happy to prepare a PR once there is agreement on which
of the three should be excluded from panic selection.
Note `fail_traffic_on_panic` is not a workaround: it is all-or-nothing and also disables the
legitimate panic fallback when no draining hosts are present.
### Version
Observed on 1.38.3. The four code paths above are unchanged on `main` as of 2026-08-19.
Contributor guide
Research direction
Start with LoadBalancerBase::isHostSetInPanic and ZoneAwareLoadBalancerBase::hostSourceToUse/hostSourceToHosts in source/extensions/load_balancing_policies/common/load_balancer_impl.cc, then inspect excludeBasedOnHealthFlag in source/common/upstream/upstream_impl.cc and the exclusion comment in envoy/upstream/upstream.h. Clarify which flags should remain eligible during panic, especially EDS_STATUS_DRAINING, and validate that draining endpoints receive no new traffic except through the documented host override.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- backend, networking
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100