yashmayya opened a new pull request, #19875:
URL: https://github.com/apache/pinot/pull/19875
## Problem
Multi-stage (MSE) queries can time out for hours after server pods restart
together, when every server listens on the same query and mailbox ports (for
example 8100 and 8101).
Two server pods can swap IP addresses during a batch restart. A gRPC channel
to server A can then keep A's old IP, which now belongs to server B. B accepts
the data meant for A, or runs A's stage plan, and A waits until the query times
out. Nothing logs an error.
## Why a channel keeps the old address
grpc-java 1.81 and 1.84 have the same code here:
- The default load balancer is the old `PickFirstLoadBalancer`.
- When a connection drops, the channel resolves the hostname once, at that
moment. A terminating pod can still be in DNS (for example with
`publishNotReadyAddresses=true`). `DnsNameResolver` also skips a refresh less
than 30 s after the last one.
- The next RPC, maybe much later, connects to that address and does not
resolve again. If another server now listens on that IP and port, the
connection works, and a working connection is never resolved again.
The existing `ServerGrpcChannelBackoffResetHandler` does not help: it only
resets the backoff of a channel in TRANSIENT_FAILURE, and a stale channel is
IDLE or READY.
## Fix
- **Servers:** when a peer server finishes startup, refresh the mailbox
channel to it with `ManagedChannel.enterIdle()`. The next stream then resolves
the hostname again. The handler is renamed to
`ServerMailboxChannelRefreshHandler`.
- **Brokers:** refresh the dispatch channel to a server each time the server
becomes routable. The routing callback now runs for every newly enabled server,
not only for servers that the failure detector excluded.
- **Old addresses:** when a server moves to another address (for example a
new random port) or leaves the cluster, shut down the channel to the old
address, unless another server uses that address now. Before, each restart left
one unused channel per peer.
- A refresh keeps the channel object that senders hold, and lets streams in
flight finish. A channel is refreshed at most once a second.
## Limits
This makes misdelivery rare, but cannot rule it out. DNS can still return
the old IP when the refresh runs. A query can reach the server between routing
and the refresh. Channels from servers to brokers are not refreshed. #19873
adds a receiver-side check for these cases.
## Compatibility
- No wire, config or metric change.
- API changes:
- `BaseBrokerRoutingManager.setServerReenableCallback` is now
`setServerEnabledCallback`, and runs for every newly enabled server.
- `QueryDispatcher.resetClientConnectionBackoff`,
`MailboxService.resetConnectBackoff` and `ChannelManager.resetConnectBackoff`
are removed. A refresh also clears the backoff.
- `MailboxService.shutdown()` also shuts down its client channels.
## Tests
- `ChannelManagerTest`: a refresh keeps the channel object and moves it to
IDLE; refreshes are debounced; retire and shutdown.
- `ServerMailboxChannelRefreshHandlerTest`: refresh on startup; retire when
a server moves or leaves; keep an address that another server took over; no
retire when a config read fails.
- `BrokerRoutingManagerTest`: the callback runs for every newly enabled
server, including a restarted one.
- `QueryDispatcherTest`: refresh and retire when a server is enabled; the
dispatch channel refresh is debounced.
- The full `pinot-query-runtime`, `pinot-server` and `pinot-broker` suites
pass.
This is the first of three PRs: this one, #19873 (receiver check) and a
third PR (link to follow) (diagnostics).
--
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]
