jineshparakh opened a new pull request, #19407:
URL: https://github.com/apache/pinot/pull/19407
## Summary
On a freshly (re)started broker, the broker to server Netty channels start
empty. The **first** query to
each server therefore pays the blocking TCP `connect()`, and, when broker to
server TLS is enabled, the
full TLS handshake, **on its own critical path** while holding the per
server channel lock. Under a cold
burst of concurrent queries this serializes: every request thread targeting
that server blocks on the one
in flight connect, and the TLS handshake (two round trips plus certificate
validation) lengthens the
critical section.
This PR opens all broker to server channels **ahead of query traffic**
during startup, behind a readiness
gate. When it is enabled the broker reports `STARTING` until the channels
are connected (or a budget
expires), so no traffic is routed to it until the connect and handshake cost
has already been paid off the
query path. It is a startup only, best effort warmup: on by default, bounded
so it can never stall a
rolling restart, and a no op reversion to the existing lazy connect path
when disabled.
## What it does
1. **After Helix converges** at startup (the point at which routing, and the
servers it references,
exist), a background thread opens a channel to every routable server, for
**both** table types.
`ServerRoutingInstance` identity includes the table type, so OFFLINE and
REALTIME are separate channels
to the same physical server.
2. Pre-connect explicitly **awaits the TLS handshake** on the connecting
thread
(`SslHandler.handshakeFuture().sync()`), so the handshake, not just the
TCP connect, is off the first
query's critical path. On a plaintext channel there is no `SslHandler` in
the pipeline and this step is
a no op.
3. **Readiness is gated** on completion: the broker's `ServiceStatus`
reports `STARTING` (an existing
status value, so no new enum and mixed version peers are unaffected)
until pre-connect finishes. This
reuses the existing readiness endpoint that the Kubernetes startup probe
already polls; no health
endpoint or probe change is required.
4. **Bounded and best effort.** The whole step is capped by a configurable
budget; the readiness gate
opens even if pre-connect throws, is interrupted, or the budget expires,
so a slow or unreachable
server can never hold a broker not ready forever. A channel that fails to
connect simply falls back to
the existing lazy path. Connecting an already active channel is a no op,
so this is idempotent.
This is **single stage (SSE) only**. The multi stage (gRPC) and time series
paths use a different
transport and are unaffected.
## Configuration
| Property | Default | Description |
|---|---|---|
| `pinot.broker.startup.preconnect.enabled` | `true` | Open broker to server
channels at startup and gate readiness on it. Set to `false` to restore the
pure lazy connect path (behaviour then unchanged). |
| `pinot.broker.startup.preconnect.timeoutMs` | `30000` | Upper bound on the
whole pre-connect step (measured from Helix convergence). Channels not
connected within the budget fall back to the lazy path. |
## Metrics
| Metric | Type | Meaning |
|---|---|---|
| `STARTUP_PRECONNECT_DURATION_MS` | timer | Duration of pre-connect, from
Helix convergence to the last channel connecting or the budget expiring. |
| `NETTY_CONNECTION_CONNECT_TIME` | timer | Time to establish a single
broker to server channel (TCP connect plus TLS handshake). Complements the
existing last value gauge with a full distribution, so a burst of connections
on a cold broker is analyzable. |
## Performance
### Setup
All numbers below are from a **TLS-enabled** single broker test cluster (a
real ZK plus controller plus
server plus broker, not a synthetic mock):
- One broker and one server, with **broker to server TLS on** (`nettytls`),
so the channels pre-connect
opens pay a real TLS handshake. This is exactly the cost the fix targets.
- One offline table: about **4.0 million documents** across **41 segments**,
**MMAP** load mode, single
replica. It routes to the one server, so the broker holds **2 channels**
to it (OFFLINE and REALTIME).
- **Open loop** synthetic cold load at a fixed arrival rate of about **288
queries/second** (step
arrival, no ramp). Because arrivals do not wait for responses, under
elevated cold latency the in flight
count builds to **several hundred concurrent requests**, which is what
makes the per server connect and
handshake serialize.
- Two measurements per arm (`pinot.broker.startup.preconnect.enabled` OFF vs
ON), n=10 cold restarts each:
a per-query `serverStats` comparison of the first-query legs, and an
open-loop run split into a **cold**
window `[0, 8 s)` and a **warm** window `[150, 180 s)` measured from the
first traffic at readiness,
with wall and lock profiles captured.
- Engagement verified in the broker log every ON run: `Broker pre-connected
2/2 channel(s) in 70 to
121 ms`, then readiness.
### Gain
Pre-connect removes the connect and TLS handshake from the first query's
critical path. Measured directly
from the first query's `serverStats` legs (median [range] across 10 cold
restarts per arm):
```
first-query SubmitDelayMs (connect + channel lock) : 240 ms [167-518] ->
36 ms [0-53] (-85%)
first-second submit p95 : 257 ms [43-778] ->
38 ms [18-249] (-85%)
```
The broker to server **TLS handshake** for the 2 channels is now paid once
at startup
(`pre-connected 2/2 channel(s) in 70 to 121 ms`), off the query path, rather
than by the first query
holding the channel lock. This is corroborated by the wall profile: the
broker to server
connect/handshake park category (`QueryRouter.submitQuery` to
`ServerChannels.sendRequest` to park) is the
**only** category that changes between arms, dropping from **0.23% to
0.09%** (about 2.5x). By readiness
`_channel.isActive()` is `true`, so the lazy `connectWithoutLocking` path no
ops and the first query
serialization never happens.
Readiness cost is negligible: the gate adds effectively no startup time
(both arms reached readiness in
about 90 s), and it opens even on failure, so it cannot delay a rolling
restart beyond the configured
budget.
## Testing
- **Unit** (`ServerPreConnectorTest`, 6 tests): connects every server for
both table types; empty server
list is a no op; already passed deadline is a no op; counts only
successful connects; a throwing connect
is swallowed and the others still connect; the budget bounds the wait and
does not block on slow
connects. The connector takes its dependencies (routable server supplier,
connect function) as
functions, so parallelism, budget, and failure handling are covered
without a live broker.
- **Integration** (`BrokerServerPreConnectIntegrationTest`): brings up a
real ZK plus controller plus
server plus broker with an offline table and asserts (1) with pre-connect
enabled the readiness gate
opens, the broker's `ServiceStatus` reaches `GOOD`; and (2) the production
path (`RoutingManager`
routable server supplier to `QueryRouter` to `ServerChannels` to a live
server) opens one channel per
(server, table type).
## Backward compatibility
- Readiness reports the existing `STARTING` status, with **no new status
enum value**, so mixed version
broker/controller peers are unaffected.
- With `preconnect.enabled=false` the code path is a strict no op and
behaviour is identical to before.
- No wire protocol or serialization changes.
--
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]
