xiangfu0 opened a new pull request, #19568:
URL: https://github.com/apache/pinot/pull/19568
On tables with many segments, the top broker CPU consumer of a multi-stage
query is
`WorkerMetadata.setTableSegmentsMap`: for every leaf-stage worker the
planner JSON-encodes the full list of routed
segment names with Jackson, on the fixed-size
`multi-stage-query-compile-executor`, only for the dispatcher to copy
that string into a proto `map<string, string>` custom property and for every
server to JSON-parse it back (once per
worker).
This PR removes the encode from the compile path and adds a native protobuf
encoding for the segment lists, behind an
opt-in that can be flipped live.
## Changes
- `WorkerMetadata` now holds the segment maps as plain objects; nothing is
encoded at plan time anymore.
- `QueryPlanSerDeUtils` encodes them per server at dispatch time, once per
worker, in one of two wire encodings
picked per request:
- **proto**: new native `tableSegmentsMap` / `logicalTableSegmentsMap`
fields of `Worker.WorkerMetadata`
(`SegmentsMap` / `SegmentList` messages, new field numbers 4 and 5).
- **legacy JSON**: the previous custom-property string, still the default.
Decoding accepts both, so a new server understands every broker.
- New broker config `pinot.broker.mse.proto.segment.list` (default `false`)
and query option `protoSegmentList` to
enable the proto encoding.
- That config is **live**: `ProtoSegmentListPredicate` seeds it from the
static broker config and then follows cluster
config on the same key, registered by `BaseBrokerStarter` against the
existing cluster-config change handler, and
`QueryDispatcher` reads it per request. Precedence is per-query `SET
protoSegmentList`, then cluster config, then
static broker config; clearing the cluster-config key falls back to the
shipped `false` rather than to the static
seed. A `WARN` naming the precondition is logged on every transition to
enabled.
- New broker config `pinot.broker.mse.query.compile.executor.threads`. The
compile executor was hard-wired to half the
cores and saturates (queueing every query) well before the broker does on
short-query workloads. A non-positive
value keeps the old sizing.
## Backward compatibility
The proto fields are additive, so the wire format stays compatible in both
directions and the default behavior is
byte-for-byte what it is today.
The new encoding **must stay opt-in for one release**: an older server finds
no segments under the proto fields,
concludes the worker is not a leaf-stage worker and fails its leaf stage. It
may only be enabled once every server in
the cluster runs a version that understands the fields. That is exactly why
the config is watched in cluster config —
operators can turn it on the moment a rolling upgrade completes, and turn it
back off immediately if it misbehaves,
without restarting the brokers.
The explain path (`QueryServer`) deliberately keeps the legacy encoding:
that response travels server to broker and
cannot negotiate an encoding with its requester.
## Benchmark
Micro-benchmark, one leaf worker, 60-char segment names (Java 25, x86):
| segments | legacy JSON encode | proto encode | legacy decode | proto
decode |
|---------:|-------------------:|-------------:|--------------:|-------------:|
| 1,000 | 177 us | 132 us | 66 us | 29
us |
| 3,000 | 624 us | 394 us | 200 us | 95
us |
| 20,000 | 5,101 us | 2,709 us | 1,419 us | 782
us |
| 60,000 | 15,950 us | 8,217 us | 4,602 us | 2,386
us |
Beyond the raw encode/decode win, the encode no longer happens on the
compile executor at all.
## Testing
New tests:
- `QueryPlanSerDeUtilsTest` — round-trips both encodings, and covers a
legacy-broker message decoded by a new server
(JSON custom property present, proto fields absent) and the reverse.
- `ProtoSegmentListPredicateTest` — static seed, cluster-config override,
precedence, clearing the key, and
unparseable values.
Verified on JDK 25 against master `a979a6a`:
- `QueryPlanSerDeUtilsTest` 10, `ProtoSegmentListPredicateTest` 10,
`QueryDispatcherTest` 72, `QueryServerTest` 144,
`QueryServerAuthzTest` 2 — 238 tests, 0 failures.
- `MultiStageBrokerRequestHandlerTest` 11, `QueryOptionsUtilsTest` 28 — 0
failures.
- `spotless:apply`, `license:format`, `checkstyle:check`, `license:check`
clean on all five touched modules; no new
compiler warnings on added lines.
--
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]
