st-omarkhalid opened a new pull request, #18602:
URL: https://github.com/apache/pinot/pull/18602
## Summary
Per-server scatter stats (submit delay, response delay, response size,
deserialization timing, request-sent delay) are computed by the Netty
single-connection transport during scatter and stashed on a local
\`BaseSingleStageBrokerRequestHandler.ServerStats\` holder inside
\`handleRequest\`. Today the formatted string only reaches the SLF4J
\`QueryLogger\` that consumes the holder before \`handleRequest\` returns;
subscribers of the \`onQueryCompletion(RequestContext, BrokerResponse)\` hook
and JSON-API clients have no way to read it because neither \`RequestContext\`
nor \`BrokerResponse\` exposes it.
This PR surfaces the string on \`BrokerResponse\`, following the existing
\`materializedViewQueried\` asymmetric pattern (default getter on the
interface, concrete setter on \`BrokerResponseNative\` only) so external
implementations need no override.
## Changes
- **\`BrokerResponse\`**: new \`default @Nullable String getServerStats()\`
returning \`null\`.
- **\`BrokerResponseNative\`**: new \`_serverStats\` field,
Jackson-annotated getter/setter, included in \`@JsonPropertyOrder\`, gated by
\`@JsonInclude(NON_NULL)\` so existing clients that don't expect the field see
no behavioral change.
- **\`BaseSingleStageBrokerRequestHandler#handleRequest\`**: copies
\`serverStats.getServerStats()\` onto the response just before
\`_queryLogger.logQueryCompleted(...)\`, on both the regular path and the
materialized-view split path.
Only the Netty SSE transport (\`SingleConnectionBrokerRequestHandler\`)
produces a non-null value today; gRPC SSE and MSE leave it \`null\` because OSS
does not currently compute a per-server stats string on those paths.
## Backward / forward compatibility
- **Wire format**: \`@JsonInclude(NON_NULL)\` keeps the field out of the
JSON response when null, so existing clients that don't expect it see no
payload change. Newer clients that do read it will simply see \`serverStats\`
absent on responses from older brokers — no parsing error.
- **SPI**: \`BrokerResponse.getServerStats()\` is a \`default\` method, so
external implementations of the interface continue to compile.
- **\`BrokerResponseNative.setServerStats\`** is intentionally not on the
interface, mirroring the established \`setMaterializedViewQueried\` asymmetry
so unknown impls can't silently drop the value.
## Testing
\`BrokerResponseNativeTest\` adds three tests:
1. **\`testServerStatsRoundTrip\`** — set, JSON-serialize, JSON-deserialize,
getter returns the same string.
2. **\`testServerStatsAbsentWhenNull\`** — null value is suppressed from the
JSON payload (backward-compat guarantee for existing clients).
3. **\`testServerStatsDefaultsToNull\`** — fresh \`BrokerResponseNative\`
reports \`null\`, so callers can distinguish \"not populated\" from \"populated
empty\".
\`\`\`
$ ./mvnw -pl pinot-common -am -Dtest='BrokerResponseNativeTest' \\
-Dsurefire.failIfNoSpecifiedTests=false -Dcheckstyle.skip -Dlicense.skip
test
[INFO] Tests run: 8, Failures: 0, Errors: 0, Skipped: 0
[INFO] BUILD SUCCESS
\`\`\`
Checkstyle: \`./mvnw -pl pinot-spi,pinot-common,pinot-broker
checkstyle:check\` → 0 violations.
## Motivation
Downstream components that subscribe to \`onQueryCompletion(rc, response)\`
to forward per-query records (e.g. async query-log Parquet pipelines) currently
see \`serverStats\` as \`null\` despite OSS having a usable per-server
breakdown string in hand. Adding this surface lets those subscribers
materialize the column without needing to fork OSS or carry per-thread state
across the \`handleRequest\` → \`onQueryCompletion\` stack boundary.
--
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]
