jineshparakh opened a new pull request, #19558:
URL: https://github.com/apache/pinot/pull/19558
## Intent
When a broker holds no routing entry for a table, the two query engines
report the same condition
with different error codes.
The single-stage engine returns `BROKER_RESOURCE_MISSING` (410). The
multi-stage engine signals it
with a plain precondition failure, which the request handler rewraps as
`INTERNAL` (450) because it
only propagates causes that are already a `QueryException`.
So a client sees 450 from one engine and 410 from the other for the
identical situation. That
matters because `INTERNAL` is the catch-all for any unexpected planning
fault — it tells the caller
nothing actionable and is indistinguishable from a genuine planner bug.
`BROKER_RESOURCE_MISSING` is
the specific signal for this case; `QueryErrorCode#isClientError` already
documents it as something
that "can be triggered due to issues with EV updates", which is exactly what
this is.
In practice the window shows up when a table moves between broker tenants,
or a broker is scaled
down: the broker keeps receiving queries for a table it no longer routes.
Single-stage clients get a
clear 410 they can react to; multi-stage clients get an opaque 450.
## What this changes
`WorkerManager` reaches the "no routing for this table" conclusion on two
planning paths, and both
now throw the typed `BROKER_RESOURCE_MISSING` exception instead of a bare
`IllegalStateException`:
| Method | Reached by |
|---|---|
| `assignWorkersToNonPartitionedLeafFragment` | ordinary leaf fragments |
| `calculatePartitionTableInfo` | partitioned / colocated-join plans |
Both are changed because a caller cannot be expected to know which planning
path its query took.
Error messages are unchanged, and `MultiStageBrokerRequestHandler` needs no
edit — it already passes
a `QueryException` through with its code intact.
### This is precisely the single-stage condition, not an approximation
Both engines end up asking the same question of the same object:
- Single-stage populates its offline/realtime route flags directly from
`RoutingManager.routingExists`
and ORs them for a hybrid table.
- Multi-stage's partitioned path makes the identical `routingExists` calls
and ORs them.
- Multi-stage's leaf path sees an empty routing map only when the routing
table came back null for
every table type — and the broker routing manager returns null **only**
when the routing entry is
absent.
That last point also rules out a false positive: a table whose segments are
all pruned away still
produces a non-null routing table with an empty server map, so neither site
can fire for "no matching
data". Those queries keep returning an empty result rather than an error.
## Scope
`WorkerManager` has 33 precondition checks. Only the two that mean "this
table has no routing" are
changed. The others — missing partition info, segments with an invalid
partition, no fully replicated
server — describe different conditions and would each need their own mapping
decision, so they are
deliberately left alone.
## Compatibility
A routeless table in the multi-stage engine now reports 410 where it
previously reported 450. A
client keying on 450 for that case would need to key on 410 — which is
already what it does for the
same case on the single-stage engine.
HTTP behaviour is unchanged by default: query errors are returned as HTTP
200 with the code in the
`X-Pinot-Error-Code` header, and only map to a real status when the caller
opts in via
`USE_HTTP_STATUS_FOR_ERRORS_HEADER` (where 410 maps to 503 rather than 500).
## Test plan
Two tests added to `WorkerManagerTest`, one per planning path, each
asserting the thrown exception
carries `BROKER_RESOURCE_MISSING`:
- `testLeafAssignmentReportsBrokerResourceMissingWhenRoutingIsAbsent`
- `testPartitionedAssignmentReportsBrokerResourceMissingWhenRoutingIsAbsent`
Results:
- `WorkerManagerTest`: **72 tests, 0 failures.**
- Both new tests were verified to be load-bearing: reverting either site to
the original precondition
makes the corresponding test fail.
- The four existing tests asserting `IllegalStateException` cover different
preconditions (invalid
segment partitions, no fully replicated server, no partition for the
table) and are unaffected.
- `checkstyle` and `license` pass on `pinot-query-planner`.
--
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]
