yashmayya opened a new pull request, #19972:
URL: https://github.com/apache/pinot/pull/19972
Follow-up to #19510. Today `approximateFunctionApplied` is set whenever the
broker rewrites `DISTINCTCOUNT` or `PERCENTILE` into a SMART function. A SMART
function stays exact until a group crosses its conversion threshold. So with a
high threshold, almost every rewritten query returns an exact result, but the
response still says it is approximate.
With this change, `approximateFunctionApplied` and the
`APPROXIMATE_FUNCTION_OVERRIDES` meter mean: a function that the broker rewrote
returned an approximate result.
## How
A SMART function cannot go back to exact, because a merge with a sketch
always returns a sketch. So the type of the final intermediate result shows
whether the result is approximate.
- **Hook:** `AggregationFunction#hasApproximatedResult()`, `false` by
default. The 4 SMART functions (`DISTINCTCOUNTSMARTHLL`,
`DISTINCTCOUNTSMARTHLLPLUS`, `DISTINCTCOUNTSMARTULL`, `PERCENTILESMARTTDIGEST`)
set it in `extractFinalResult` when they read a sketch. Function instances
belong to one query.
- **Single-stage:** after the reduce, `BrokerReduceService` checks the
functions that it used.
- A server that extracts final results itself (`serverReturnFinalResult`)
sends a new `approximatedResult` DataTable metadata key, and
`ExecutionStatsAggregator` ORs it in.
- The response metadata is read before the data table is built. So
`AggregationResultsBlock` now extracts final results once, before that point.
- **Multi-stage:** `AggregateOperator` sets a new `APPROXIMATED_RESULT`
stat. A leaf that returns final results maps the metadata key into a
`LeafOperator` stat. Both merge into a new `BrokerResponseNativeV2` stat, which
is not in the response JSON.
- **Handlers:** both set the flag only when the broker rewrote a function
AND a function returned an approximate result. A query that calls a SMART
function itself does not set the flag.
## Notes
- Group trimming also extracts final results, so a group that is trimmed
away can still set the flag. The error goes only one way: the flag can be true
when it does not need to be.
- **Mixed versions:** an older reader cannot skip the new keys safely.
- `StatMap.merge` reads `keys[ordinal]` with no bounds check, and
`MultiStageQueryStats` does not catch the resulting
`ArrayIndexOutOfBoundsException`. An older broker or upstream stage fails on
the new stat.
- `DataTableImplV4` skips an unknown metadata key without reading its
value, so the entries after it are misread.
- Both only happen when a query actually turns approximate, and only while
a reader runs an older version than the sender.
## Testing
- Unit tests:
- `SmartFunctionApproximatedResultTest`: all 4 SMART functions, and an
exact function that keeps the default.
- `BrokerReduceServiceTest`: a broker merge that crosses the threshold,
and a server that reports the metadata key.
- `ExecutionStatsAggregatorTest`, `AggregationResultsBlockTest` (server
final results), `AggregateOperatorTest` (group-by and aggregation-only),
`LeafOperatorTest`.
- `ApproximateFunctionOverrideIntegrationTest`, both engines:
- The flag is `false` under the threshold and `true` above it.
- The flag is `false` for a SMART function that the query calls itself.
- The wait for a cluster config change now checks the rewrite itself (the
single-stage column name and the multi-stage plan), not the flag.
- The distinct count column is now `Origin` instead of `AirlineID`.
`AirlineID` has one value per carrier, so a threshold of 1 never converted it.
--
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]
