apache / apache/pinot

Broker silently masks DataTable deserialization failures as query timeouts

Open
#18,866 2 comments 0 reactions 0 assignees View on GitHub
bug query
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.