kpatel006 opened a new pull request, #19908:
URL: https://github.com/apache/pinot/pull/19908
# Background
The broker has two separate ideas of which servers it can use:
- **Enabled servers** (`_enabledServerInstanceMap`): servers Helix marks as
enabled, i.e. not disabled by an operator.
- **Routable servers** (`_routableServerInstanceMap`): enabled servers minus
the ones the `FailureDetector` has
excluded after connection or query failures.
Instance selectors only ever see the routable set. If every replica of a
segment is excluded, the segment counts as
*unavailable*. The query then returns partial results and an
unavailable-segments error.
## Motivation
The failure detector can be wrong or too aggressive: a network blip, a slow
GC, or a burst of timeouts across replicas.
When that hits every replica of a segment, the broker drops data it could
probably have served, which is a
self-inflicted partial outage. **Fail-open routing** makes the broker try an
unhealthy replica as a last resort instead
of skipping the segment.
It never routes to a server an operator has disabled. It also never
overrides a healthy routing choice.
# Summary of changes
- When every replica of a segment is excluded by the failure detector, the
broker routes to an unhealthy replica that
an operator hasn't disabled.
- It's all-or-nothing: if any unavailable segment has no such replica, you
get the normal unavailable-segments result.
- Healthy picks are never overridden. Multi-stage queries are excluded for
now (see [Follow-up](#follow-up)).
- The balanced and replica-group selectors get their own fail-open selection
logic.
- A successful fail-open query doesn't mark the server healthy. Recovery is
still up to the `FailureDetector`.
- Multi-cluster brokers now forward exclude/include notifications to
remote-cluster routing managers. The retry path can
also find servers that exist only in a remote cluster.
> **This also fixes an existing gap in multi-cluster brokers, whether or not
fail-open is enabled.** On master, the
> failure detector only notifies the local routing manager, so
remote-cluster servers it flags are never excluded from
> routing. The retry path also can't find those servers, so they stay
"unhealthy" until max retries force them healthy.
> With this change, remote-cluster routing managers exclude and re-include
servers the same way the local one does.
# Follow-up
A separate PR (#720) will add fail-open support for multi-stage (MSE)
queries, `StrictReplicaGroupInstanceSelector`, and
`MultiStageReplicaGroupSelector`.
# Diagrams
## Request-time selection flow
```mermaid
flowchart TD
A["Query arrives<br/>BaseInstanceSelector.select()"] --> B["Normal
select()<br/>over routable servers"]
B --> C{"Any requested segments<br/>unavailable?"}
C -- No --> R1["Return normal mapping"]
C -- Yes --> D{"Any of them have<br/>enabled-but-excluded replicas?"}
D -- No --> R2["Return mapping +<br/>unavailable segments"]
D -- Yes --> M["Emit FAIL_OPEN_ELIGIBLE_REQUESTS"]
M --> E{"FailOpenPolicy enabled<br/>AND not multi-stage?"}
E -- No --> R2
E -- Yes --> F["For each unavailable segment:<br/>enabled replicas ∩
detector.getUnhealthyServers()"]
F --> G{"Every segment has<br/>≥1 candidate?"}
G -- No --> R2
G -- Yes --> H["selectForFailOpen()<br/>Balanced: round-robin by
requestId<br/>ReplicaGroup: fixed-replica off"]
H --> I{"Full coverage?"}
I -- No --> R2
I -- Yes --> J["Merge: healthy mapping + fail-open picks<br/>record
POOL_SEG_QUERIES"]
J --> R3["Return complete mapping,<br/>no unavailable segments"]
```
## Component wiring and state propagation
```mermaid
flowchart LR
subgraph Config
BC["Broker conf<br/>pinot.broker.enable.fail.open"]
CC["Cluster config override<br/>(dynamic)"]
end
BC --> FOP["FailOpenPolicy<br/>volatile enabled flag"]
CC -- "onChange()<br/>delete reverts to broker value" --> FOP
FD["FailureDetector"] -- "exclude/include server" -->
BS["BaseBrokerStarter /<br/>MultiClusterHelixBrokerStarter"]
BS --> LRM["BrokerRoutingManager (local)"]
BS -- "new: forwarded" --> RRM["RemoteClusterBrokerRoutingManager(s)"]
FOP --> LRM
FD --> LRM
FOP --> RRM
FD --> RRM
LRM -- "onInstancesChange(routable,<br/>failOpenEligible = all enabled,
changed)" --> IS["InstanceSelector<br/>(+ sampler selectors)"]
RRM --> IS
IS -- "rebuild
SegmentStates:<br/>failOpenCandidatesMap,<br/>segmentsWithoutCandidates" -->
SS["SegmentStates"]
SS -- "FAIL_OPEN_ELIGIBLE_SEGMENTS" --> MET["BrokerMetrics"]
RH["Request handlers<br/>(gRPC / SingleConn / MSE)"] --
"retryUnhealthyServer →<br/>getServerInstanceForRetry<br/>local, then
multi-cluster" --> FD
```
# Tests added
- **`InstanceSelectorTest`**: about 17 tests run per selector type. They
cover the policy being disabled, the
multi-stage gate, the all-or-nothing rule, rejection of disabled or
unassigned servers, keeping healthy picks,
deterministic choice by `requestId`, the replica-group behaviors, and pool
metrics.
- **`BrokerRoutingManagerTest`**: fail-open picks become ordinary routing
table entries, for both the main and sampler
selectors, and excluded servers stay eligible after an instance config
change.
- **`BaseBrokerRequestHandlerTest`**: covers the fallback order in
`getServerInstanceForRetry`.
- **`MultiClusterHelixBrokerStarterTest`**: checks that detector
notifications reach the remote routing managers,
including notifications that arrive before those managers exist.
--
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]
