Skip to content

Improve Failure Detector on Broker. #230

Open
siddharthteotia wants to merge 1 commit into
masterfrom
siddharthteotia/failure-detector-fast-detect-health-probe
Open

Improve Failure Detector on Broker. #230
siddharthteotia wants to merge 1 commit into
masterfrom
siddharthteotia/failure-detector-fast-detect-health-probe

Conversation

@siddharthteotia

@siddharthteotia siddharthteotia commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator

Problem

JIRA - https://linkedin.atlassian.net/browse/OA-1693

  • While investigating the SLO breach for WvmxSearchAppearances PROD table, I discovered that a server was going unresponsive frequently (roughly every 2 hours).
  • For a couple of mins, broker kept sending query to a server while the requests were failing with 427.
  • Note that this table is 10-way replicated. Each replica group has 2 hosts but the traffic still kept landing on a host repeatedly
  • Error code 500 on broker resulted in uptime SLO violation.
  • The gaps (see RCA below) in the failure detector's circuit breaker machinery made the situation worse. Led to significantly more number of failed requests -> depleted budget -> breached SLO

Timeline

  • Tsecs: server stops responding for queries. socket is still open / established.
  • T to T+2 mins: several queries failed with 427 returned to broker.
    • Pinot error code: 427. 1 servers [<host>] not responded. Internal server error for query=blah
  • T + 3mins: The socket finally tears down (is a server side event). On the broker, the query receives
    • Caught exception while handling response from server: lva2-app27368_O java.net.SocketException: Connection reset
  • T + 3m1s: This is caught off the netty channel in the DataTableHandler.exceptionCaught() followed by channelnactive() listener marking the server down.
    • Channel for server: <host> is now inactive, marking server down

The 2 events at T + 3mins continue to repeat for about 30secs.

T = T + 3mins

06/26/2026, 9:52:48 AM
<redacted>
Channel for server: <redacted> is now inactive, marking server down
06/26/2026, 9:52:48 AM
<redacted>
Caught exception while handling response from server: <redacted>
06/26/2026, 9:52:46 AM
<redacted>
Channel for server: <redacted> is now inactive, marking server down
06/26/2026, 9:52:46 AM
<redacted>
Caught exception while handling response from server: <redacted>
06/26/2026, 9:52:46 AM
<redacted>
Channel for server: <redacted> is now inactive, marking server down
06/26/2026, 9:52:46 AM
<redacted>
Caught exception while handling response from server: <redacted>
............. repeat.......

T = T + 3mins 30secs.

After every 1.5 to 2 hours, the entire pattern gets repeated (from what we observed originally at T). In that gap of 1.5/2hours, the server is processing queries. Separate investigation on what happened at server.

Impact

Root Cause Analysis

Current state machine has gaps.

PHASE 1 — server goes silent , socket still established
TL;DR - No failure-detector call. Server stays in routing. It will be queried again.

────────────────────────────────────────────────────────────────
[broker query thread]
  SingleConnectionBrokerRequestHandler.processBrokerRequest()
    ├─ QueryRouter.submitQuery()
    │     └─ ServerChannels.ServerChannel.sendRequest() → Channel.writeAndFlush()   (request sent)
    └─ AsyncQueryResponse.getFinalResponses()
          └─ CountDownLatch.await(timeout) ── server never responds ──► TIMES OUT
               • status = TIMED_OUT
               • getFailedServer() == null                 
               • serversNotResponded → 427 SERVER_NOT_RESPONDING

PHASE 2 — socket finally reset
TL;DR- Server gets excluded from routing.

────────────────────────────────────────────────────────────────
[netty event-loop thread]
  NioEventLoop → read() → SocketException: Connection reset
    ├─ DataTableHandler.exceptionCaught()
    │     └─ LOG "Caught exception…" + meter RESPONSE_FETCH_EXCEPTIONS   ◄── no action
    └─ (channel closed) DataTableHandler.channelInactive()
          └─ QueryRouter.markServerDown()
                └─ AsyncQueryResponse.markServerDown() → markQueryFailed()
                      • sets _failedServer; latch.countDown()   (unblocks in-flight queries)
[broker query thread resumes]
  processBrokerRequest(): getFailedServer() != null
    └─ FailureDetector.markServerUnhealthy()                   ◄── 
          └─ unhealthyServerNotifier → RoutingManager.excludeServerFromRouting()

PHASE 3 — recovery probe (every ~5s) → FLAP
TL;DR - Eagerly re-admits after a TCP handshake succeeds. A server may be half-dead (not ready to serve queries) but the kernel will still be able to ACK the SYN. The flapping loop of UNHEALTHY -> HEALTHY-> UNHEALTHY -> HEALTHY is persistently tried with 5secs retry_delay since the eager re-admission does not preserve the back-off.

────────────────────────────────────────────────────────────────
[failure-detector-retry thread]  BaseExponentialBackoffRetryFailureDetector (retry loop)
    └─ SingleConnectionBrokerRequestHandler.retryUnhealthyServer()
          └─ QueryRouter.connect() → ServerChannel.connectWithoutLocking() → Bootstrap.connect()
                • TCP handshake SUCCEEDS 
             returns HEALTHY
    └─ markServerHealthy()
          └─ healthyServerNotifier → RoutingManager.includeServerToRouting()   (re-admitted)
          •   backoff RESET
  …      next query hits still-bad server → PHASE 2 → markServerUnhealthy → PHASE 3 …

Flap demonstration in the existing (old) code against the half-dead server (whose TCP connect always succeeds). Nominal  D = 5s  initial delay, no jitter in the example for clarity (jittering is applied in the existing code)

┌───────┬────────────────────────────────────────────────────────────────┬───────────────┬────────────────────────────────────────────┬─────────────────────────┬─────────────┐
│ ~time │ Event                                                          │ TCP probe     │ Detector action                            │ Backoff (_retryDelayNs) │ Routing     │
├───────┼────────────────────────────────────────────────────────────────┼───────────────┼────────────────────────────────────────────┼─────────────────────────┼─────────────┤
│ 0s    │ query fails → channelInactive → markServerUnhealthy            │ —             │ create RetryInfo #1                        │ 5s (initial)            │ EXCLUDED    │
├───────┼────────────────────────────────────────────────────────────────┼───────────────┼────────────────────────────────────────────┼─────────────────────────┼─────────────┤
│ 5s    │ retry fires → probe                                            │ ✅ connect OK │ markServerHealthy → RetryInfo #1 discarded │ (gone)                  │ RE-ADMITTED │
├───────┼────────────────────────────────────────────────────────────────┼───────────────┼────────────────────────────────────────────┼─────────────────────────┼─────────────┤
│ 5s    │ next query hits still-bad server → fails → markServerUnhealthy │ —             │ create RetryInfo #2                        │ 5s ← RESET              │ EXCLUDED    │
├───────┼────────────────────────────────────────────────────────────────┼───────────────┼────────────────────────────────────────────┼─────────────────────────┼─────────────┤
│ 10s   │ retry fires → probe                                            │ ✅ connect OK │ markServerHealthy → RetryInfo #2 discarded │ (gone)                  │ RE-ADMITTED │
├───────┼────────────────────────────────────────────────────────────────┼───────────────┼────────────────────────────────────────────┼─────────────────────────┼─────────────┤
│ 10s   │ next query fails → markServerUnhealthy                         │ —             │ create RetryInfo #3                        │ 5s ← RESET              │ EXCLUDED    │
├───────┼────────────────────────────────────────────────────────────────┼───────────────┼────────────────────────────────────────────┼─────────────────────────┼─────────────┤
│ 15s   │ retry fires → probe                                            │ ✅ connect OK │ markServerHealthy → discarded              │ (gone)                  │ RE-ADMITTED │
├───────┼────────────────────────────────────────────────────────────────┼───────────────┼────────────────────────────────────────────┼─────────────────────────┼─────────────┤
│ …     │ flaps every ~5s                                                │ ✅ always     │ re-admit → fail → re-mark                  │ stuck at 5s forever     │ oscillates  │
└───────┴────────────────────────────────────────────────────────────────┴───────────────┴────────────────────────────────────────────┴─────────────────────────┴─────────────

Solution

  • In the new state machine, upon receiving a 427 (server not responded), we invoke FailureDetector's notifyServerNotResponded. Upon 5 such consecutive events, failure detector pulls the server from routing as opposed to -- letting the queries be routed to it (receive 427) and wait for the socket connection reset trigger to invoke the failure detector.

  • Similarly on the recovery side, improved the health check probe to layer HTTP health readiness call on top of current QueryRouter.connect() (which succeeds after TCP ack).

  • Furthermore, the back-off delay is preserved in a way to prevent FLAP as much as possible. See the example timelines below to understand the recovery mechanics.

  • No new configs have been added.

    • Detector uses a hard-coded constant of CONSECUTIVE_FAILURES_THRESHOLD (5) to determine if server need to be taken out of routing.
    • Readiness probe uses a hard-coded constant of HEALTHY_THRESHOLD (3) to determine readmission.
    • Health check re-probe uses the existing retry.initial.delay.ms (5sec)
  • In the actual problematic scenario we saw in production (uptime SLO breach), the solution would lead to substantially fewer failed requests, and the impact duration drops from ~5 minutes → ~seconds. The error-budget burn per occurrence shrinks by a large factor. The bad server is taken out of routing quickly (detection is better) and doesn't flap back, letting the broker serve from the healthy replica groups and protecting the uptime SLO. The residual failures are a small.

Recovery examples

TL;DR -

  • If you see the table in RCA section, every cycle destroys and recreates the  RetryInfo  → the delay is reborn at 5s. The exponential growth ( _retryDelayNs *= factor ) only happens when the probe fails branch but here the probe succeeds every time (TCP connect lies) and the  RetryInfo  is discarded on re-admit, so growth never accumulates. So pinned at 5s → re-admit ↔ fail flap every ~5s for the whole outage window.

  • In this PR, the server is re-admitted only after 3 consecutive successful probes, and healthy check probe successes don't discard/reset the RetryInfo.

NOTE - Jittering existed in retry code and is left unchanged.

Scenario A — clean recovery (nominal, no jitter for simplicity):

┌───────┬────────────────────┬─────────┬──────────────┬─────────┬─────────────┬────────────┐
│ t (s) │ probe              │ result  │ succ         │ retries │ delay after │ routing    │
├───────┼────────────────────┼─────────┼──────────────┼─────────┼─────────────┼────────────┤
│ 0     │ (marked unhealthy) │ —       │ 0            │ 0       │ 5           │ excluded   │
├───────┼────────────────────┼─────────┼──────────────┼─────────┼─────────────┼────────────┤
│ 5     │ #1                 │ FAIL    │ 0            │ 1       │ 10          │ excluded   │
├───────┼────────────────────┼─────────┼──────────────┼─────────┼─────────────┼────────────┤
│ 15    │ #2                 │ FAIL    │ 0            │ 2       │ 20          │ excluded   │
├───────┼────────────────────┼─────────┼──────────────┼─────────┼─────────────┼────────────┤
│ 35    │ #3                 │ HEALTHY │ 1            │ 2       │ 20          │ excluded   │
├───────┼────────────────────┼─────────┼──────────────┼─────────┼─────────────┼────────────┤
│ 40    │ #4                 │ HEALTHY │ 2            │ 2       │ 20          │ excluded   │
├───────┼────────────────────┼─────────┼──────────────┼─────────┼─────────────┼────────────┤
│ 45    │ #5                 │ HEALTHY │ 3 → re-admit │ —       │ —           │ IN routing │
└───────┴────────────────────┴─────────┴──────────────┴─────────┴─────────────┴────────────┘

Scenario B — success interrupted (the anti-flap case, both successes and failures)

┌───────┬──────────────────────┬─────────┬──────────────┬─────────┬───────────────────────────────────┬────────────┬────────────┐
│ t (s) │ probe                │ result  │ succ         │ retries │ delay after                       │ routing    │ next probe │
├───────┼──────────────────────┼─────────┼──────────────┼─────────┼───────────────────────────────────┼────────────┼────────────┤
│ 0     │ — (marked unhealthy) │ —       │ 0            │ 0       │ 5                                 │ excluded   │ +5 → t=5   │
├───────┼──────────────────────┼─────────┼──────────────┼─────────┼───────────────────────────────────┼────────────┼────────────┤
│ 5     │ #1                   │ FAIL    │ 0            │ 1       │ 10                                │ excluded   │ +10 → t=15 │
├───────┼──────────────────────┼─────────┼──────────────┼─────────┼───────────────────────────────────┼────────────┼────────────┤
│ 15    │ #2                   │ HEALTHY │ 1            │ 1       │ 10 (unchanged)                    │ excluded   │ +5 → t=20  │
├───────┼──────────────────────┼─────────┼──────────────┼─────────┼───────────────────────────────────┼────────────┼────────────┤
│ 20    │ #3                   │ HEALTHY │ 2            │ 1       │ 10                                │ excluded   │ +5 → t=25  │
├───────┼──────────────────────┼─────────┼──────────────┼─────────┼───────────────────────────────────┼────────────┼────────────┤
│ 25    │ #4                   │ FAIL    │ 0            │ 2       │ 20 ← grew from 10, not reset to 5 │ excluded   │ +20 → t=45 │
├───────┼──────────────────────┼─────────┼──────────────┼─────────┼───────────────────────────────────┼────────────┼────────────┤
│ 45    │ #5                   │ HEALTHY │ 1            │ 2       │ 20                                │ excluded   │ +5 → t=50  │
├───────┼──────────────────────┼─────────┼──────────────┼─────────┼───────────────────────────────────┼────────────┼────────────┤
│ 50    │ #6                   │ HEALTHY │ 2            │ 2       │ 20                                │ excluded   │ +5 → t=55  │
├───────┼──────────────────────┼─────────┼──────────────┼─────────┼───────────────────────────────────┼────────────┼────────────┤
│ 55    │ #7                   │ HEALTHY │ 3 → re-admit │ —       │ (discarded)                       │ IN routing │ —          │
└───────┴──────────────────────┴─────────┴──────────────┴─────────┴───────────────────────────────────┴────────────┴────────────┘

Testing

  • Added new tests to simulate the new scenario in failure detector.

Follow-Ups

  • I noticed that entire failure dector code is not as clean as it can be. Abstractions are not clear + duplication of responsibilities. Refactor  markServerDown  vs  markServerUnhealthy  into a single explicit  FailureDetector  event surface, instead of the implicit  getFailedServer()  poll bridging three call sites.

  • Retry jitter: the pre-existing jitter is one-sided (only shortens). Consider a mean-preserving without shortening the effective backoff.

…ed recovery probe with probation

- Detect and quarantine a silent-but-connected server after N consecutive no-response
  queries (notifyServerResponded/notifyServerNotResponded), instead of waiting for a
  transport-level channelInactive/reset.
- Replace the bare TCP-connect recovery probe with an HTTP /health/readiness probe so a
  half-dead server (kernel completes the handshake but cannot serve) is not re-admitted.
- Add a probation window: require N consecutive successful probes before re-admitting and
  do not reset the exponential backoff on a single success, eliminating flapping.

Behavior uses hardcoded best-effort defaults (consecutive-failures threshold = 5, healthy
threshold = 3; probation re-probe reuses the retry initial delay) - no new configs added.

Tests: ConnectionFailureDetectorTest, SingleConnectionBrokerRequestHandlerHealthProbeTest,
FailureDetectorIncidentSimulationTest.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@siddharthteotia siddharthteotia force-pushed the siddharthteotia/failure-detector-fast-detect-health-probe branch from c8adb5d to 44305eb Compare July 2, 2026 23:45
@siddharthteotia siddharthteotia changed the title Broker failure detector: fast silent-server detection + readiness-based recovery probe (anti-flap) Improve failure detector on broker. Jul 3, 2026
@siddharthteotia siddharthteotia changed the title Improve failure detector on broker. Improve Failure Detector on Broker. Jul 3, 2026
@siddharthteotia siddharthteotia marked this pull request as ready for review July 3, 2026 21:19
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.

1 participant