kosiew opened a new pull request, #25051:
URL: https://github.com/apache/datafusion/pull/25051
## Which issue does this PR close?
* Closes #23570
*
This is the last of a series of PR to close the issue
## Rationale for this change
The existing per-aggregate metrics measure complete accumulator operations
such as `update`, `merge`, `state`, and `evaluate`, but they cannot show how
much time an aggregate spends in meaningful internal subphases.
This PR adds a separate contract for optional aggregate-owned submetrics so
implementations can expose internal work while retaining a clear association
with the owning aggregate expression. These submetrics complement the existing
call-boundary timers rather than replacing them.
## What changes are included in this PR?
* Adds `AggregateMetric` and `AggregateMetrics` interfaces and optional
`set_metrics` hooks on `Accumulator` and `GroupsAccumulator`.
* Supplies aggregate-owned metrics when creating regular and groups
accumulators, including replacement accumulators.
* Adds lazily registered per-aggregate submetrics using the naming
convention `agg_expr_{index}_internal_{subphase}_time` and the existing
`aggregate` label.
* Preserves aggregate expression identity across partitions and repeated
aggregate functions.
* Instruments the distinct-value processing in `array_agg(DISTINCT ...)`
with the `distinct` subphase.
* Documents aggregate-internal submetrics and their relationship to the
existing `update`, `merge`, `state`, and `evaluate` timers.
## Are these changes tested?
Yes. This PR adds the following tests:
* `distinct_accumulator_size_includes_metric_handle`
* `aggregate_submetrics_merge_across_partitions`
* `aggregate_stream_reports_distinct_array_agg_submetrics`
* `aggregate_stream_merges_submetrics_across_partitions`
These cover accumulator size accounting, submetric naming and labels,
repeated `array_agg(DISTINCT ...)` expressions, coexistence with existing
call-boundary metrics, aggregates that do not request an internal submetric,
and collection across multiple partitions.
## Are there any user-facing changes?
Yes. Aggregate implementations can now expose optional internal timing
submetrics. `array_agg(DISTINCT ...)` reports its distinct-value processing
time as `agg_expr_{index}_internal_distinct_time`.
The new metrics are documented in the metrics user guide and can appear
alongside the existing per-aggregate operation timers in metric output such as
`EXPLAIN ANALYZE`.
This PR also adds public aggregate metrics interfaces and optional default
methods to the accumulator traits; existing accumulators that do not use
internal submetrics do not need to implement the new methods.
## LLM-generated code disclosure
This PR includes LLM-generated code and comments. All LLM-generated content
has been manually reviewed.
--
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]
