kosiew opened a new pull request, #24757:
URL: https://github.com/apache/datafusion/pull/24757
## Which issue does this PR close?
* Part of #23570
## Rationale for this change
Aggregate-specific metrics are a user-visible observability surface in
`EXPLAIN ANALYZE`, but their phase boundaries and coverage need to be clear and
consistent so users can understand where grouped aggregation time is spent.
This PR refines the timing boundaries for grouped aggregation paths and
documents how the operator-level and per-aggregate metrics should be
interpreted. In particular, it separates group-key preparation, aggregate
argument evaluation, accumulator work, output emission, and Grouped TopK
maintenance so the reported metrics more closely correspond to distinct
execution phases.
## What changes are included in this PR?
* Adds helper methods to `GroupByMetrics` to consistently time:
* group-key preparation,
* aggregate argument evaluation,
* accumulator update/merge work,
* output emission, and
* Grouped TopK priority-map maintenance.
* Extends `time_calculating_group_ids` to cover group expression evaluation
as well as group interning and ordering updates.
* Includes collectively evaluated aggregate filters in
`aggregate_arguments_time`.
* Changes `aggregation_time` to cover accumulator `update` and `merge`
operations as a single timed interval rather than repeatedly charging elapsed
time while iterating over aggregate expressions.
* Includes accumulator `state` and `evaluate` operations in `emitting_time`.
* Adds `topk_maintenance_time` for Grouped TopK and avoids registering
`aggregation_time` for that path, since Grouped TopK does not use accumulators.
* Applies the timing boundaries consistently across the aggregate hash
table, ordered aggregate table, legacy grouped hash stream, empty grouping-set
initialization, and Grouped TopK paths.
* Documents `AggregateExec` operator-level metrics and per-aggregate phase
timers, including indexed metric names, aggregate-expression labels,
mode-dependent phases, filter timing, Grouped TopK behavior, and `Dev`/verbose
display behavior.
## Are these changes tested?
Yes.
The patch adds a test verifying that `GroupByMetrics::new_topk` does not
register `aggregation_time`, while `time_aggregation` still executes its
closure.
The existing Grouped TopK metrics test is also extended to verify that:
* `topk_maintenance_time` is present and non-zero,
* `time_calculating_group_ids` is present and non-zero, and
* `aggregation_time` is not registered.
## Are there any user-facing changes?
Yes. The metrics reported for grouped aggregation have more explicit and
consistent phase boundaries, and Grouped TopK now reports a dedicated
`topk_maintenance_time` metric instead of attributing that work to group-ID
calculation.
The aggregate metrics documentation is expanded to describe the
operator-level metrics, per-aggregate phase timers, indexed naming and
expression labels, path-specific coverage, and how these metrics appear in
`EXPLAIN ANALYZE` and `EXPLAIN ANALYZE VERBOSE`.
There are no public API changes in this patch.
## 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]
