Skip to content

[SPARK-1301] add navigation links to StagePage for 3 tables #5121

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
ryan-williams wants to merge 2 commits into from
Closed

[SPARK-1301] add navigation links to StagePage for 3 tables #5121

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a1307b4
add navigation links to StagePage for 3 tables
ryan-williams Mar 21, 2015
95dfff0
fixes
ryan-williams Mar 21, 2015
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
18 changes: 18 additions & 0 deletions core/src/main/resources/org/apache/spark/ui/static/webui.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,24 @@ span.additional-metric-title {
color: #777;
}

#additional-metrics {
float: left;
padding-bottom: 10px;
}

#links {
float: left;
padding-left: 100px;
Copy link
Member

@srowen srowen Mar 21, 2015

Choose a reason for hiding this comment

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

This feels hacky, layout-wise. Can this be properly laid out in a table or something?

Copy link
Member

@srowen srowen Mar 21, 2015

Choose a reason for hiding this comment

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

Er, can it simply be at the top under the title?

Copy link
Contributor Author

@ryan-williams ryan-williams Mar 21, 2015

Choose a reason for hiding this comment

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

My understanding has always been that using <table>s to lay out things that are not tabular but that you merely want to be horizontally adjacent is hacky and should be avoided. If you're adamant I can do it though.

I triedy display: inline since that is maybe supposed to directly address this kind of use case:

that lets me kill the clear:both on the "Summary metrics …" <h4>, but it gets in to trouble when I expand the "additional metrics":

re: adding it under the title, it would be nice to not have this push the actual important info on the page further below the fold. There is a preponderance of unused horizontal space on the page currently, and the row it is being added to is a fairly underutilized one since the "additional metrics" dropdown is presumably not frequently engaged with.

Here it is full-screen on my macbook:

The "Summary Metrics …" <h4> is 31% of the way down the screen.

With "additional metrics" open it is 50% of the way down:

Copy link
Member

@srowen srowen Mar 21, 2015

Choose a reason for hiding this comment

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

shrug using absolute layout and clearing float feels worse than tables, but, the page isn't exactly optimized anyway. It seems like the "jump to" links should be at the top and could just use inline layout like the rest of this, even if it bumps everything down a line. Or... if floating, how about at the top-right to use some of the whitespace?

Copy link
Contributor Author

@ryan-williams ryan-williams Mar 21, 2015

Choose a reason for hiding this comment

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

Calling this "absolute layout" seems misleading, given what absolute usually means in this context (e.g. position: absolute). These links are positioned "relatively", to the right of the "additional metrics" <div>, with a fixed amount of padding.

I'm also not sure what you mean by "inline layout like the rest of this"; again, "inline" has a specific meaning in CSS, e.g. display: inline, that I don't think anything else on the page is using. Everything else is basically structured as <div>s and <table>s that are display: block by default meaning that they don't fall next to each other on the same line but each get their own lines, hence the horizontal wasted space.

I'm open to trying something like a float: right for the links, though relative to this that would mostly serve to make them farther away from the LHS where the user's eyes and mouse are likely to start out.

Here's a screen fwiw:

Lmk what you think.

Copy link
Member

@srowen srowen Mar 22, 2015

Choose a reason for hiding this comment

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

That's right, I'm just referring to "100px right" when I say absolute. It's not positioned absolutely vertically. And "inline" flow meaning, what you'd get just concatenating divs on a page rather than making one float right. That's the wrong term in HTML isn't it? I think "block" is what I'm looking for. I'm neutral on adding this in any event, so will let others pick it up or not.

}

#links a {
padding-left: 10px;
}

.clear {
Copy link
Member

@srowen srowen Mar 21, 2015

Choose a reason for hiding this comment

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

this is unused?

Copy link
Contributor Author

@ryan-williams ryan-williams Mar 21, 2015

Choose a reason for hiding this comment

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

no, it's added to the "Summary Metrics …" <h4> element here

Copy link
Member

@srowen srowen Mar 21, 2015

Choose a reason for hiding this comment

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

Got it, missed that.

clear: both;
}

/* Hide all additional metrics by default. This is done here rather than using JavaScript to
* avoid slow page loads for stage pages with large numbers (e.g., thousands) of tasks. */
.scheduler_delay, .deserialization_time, .fetch_wait_time, .shuffle_read_remote,
Expand Down
28 changes: 22 additions & 6 deletions core/src/main/scala/org/apache/spark/ui/jobs/StagePage.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ package org.apache.spark.ui.jobs
import java.util.Date
import javax.servlet.http.HttpServletRequest

import scala.xml.{Node, Unparsed}
import scala.xml.{Elem, Node, Unparsed}

import org.apache.commons.lang3.StringEscapeUtils

Expand Down Expand Up @@ -110,7 +110,7 @@ private[ui] class StagePage(parent: StagesTab) extends WebUIPage("stage") {
</div>

val showAdditionalMetrics =
<div>
<div id="additional-metrics">
<span class="expand-additional-metrics">
<span class="expand-additional-metrics-arrow arrow-closed"></span>
<strong>Show additional metrics</strong>
Expand Down Expand Up @@ -169,6 +169,17 @@ private[ui] class StagePage(parent: StagesTab) extends WebUIPage("stage") {
</div>
</div>

val quickLinks: Seq[Elem] =
(Some(<a href="#executors">Executors</a>) ::
(if (accumulables.nonEmpty) {
Some(<a href="#accumulators">Accumulators</a>)
} else {
None
}) ::
Some(<a href="#tasks">Tasks</a>) :: Nil).flatten

val showQuickLinks = <div id="links">Jump to: {quickLinks}</div>

val accumulableHeaders: Seq[String] = Seq("Accumulable", "Value")
def accumulableRow(acc: AccumulableInfo) = <tr><td>{acc.name}</td><td>{acc.value}</td></tr>
val accumulableTable = UIUtils.listingTable(accumulableHeaders, accumulableRow,
Expand Down Expand Up @@ -431,16 +442,21 @@ private[ui] class StagePage(parent: StagesTab) extends WebUIPage("stage") {
val executorTable = new ExecutorTable(stageId, stageAttemptId, parent)

val maybeAccumulableTable: Seq[Node] =
if (accumulables.size > 0) { <h4>Accumulators</h4> ++ accumulableTable } else Seq()
if (accumulables.size > 0) {
<h4 id="accumulators">Accumulators</h4> ++ accumulableTable
} else {
Seq()
}

val content =
summary ++
showAdditionalMetrics ++
<h4>Summary Metrics for {numCompleted} Completed Tasks</h4> ++
showQuickLinks ++
<h4 class="clear">Summary Metrics for {numCompleted} Completed Tasks</h4> ++
<div>{summaryTable.getOrElse("No tasks have reported metrics yet.")}</div> ++
<h4>Aggregated Metrics by Executor</h4> ++ executorTable.toNodeSeq ++
<h4 id="executors">Aggregated Metrics by Executor</h4> ++ executorTable.toNodeSeq ++
maybeAccumulableTable ++
<h4>Tasks</h4> ++ taskTable
<h4 id="tasks">Tasks</h4> ++ taskTable

UIUtils.headerSparkPage("Details for Stage %d".format(stageId), content, parent)
}
Expand Down