Broker silently masks DataTable deserialization failures as query timeouts
- Dominant language
- Java
- Stars
- 6.1k
- Forks
- 1.5k
- Avg merge
- 2d 3h
- Merged PRs (30d)
- 195
Description
### Problem
If a Pinot server sends back a response that the broker can't deserialize into a `DataTable`, the broker just logs the error and bumps a metric — it never tells the query that this server failed. The query then sits and waits until the whole broker timeout expires, and finally returns a `BROKER_TIMEOUT` with partial results.
So a real error ("server X sent bad bytes") shows up as a timeout, which is misleading and slow.
### Where
`pinot-core/src/main/java/org/apache/pinot/core/transport/DataTableHandler.java`, in `channelRead0`:
```java
} catch (Exception e) {
LOGGER.error("Caught exception while deserializing data table of size: {} from server: {}",
responseSize, _serverRoutingInstance, e);
_brokerMetrics.addMeteredGlobalValue(BrokerMeter.DATA_TABLE_DESERIALIZATION_EXCEPTIONS, 1);
// Nothing signals the query — its latch is never decremented for this server.
}
```
### Side effects (beyond the timeout itself)
Because nothing tells the query or the broker about the failure:
- **Failure detector isn't notified.** `AsyncQueryResponse.getFailedServer()` stays `null`, so `_failureDetector.markServerUnhealthy(...)` never runs. The same server keeps receiving queries and the bug keeps repeating.
- **Routing stats get poisoned.** In `getFinalResponses()`, a server that "didn't respond" is charged the full `_timeoutMs` as its latency. The bad server looks uniformly slow to every future routing decision, even though it actually replied within milliseconds — just with unusable bytes.
- **Generic timeout metric hides the real cause.** `BROKER_RESPONSES_WITH_TIMEOUTS` goes up. Dashboards read "broker is timing out queries" instead of "server X is sending garbage".
- **Channel stays open and healthy.** Netty has no idea anything went wrong, so the same connection keeps taking new requests.
### How to reproduce
In `QueryRoutingTest`, send garbage bytes (e.g. `new byte[]{0,1,2,3,4,5}`) as the server response and submit a query with a short timeout. Today the query blocks for the full timeout and returns `BROKER_TIMEOUT`. After a fix it should return quickly with a clear deserialization error.
### Suggested fix
1. In the catch block above, notify `QueryRouter` that this server failed for this query so the response can complete immediately (instead of waiting for timeout).
- **Design note**: `AsyncQueryResponse.markQueryFailed(...)` currently drains **all** remaining latches, aborting the whole query on any single-server failure. That's likely too aggressive for a deserialization error — a new per-server signal that decrements only this server's latch and records the exception on its `ServerResponse` is probably a better fit, so the query can still return partial results from the healthy servers.
2. Add a `DESERIALIZATION_ERROR` value to `QueryErrorCode` so the failure is distinguishable from `INTERNAL` and `BROKER_TIMEOUT`.
3. Add a regression test in `QueryRoutingTest` covering the case above.
Contributor guide
Research direction
Start in pinot-core/src/main/java/org/apache/pinot/core/transport/DataTableHandler.java at channelRead0, then trace AsyncQueryResponse, QueryRouter, and QueryErrorCode to understand per-server completion and error propagation. Reproduce the garbage-byte response in QueryRoutingTest; done means the query completes promptly with DESERIALIZATION_ERROR, preserves healthy-server partial results, and no longer reports only BROKER_TIMEOUT.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java
- Domain
- backend-api-design, distributed-systems, testing
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100