Add shuffle-read fetch metrics and surface per-operator metrics in plan display#1968
Open
andygrove wants to merge 6 commits into
Open
Add shuffle-read fetch metrics and surface per-operator metrics in plan display#1968andygrove wants to merge 6 commits into
andygrove wants to merge 6 commits into
Conversation
…governor permit wait
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
Closes #1958.
Rationale for this change
Diagnosing shuffle-heavy queries (for example the SF100 TPC-H Ballista-vs-Comet gap) is currently blind.
ShuffleReaderExecexposes onlyBaselineMetrics(elapsed_compute/output_rows), so the shuffle-read/transport cost — the time spent fetching remote partitions, the bytes moved, and the time blocked on the reduce-side in-flight governor (#1951) — is unmeasurable. Separately, the executor'sLoggingMetricsCollectorrenders only the shuffle-writer wrapper metrics, so inner-operator compute (SortExec/SortMergeJoinExec/DataSourceExecelapsed_compute/spill) never appears even though DataFusion already collects it. Together these make it impossible to attribute where a shuffle-heavy query spends its time.What changes are included in this PR?
Fetch metrics on
ShuffleReaderExec. AShuffleReadMetricsstruct (mirroring the existingShuffleWriteMetrics) recorded per output partition and threaded throughsend_fetch_partitionsinto the concurrent local/remote fetch tasks:fetch_time— wall-time fetching remote partitions (Arrow-Flight fetch + buffering)local_read_time— wall-time opening node-local shuffle filespermit_wait_time— a single combined timer over the three reduce-side governor semaphores (request + per-address + byte), i.e. backpressure wait (feat: bound shuffle fetch with a reduce-side in-flight governor #1951)decoded_bytes— decoded in-memory Arrow footprint of fetched remote partitions (not compressed wire bytes)fetch_requests— total remote fetch attempts issued, including retriesfetch_retries— of those, how many were retrieslocal_partitions/remote_partitions— the local/remote read splitAll metrics go through the standard
MetricBuilder/MetricsSetmachinery, so they also appear inEXPLAIN ANALYZE. The handles areArc-backed atomics cloned into the concurrent fetch tasks (no added locking). Doc comments note thatfetch_time/permit_wait_timeare summed across concurrent tasks (aggregate cost, not wall-clock) and thatfetch_timeis additive to — not a re-slice of — the reader's existingelapsed_compute.Per-operator metrics in the plan display.
ShuffleWriterExec::Displaynow renders the child plan with metrics (DisplayableExecutionPlan::with_metrics,ShowMetrics::Aggregated) in addition to statistics, so inner-operatorelapsed_compute/spill/output-rows surface inLoggingMetricsCollectoroutput andEXPLAIN ANALYZE. (The originally-consideredset_show_metricssetter does not exist onDisplayableExecutionPlan;with_metricsis the equivalent constructor and is already used elsewhere in the repo.)Are there any user-facing changes?
No API changes. The additional metrics are visible in
EXPLAIN ANALYZEoutput and executor metrics logs.