Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -58,4 +58,36 @@ table.dataTable thead .sorting_desc_disabled::after {

div.dataTables_wrapper div.dataTables_length select {
width: 100%;
}

/* SQL tab sub-execution disclosure (SPARK-56811) */
table#sql-table td.sub-exec-toggle {
white-space: nowrap;
}

table#sql-table td.sub-exec-toggle a.toggle-sub-exec {
text-decoration: none;
}

table#sql-table td.sub-exec-toggle a.toggle-sub-exec:hover {
text-decoration: underline;
}

table#sql-table tr.shown td.sub-exec-toggle a.toggle-sub-exec {
font-weight: 600;
}

table#sql-table tr.shown + tr > td {
background-color: var(--bs-tertiary-bg, #f4f7fa);
}

table.sub-exec-table {
margin-left: 1.5rem !important;
width: calc(100% - 1.5rem) !important;
background-color: transparent;
}

table.sub-exec-table thead th {
font-weight: 600;
background-color: transparent;
}
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,13 @@ $(document).ready(function () {
}
}

// Read the cluster-level grouping toggle rendered into the page by Scala
var groupSubExecEnabled = true;
var configEl = document.getElementById("group-sub-exec-config");
if (configEl) {
groupSubExecEnabled = configEl.getAttribute("data-value") === "true";
}

function init(resolvedAppId) {
var sqlTableEndPoint = createSQLTableEndPoint(resolvedAppId);

Expand All @@ -131,6 +138,91 @@ $(document).ready(function () {
'<table id="sql-table" class="table table-striped compact cell-border" ' +
'style="width:100%"></table>';

var columns = [
{
data: "id", name: "id", title: "ID",
render: function (data, type) {
if (type !== "display") return data;
var basePath = uiRoot + appBasePath;
return '<a href="' + basePath + '/SQL/execution/?id=' + data + '">' +
data + '</a>';
}
},
{
data: "queryId", name: "queryId", title: "Query ID",
orderable: false,
render: function (data, type) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

data (queryId) is interpolated into both a title attribute and visible text without escapeHtml. The other rendered columns (description, errorMessage) use escapeHtml. Today queryId is a UUID, so this is safe in practice, but the inconsistency is the kind of thing that bites later when a schema changes. Consider wrapping in escapeHtml(data) to match the rest.

if (type !== "display" || !data) return data || "";
return '<span title="' + data + '">' + data.substring(0, 8) + '...</span>';
}
},
{
data: "status", name: "status", title: "Status",
render: function (data, type) {
if (type !== "display") return data;
return statusBadge(data);
}
},
{
data: "description", name: "description", title: "Description",
render: function (data, type, row) {
if (type !== "display") return data || "";
return descriptionHtml({ id: row.id, description: data });
}
},
{
data: "submissionTime", name: "submissionTime", title: "Submitted",
render: function (data, type) {
if (type !== "display") return data;
return formatDateSql(data);
}
},
{
data: "duration", name: "duration", title: "Duration",
render: function (data, type) {
if (type !== "display") return data;
return formatDurationSql(data);
}
},
{
data: "jobIds", name: "jobIds", title: "Succeeded Jobs",
orderable: false,
render: function (data, type) {
if (type !== "display") return (data || []).join(",");
return jobIdLinks(data || []);
}
},
{
data: "errorMessage", name: "errorMessage", title: "Error Message",
orderable: false,
render: function (data, type) {
if (type !== "display" || !data) return data || "";
if (data.length > 100) {
return '<span title="' + escapeHtml(data) + '">' +
escapeHtml(data.substring(0, 100)) + '...</span>';
}
return escapeHtml(data);
}
}
];
if (groupSubExecEnabled) {
// Trailing "Sub Executions" column matching the SPARK-41752 / 4.1 layout:
// shows "+N sub" when the root has children, blank otherwise. Click to
// expand a child row containing the sub-execution rows.
columns.push({
data: null, name: "subExecutions", title: "Sub Executions",
orderable: false, searchable: false,
className: "sub-exec-toggle",
render: function (data, type, row) {
if (type !== "display") return "";
var subs = row.subExecutions || [];
if (subs.length === 0) return "";
return '<a href="#" class="toggle-sub-exec">' +
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The disclosure is a <a href="#"> with a click handler — functionally a button. Screen readers see it as a generic link with no expanded/collapsed state. Worth adding:

  • role="button" (so AT announces it as a control, not navigation),
  • aria-expanded="false" on render, toggled to "true" / "false" in the click handler alongside the shown class,
  • preferably aria-controls= referencing the generated child row id.

Existing components in the Spark UI are already inconsistent with such accessibility standards, and we can unify and standardize this in subsequent iterations.

'+' + subs.length + ' sub</a>';
}
});
}

var table = $("#sql-table").DataTable({
serverSide: true,
processing: true,
Expand All @@ -146,83 +238,52 @@ $(document).ready(function () {
if (sel) {
d.status = sel;
}
d.groupSubExecution = groupSubExecEnabled ? "true" : "false";
},
dataSrc: function (json) { return json.aaData; },
error: function () {
$("#sql-table_processing").css("display", "none");
}
},
columns: [
{
data: "id", name: "id", title: "ID",
render: function (data, type) {
if (type !== "display") return data;
var basePath = uiRoot + appBasePath;
return '<a href="' + basePath + '/SQL/execution/?id=' + data + '">' +
data + '</a>';
}
},
{
data: "queryId", name: "queryId", title: "Query ID",
orderable: false,
render: function (data, type) {
if (type !== "display" || !data) return data || "";
return '<span title="' + data + '">' + data.substring(0, 8) + '...</span>';
}
},
{
data: "status", name: "status", title: "Status",
render: function (data, type) {
if (type !== "display") return data;
return statusBadge(data);
}
},
{
data: "description", name: "description", title: "Description",
render: function (data, type, row) {
if (type !== "display") return data || "";
return descriptionHtml({ id: row.id, description: data });
}
},
{
data: "submissionTime", name: "submissionTime", title: "Submitted",
render: function (data, type) {
if (type !== "display") return data;
return formatDateSql(data);
}
},
{
data: "duration", name: "duration", title: "Duration",
render: function (data, type) {
if (type !== "display") return data;
return formatDurationSql(data);
}
},
{
data: "jobIds", name: "jobIds", title: "Succeeded Jobs",
orderable: false,
render: function (data, type) {
if (type !== "display") return (data || []).join(",");
return jobIdLinks(data || []);
}
},
{
data: "errorMessage", name: "errorMessage", title: "Error Message",
orderable: false,
render: function (data, type) {
if (type !== "display" || !data) return data || "";
if (data.length > 100) {
return '<span title="' + escapeHtml(data) + '">' +
escapeHtml(data.substring(0, 100)) + '...</span>';
}
return escapeHtml(data);
}
}
],
columns: columns,
order: [[0, "desc"]],
language: { search: "Search:&#160;" }
});

// Child-row expansion for sub-executions. Sub data is embedded per root row
// in the server payload (`row.subExecutions`), so no second fetch is needed.
if (groupSubExecEnabled) {
$("#sql-table tbody").on("click", "a.toggle-sub-exec", function (e) {
e.preventDefault();
var tr = $(this).closest("tr");
var dtRow = table.row(tr);
var rowData = dtRow.data();
var subs = (rowData && rowData.subExecutions) || [];
if (dtRow.child.isShown()) {
dtRow.child.hide();
tr.removeClass("shown");
$(this).text("+" + subs.length + " sub");
} else {
var basePath = uiRoot + appBasePath;
var html = '<table class="table table-sm table-bordered mb-0 sub-exec-table">';
html += '<thead><tr><th>ID</th><th>Status</th><th>Description</th>' +
'<th>Duration</th><th>Succeeded Jobs</th></tr></thead><tbody>';
subs.forEach(function (child) {
html += '<tr><td><a href="' + basePath + '/SQL/execution/?id=' +
child.id + '">' + child.id + '</a></td>';
html += '<td>' + statusBadge(child.status) + '</td>';
html += '<td>' + escapeHtml(child.description || "") + '</td>';
html += '<td>' + formatDurationSql(child.duration) + '</td>';
html += '<td>' + jobIdLinks(child.jobIds || []) + '</td></tr>';
});
html += '</tbody></table>';
dtRow.child(html).show();
tr.addClass("shown");
$(this).text("\u2212" + subs.length + " sub");
}
});
}

$("#status-filter").on("change", function () {
table.draw();
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import jakarta.ws.rs._
import jakarta.ws.rs.core.{Context, MediaType, UriInfo}

import org.apache.spark.JobExecutionStatus
import org.apache.spark.internal.config.UI.UI_SQL_GROUP_SUB_EXECUTION_ENABLED
import org.apache.spark.sql.execution.ui.{SparkPlanGraph, SparkPlanGraphCluster, SparkPlanGraphNode, SQLAppStatusStore, SQLExecutionUIData}
import org.apache.spark.status.api.v1.{BaseAppResource, NotFoundException}
import org.apache.spark.ui.UIUtils
Expand Down Expand Up @@ -74,6 +75,11 @@ private[v1] class SqlResource extends BaseAppResource {
* Server-side DataTables endpoint for SQL executions listing.
* Accepts DataTables server-side parameters (start, length, order, search)
* and returns paginated results with recordsTotal/recordsFiltered counts.
*
* When `groupSubExecution=true` (default = `spark.ui.sql.groupSubExecutionEnabled`),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The actual config (from core/src/main/scala/org/apache/spark/internal/config/UI.scala) is spark.ui.groupSQLSubExecutionEnabled. Fix the docstring so users don't waste time searching for a non-existent key.

* pagination is over root executions only and each row carries its sub-executions
* inline as `subExecutions: [...]`. Sub-executions whose root is missing from the
* filtered set (orphans) are surfaced as roots so they don't disappear.
*/
@GET
@Path("sqlTable")
Expand All @@ -85,7 +91,10 @@ private[v1] class SqlResource extends BaseAppResource {
// Echo draw counter to prevent stale responses
val draw = Option(uriParams.getFirst("draw")).map(_.toInt).getOrElse(0)

val totalRecords = sqlStore.executionsCount()
// Sub-execution grouping flag; default to the cluster config
val groupSubExec = Option(uriParams.getFirst("groupSubExecution"))
.map(_.toBoolean)
.getOrElse(ui.conf.get(UI_SQL_GROUP_SUB_EXECUTION_ENABLED))

// Search and status filter
val searchValue = Option(uriParams.getFirst("search[value]"))
Expand All @@ -94,9 +103,12 @@ private[v1] class SqlResource extends BaseAppResource {
.filter(_.nonEmpty)
val needsFilter = searchValue.isDefined || statusFilter.isDefined

// Always load all execs once. The KVStore-level pagination optimization
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// Always load all execs once. We need the full set to (a) identify orphan
// sub-executions whose root is filtered out and (b) count root rows for
// `recordsTotal`. `sqlStore.executionsList()` is already a full
// materialization, so there is no separate "KVStore-pagination" path being
// disabled here.

// referenced in earlier comments is no longer effective once grouping is
// enabled because we need the full set to identify roots and orphans.
val allExecs = sqlStore.executionsList()

val filteredExecs = if (needsFilter) {
// When filtering, we must load all and filter in memory
val allExecs = sqlStore.executionsList()
allExecs.filter { exec =>
val matchesSearch = searchValue.forall { search =>
val lower = search.toLowerCase(java.util.Locale.ROOT)
Expand All @@ -110,10 +122,21 @@ private[v1] class SqlResource extends BaseAppResource {
matchesSearch && matchesStatus
}
} else {
// No filter — will use KVStore pagination below
Seq.empty
allExecs
}

// Split filteredExecs into root rows and a sub-execution map. A root row is
// either an execution whose id equals its rootExecutionId, or an orphan sub
// whose root parent is absent from the filtered set.
val (rootRows, subsByRoot) = if (groupSubExec) {
val filteredIds = filteredExecs.iterator.map(_.executionId).toSet
val (roots, subs) = filteredExecs.partition { e =>
e.executionId == e.rootExecutionId || !filteredIds.contains(e.rootExecutionId)
}
(roots, subs.groupBy(_.rootExecutionId))
} else {
(filteredExecs, Map.empty[Long, Seq[SQLExecutionUIData]])
}
val filteredRecords = if (needsFilter) filteredExecs.size else totalRecords

// Sort
val sortCol = Option(uriParams.getFirst("order[0][column]"))
Expand All @@ -125,26 +148,48 @@ private[v1] class SqlResource extends BaseAppResource {
val start = Option(uriParams.getFirst("start")).map(_.toInt).getOrElse(0)
val length = Option(uriParams.getFirst("length")).map(_.toInt).getOrElse(20)

val page = if (needsFilter) {
// Filter/search: sort and paginate in memory
val sorted = sortExecs(filteredExecs, sortCol, sortDir)
if (length > 0) sorted.slice(start, start + length) else sorted
} else {
// No filter: use KVStore-level pagination for efficiency
// KVStore returns in insertion order; sort in memory for the page
val execs = sqlStore.executionsList()
val sorted = sortExecs(execs, sortCol, sortDir)
if (length > 0) sorted.slice(start, start + length) else sorted
val sortedRoots = sortExecs(rootRows, sortCol, sortDir)
val page = if (length > 0) sortedRoots.slice(start, start + length) else sortedRoots

// Convert to Java-compatible row data; embed sub-executions when grouping
val aaData = page.map { exec =>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A single CACHE TABLE on a large nested DAG, or a deeply-nested CTAS, can produce dozens of sub-executions under one root. The current design embeds every sub of every visible root into the same JSON response, so a page of 20 roots could carry hundreds of sub rows even if the user never expands any disclosure. For most workloads this is fine, but worth noting in the PR description — and worth considering a follow-up that lazy-loads sub rows on first expand (a separate ?rootId=N endpoint) if a user reports payload size issues.

val row = execToRow(exec)
if (groupSubExec) {
val subs = subsByRoot.getOrElse(exec.executionId, Seq.empty)
if (subs.nonEmpty) {
// Sort subs by id ascending so they appear in chronological order
val subRows = new java.util.ArrayList[java.util.LinkedHashMap[String, Object]]()
sortExecs(subs, "id", "asc").foreach(s => subRows.add(execToRow(s)))
row.put("subExecutions", subRows)
}
}
row
}

// Convert to Java-compatible row data
val aaData = page.map(execToRow)
// Counts: when grouping, totals reflect root-only counts so DataTables shows
// "Showing X to Y of Z entries" matching the rows the user actually sees.
val recordsTotal = if (groupSubExec) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When groupSubExec && needsFilter, this does a fresh O(N) Set construction + O(N) count on allExecs. The work is the same flavor as the filtered-set partition above (line 132-135) but on a different domain (all execs vs. filtered execs), so it can't be folded directly. Consider precomputing both partitions (filtered + unfiltered) up front behind a single helper, or memoizing allRootIds so the unfiltered count and the filtered set both consult the same lazily-built data.

if (needsFilter) {
// Re-derive root rows from the unfiltered set
val allIds = allExecs.iterator.map(_.executionId).toSet
allExecs.count { e =>
e.executionId == e.rootExecutionId || !allIds.contains(e.rootExecutionId)
}
} else {
rootRows.size
}
} else if (needsFilter) {
filteredExecs.size
} else {
sqlStore.executionsCount()
}
val recordsFiltered = if (groupSubExec) rootRows.size else filteredExecs.size

val ret = new HashMap[String, Object]()
ret.put("draw", Integer.valueOf(draw))
ret.put("aaData", aaData)
ret.put("recordsTotal", java.lang.Long.valueOf(filteredRecords))
ret.put("recordsFiltered", java.lang.Long.valueOf(filteredRecords))
ret.put("recordsTotal", java.lang.Long.valueOf(recordsTotal))
ret.put("recordsFiltered", java.lang.Long.valueOf(recordsFiltered))
ret
}
}
Expand Down
Loading