[SPARK-58552][SQL][UI] Add total task time column to the SQL / DataFrame tab - #57751
[SPARK-58552][SQL][UI] Add total task time column to the SQL / DataFrame tab#57751ulysses-you wants to merge 2 commits into
Conversation
Co-Authored-By: Claude <noreply@anthropic.com>
|
cc @dongjoon-hyun @LuciferYang @sarutak thank you |
|
Nice change, thank you @ulysses-you! Leaving up to others to also review ^^ |
| case "duration" => | ||
| execs.sortBy(e => | ||
| e.completionTime.getOrElse(new Date()).getTime - e.submissionTime) | ||
| case "totalTaskTime" => execs.sortBy(e => totalTaskTime(e, store)) |
There was a problem hiding this comment.
When sorting by totalTaskTime, the value is computed for all executions here, and then computed again for each execution on the current page in execToRow. Each computation does KVStore lookups for all stage attempts.
With the default page size of 20, the duplicated computation is limited to 20 executions × their stages, so this is unlikely to be a practical issue. sortBy's Schwartzian transform ensures the sort itself computes the key only once per element.
One approach would be to precompute a Map[Long, Long] (executionId -> totalTaskTime) and reuse it
in both sort and row construction.
| * consumed task time, including failed attempts that were retried. Returns -1 | ||
| * when the execution has no stages to aggregate, so callers can distinguish | ||
| * "no task time information" from a genuine zero. | ||
| */ |
There was a problem hiding this comment.
The Scaladoc says:
Sums `executorRunTime` (the "Total Time Across All Tasks" metric)
executorRunTime specifically measures the time executors spent running task code. It excludes deserialization time, result serialization time, and GC time. The "Total Time Across All Tasks" label used on the Stages page is the same metric, so the description isn't wrong, but a slightly more precise
phrasing might avoid confusion:
Sums `executorRunTime` (the cumulative time executors spent running tasks, which is the "Total Time Across All Tasks" stage-level metric)
- Precompute totalTaskTime per execution only when the list is sorted by it, and reuse the values for both the sort and the page rows instead of recomputing per stage attempt (review feedback). - Clarify the executorRunTime scaladoc: it is the cumulative time executors spent running tasks, excluding deserialization/serialization/GC time. Co-Authored-By: Claude <noreply@anthropic.com>
What changes were proposed in this pull request?
Add a Total Task Time column to the SQL / DataFrame tab, shown right after the existing Duration column. The value is the SQL-level total task time aggregated across all stages of the execution: it sums
executorRunTime(the "Total Time Across All Tasks" stage metric) over every attempt of every stage, since a retried/failed attempt also genuinely consumed task time.The same column also appears in the summary table on the SQL execution detail page, because the list page and the detail page share one set of column definitions.
When an execution has no stages to aggregate (e.g. a query that never launched a job), the value is reported as
-1and rendered as N/A in the UI, so it is not confused with a genuine0 ms.Why are the changes needed?
Durationmixes scheduling/queueing overhead together with the actual compute time, so it cannot tell how much real task time a query consumed. The per-stageexecutorRunTimeis already collected and shown on each stage page, but the SQL tab did not aggregate it up to the query level. A SQL-level total task time lets users compare the real compute cost of queries at a glance and identify compute-heavy plans.Does this PR introduce any user-facing change?
Yes:
N/Awhen the execution has no stages).ExecutionDatagains atotalTaskTimefield (in milliseconds,-1when unknown). The field defaults to-1, so existing API consumers are unaffected.How was this patch tested?
SqlResourceSuite—totalTaskTime aggregates executorRunTime across all attempts of all stages— verifies the aggregation across multiple stages and attempts (including a retried attempt).SqlResourceWithActualMetricsSuiteto assert thetotalTaskTimefield of thesqlTableendpoint with real queries.SqlResourceSuite(9 tests) andSqlResourceWithActualMetricsSuite(10 tests).dev/lint-jspasses.Snapshot:


Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (deepseek-v4-flash)