You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "ulysses-you (via GitHub)" <gi...@apache.org> on 2026/08/04 08:53:01 UTC

[PR] [SPARK-58552][SQL] Add total task time column to the SQL / DataFrame tab [spark]

ulysses-you opened a new pull request, #57751:
URL: https://github.com/apache/spark/pull/57751

   ### 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 `-1` and rendered as **N/A** in the UI, so it is not confused with a genuine `0 ms`.
   
   ### Why are the changes needed?
   
   `Duration` mixes scheduling/queueing overhead together with the actual compute time, so it cannot tell how much real task time a query consumed. The per-stage `executorRunTime` is 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:
   
   - The SQL / DataFrame tab and the SQL execution detail page gain a new "Total Task Time" column (rendered as `N/A` when the execution has no stages).
   - The SQL v1 REST API's `ExecutionData` gains a `totalTaskTime` field (in milliseconds, `-1` when unknown). The field defaults to `-1`, so existing API consumers are unaffected.
   
   ### How was this patch tested?
   
   - New unit test in `SqlResourceSuite` — `totalTaskTime aggregates executorRunTime across all attempts of all stages` — verifies the aggregation across multiple stages and attempts (including a retried attempt).
   - Extended `SqlResourceWithActualMetricsSuite` to assert the `totalTaskTime` field of the `sqlTable` endpoint with real queries.
   - Both suites pass: `SqlResourceSuite` (9 tests) and `SqlResourceWithActualMetricsSuite` (10 tests).
   - `dev/lint-js` passes.
   
   ### Was this patch authored or co-authored using generative AI tooling?
   
   Generated-by: Claude Code
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)
   


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-58552][SQL][UI] Add total task time column to the SQL / DataFrame tab [spark]

Posted by "sarutak (via GitHub)" <gi...@apache.org>.
sarutak commented on code in PR #57751:
URL: https://github.com/apache/spark/pull/57751#discussion_r3712759502


##########
sql/core/src/main/scala/org/apache/spark/status/api/v1/sql/SqlResource.scala:
##########
@@ -200,12 +206,33 @@ private[v1] class SqlResource extends BaseAppResource {
       case "duration" =>
         execs.sortBy(e =>
           e.completionTime.getOrElse(new Date()).getTime - e.submissionTime)
+      case "totalTaskTime" => execs.sortBy(e => totalTaskTime(e, store))

Review 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.



##########
sql/core/src/main/scala/org/apache/spark/status/api/v1/sql/SqlResource.scala:
##########
@@ -200,12 +206,33 @@ private[v1] class SqlResource extends BaseAppResource {
       case "duration" =>
         execs.sortBy(e =>
           e.completionTime.getOrElse(new Date()).getTime - e.submissionTime)
+      case "totalTaskTime" => execs.sortBy(e => totalTaskTime(e, store))
       case _ => execs.sortBy(_.executionId)
     }
     if (sortDir == "asc") sorted else sorted.reverse
   }
 
-  private def execToRow(exec: SQLExecutionUIData): java.util.LinkedHashMap[String, Object] = {
+  /**
+   * Total task time of an execution, in milliseconds, aggregated across all
+   * stages of the execution. Sums `executorRunTime` (the "Total Time Across All
+   * Tasks" metric) of every attempt of every stage: each attempt genuinely
+   * 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.
+   */

Review 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)
   ```
   



-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-58552][SQL][UI] Add total task time column to the SQL / DataFrame tab [spark]

Posted by "ulysses-you (via GitHub)" <gi...@apache.org>.
ulysses-you commented on PR #57751:
URL: https://github.com/apache/spark/pull/57751#issuecomment-5191400216

   **Merge Summary:**
   - merged into master https://github.com/apache/spark/commit/2dc9e1062dd09e1113d011387711d2ee0f371efd
   - merged into branch-4.x https://github.com/apache/spark/commit/0dd201761cb409e72d60a0dd8b15be66bb5b5491
   
   *Posted by `merge_spark_pr.py`*


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-58552][SQL][UI] Add total task time column to the SQL / DataFrame tab [spark]

Posted by "ulysses-you (via GitHub)" <gi...@apache.org>.
ulysses-you closed pull request #57751: [SPARK-58552][SQL][UI] Add total task time column to the SQL / DataFrame tab
URL: https://github.com/apache/spark/pull/57751


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-58552][SQL][UI] Add total task time column to the SQL / DataFrame tab [spark]

Posted by "uros-b (via GitHub)" <gi...@apache.org>.
uros-b commented on PR #57751:
URL: https://github.com/apache/spark/pull/57751#issuecomment-5179478912

   Nice change, thank you @ulysses-you! Leaving up to others to also review ^^


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-58552][SQL][UI] Add total task time column to the SQL / DataFrame tab [spark]

Posted by "ulysses-you (via GitHub)" <gi...@apache.org>.
ulysses-you commented on PR #57751:
URL: https://github.com/apache/spark/pull/57751#issuecomment-5178834095

   cc @dongjoon-hyun  @LuciferYang  @sarutak thank you


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-58552][SQL][UI] Add total task time column to the SQL / DataFrame tab [spark]

Posted by "ulysses-you (via GitHub)" <gi...@apache.org>.
ulysses-you commented on PR #57751:
URL: https://github.com/apache/spark/pull/57751#issuecomment-5188281880

   thank you @uros-b and @sarutak for review!


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org