Skip to content

[SPARK-23501][UI] Refactor AllStagesPage in order to avoid redundant code #20663

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
mgaido91 wants to merge 3 commits into from
Closed

[SPARK-23501][UI] Refactor AllStagesPage in order to avoid redundant code #20663

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d246df2
[SPARK-23475][UI][FOLLOWUP] Refactor AllStagesPage in order to avoid …
mgaido91 Feb 23, 2018
9a8e032
address comments
mgaido91 Feb 26, 2018
a5c2f2f
address comment
mgaido91 Feb 27, 2018
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
261 changes: 102 additions & 159 deletions core/src/main/scala/org/apache/spark/ui/jobs/AllStagesPage.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,46 +19,20 @@ package org.apache.spark.ui.jobs

import javax.servlet.http.HttpServletRequest

import scala.xml.{Node, NodeSeq}
import scala.xml.{Attribute, Elem, Node, NodeSeq, Null, Text}

import org.apache.spark.scheduler.Schedulable
import org.apache.spark.status.PoolData
import org.apache.spark.status.api.v1._
import org.apache.spark.status.{AppSummary, PoolData}
import org.apache.spark.status.api.v1.{StageData, StageStatus}
import org.apache.spark.ui.{UIUtils, WebUIPage}

/** Page showing list of all ongoing and recently finished stages and pools */
private[ui] class AllStagesPage(parent: StagesTab) extends WebUIPage("") {
private val sc = parent.sc
private val subPath = "stages"
private def isFairScheduler = parent.isFairScheduler

def render(request: HttpServletRequest): Seq[Node] = {
val allStages = parent.store.stageList(null)

val activeStages = allStages.filter(_.status == StageStatus.ACTIVE)
val pendingStages = allStages.filter(_.status == StageStatus.PENDING)
val skippedStages = allStages.filter(_.status == StageStatus.SKIPPED)
val completedStages = allStages.filter(_.status == StageStatus.COMPLETE)
val failedStages = allStages.filter(_.status == StageStatus.FAILED).reverse

val numFailedStages = failedStages.size
val subPath = "stages"

val activeStagesTable =
new StageTableBase(parent.store, request, activeStages, "active", "activeStage",
parent.basePath, subPath, parent.isFairScheduler, parent.killEnabled, false)
val pendingStagesTable =
new StageTableBase(parent.store, request, pendingStages, "pending", "pendingStage",
parent.basePath, subPath, parent.isFairScheduler, false, false)
val completedStagesTable =
new StageTableBase(parent.store, request, completedStages, "completed", "completedStage",
parent.basePath, subPath, parent.isFairScheduler, false, false)
val skippedStagesTable =
new StageTableBase(parent.store, request, skippedStages, "skipped", "skippedStage",
parent.basePath, subPath, parent.isFairScheduler, false, false)
val failedStagesTable =
new StageTableBase(parent.store, request, failedStages, "failed", "failedStage",
parent.basePath, subPath, parent.isFairScheduler, false, true)

// For now, pool information is only accessible in live UIs
val pools = sc.map(_.getAllPools).getOrElse(Seq.empty[Schedulable]).map { pool =>
val uiPool = parent.store.asOption(parent.store.pool(pool.name)).getOrElse(
Expand All @@ -67,152 +41,121 @@ private[ui] class AllStagesPage(parent: StagesTab) extends WebUIPage("") {
}.toMap
val poolTable = new PoolTable(pools, parent)

val shouldShowActiveStages = activeStages.nonEmpty
val shouldShowPendingStages = pendingStages.nonEmpty
val shouldShowCompletedStages = completedStages.nonEmpty
val shouldShowSkippedStages = skippedStages.nonEmpty
val shouldShowFailedStages = failedStages.nonEmpty
val allStatuses = Seq(StageStatus.ACTIVE, StageStatus.PENDING, StageStatus.COMPLETE,
StageStatus.SKIPPED, StageStatus.FAILED)

val allStages = parent.store.stageList(null)
val appSummary = parent.store.appSummary()
val completedStageNumStr = if (appSummary.numCompletedStages == completedStages.size) {
s"${appSummary.numCompletedStages}"
} else {
s"${appSummary.numCompletedStages}, only showing ${completedStages.size}"
}

val (summaries, tables) = allStatuses.map(
Copy link
Member

@gengliangwang gengliangwang Feb 27, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Can we have two separate functions for getting summaries and tables? So that the code is more straight forward.

Overall LGTM.

Copy link
Contributor Author

@mgaido91 mgaido91 Feb 27, 2018

Choose a reason for hiding this comment

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

since there is some (few) logic which is common to the two (ie. when they are empty), I chose this approach in order to avoid redundancy and enforce coherence. But I am fine also having two separate functions. What do you think @vanzin ?

Copy link
Contributor

@vanzin vanzin Feb 27, 2018

Choose a reason for hiding this comment

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

I think the current version is fine. It avoids filtering a potentially long list of stages twice.

summaryAndTableForStatus(allStages, appSummary, _, request)).unzip

val summary: NodeSeq =
<div>
<ul class="unstyled">
{
if (shouldShowActiveStages) {
<li>
<a href="#active"><strong>Active Stages:</strong></a>
{activeStages.size}
</li>
}
}
{
if (shouldShowPendingStages) {
<li>
<a href="#pending"><strong>Pending Stages:</strong></a>
{pendingStages.size}
</li>
}
}
{
if (shouldShowCompletedStages) {
<li id="completed-summary">
<a href="#completed"><strong>Completed Stages:</strong></a>
{completedStageNumStr}
</li>
}
}
{
if (shouldShowSkippedStages) {
<li id="completed-summary">
<a href="#skipped"><strong>Skipped Stages:</strong></a>
{skippedStages.size}
</li>
}
}
{
if (shouldShowFailedStages) {
<li>
<a href="#failed"><strong>Failed Stages:</strong></a>
{numFailedStages}
</li>
}
}
{summaries.flatten}
</ul>
</div>

var content = summary ++
{
if (sc.isDefined && isFairScheduler) {
<span class="collapse-aggregated-poolTable collapse-table"
onClick="collapseTable('collapse-aggregated-poolTable','aggregated-poolTable')">
<h4>
<span class="collapse-table-arrow arrow-open"></span>
<a>Fair Scheduler Pools ({pools.size})</a>
</h4>
</span> ++
<div class="aggregated-poolTable collapsible-table">
{poolTable.toNodeSeq}
</div>
} else {
Seq.empty[Node]
}
}
if (shouldShowActiveStages) {
content ++=
<span id="active" class="collapse-aggregated-allActiveStages collapse-table"
onClick="collapseTable('collapse-aggregated-allActiveStages',
'aggregated-allActiveStages')">
<h4>
<span class="collapse-table-arrow arrow-open"></span>
<a>Active Stages ({activeStages.size})</a>
</h4>
</span> ++
<div class="aggregated-allActiveStages collapsible-table">
{activeStagesTable.toNodeSeq}
</div>
}
if (shouldShowPendingStages) {
content ++=
<span id="pending" class="collapse-aggregated-allPendingStages collapse-table"
onClick="collapseTable('collapse-aggregated-allPendingStages',
'aggregated-allPendingStages')">
val poolsDescription = if (sc.isDefined && isFairScheduler) {
<span class="collapse-aggregated-poolTable collapse-table"
onClick="collapseTable('collapse-aggregated-poolTable','aggregated-poolTable')">
<h4>
<span class="collapse-table-arrow arrow-open"></span>
<a>Pending Stages ({pendingStages.size})</a>
<a>Fair Scheduler Pools ({pools.size})</a>
</h4>
</span> ++
<div class="aggregated-allPendingStages collapsible-table">
{pendingStagesTable.toNodeSeq}
<div class="aggregated-poolTable collapsible-table">
{poolTable.toNodeSeq}
</div>
} else {
Seq.empty[Node]
}

val content = summary ++ poolsDescription ++ tables.flatten.flatten

UIUtils.headerSparkPage("Stages for All Jobs", content, parent)
}

def summaryAndTableForStatus(
allStages: Seq[StageData],
appSummary: AppSummary,
status: StageStatus,
request: HttpServletRequest): (Option[Elem], Option[NodeSeq]) = {
val stages = if (status == StageStatus.FAILED) {
allStages.filter(_.status == status).reverse
} else {
allStages.filter(_.status == status)
}
if (shouldShowCompletedStages) {
content ++=
<span id="completed" class="collapse-aggregated-allCompletedStages collapse-table"
onClick="collapseTable('collapse-aggregated-allCompletedStages',
'aggregated-allCompletedStages')">
<h4>
<span class="collapse-table-arrow arrow-open"></span>
<a>Completed Stages ({completedStageNumStr})</a>
</h4>
</span> ++
<div class="aggregated-allCompletedStages collapsible-table">
{completedStagesTable.toNodeSeq}
</div>

if (stages.isEmpty) {
(None, None)
} else {
val killEnabled = status == StageStatus.ACTIVE && parent.killEnabled
val isFailedStage = status == StageStatus.FAILED

val stagesTable =
new StageTableBase(parent.store, request, stages, statusName(status), stageTag(status),
parent.basePath, subPath, parent.isFairScheduler, killEnabled, isFailedStage)
val stagesSize = stages.size
(Some(summary(appSummary, status, stagesSize)),
Some(table(appSummary, status, stagesTable, stagesSize)))
}
if (shouldShowSkippedStages) {
content ++=
<span id="skipped" class="collapse-aggregated-allSkippedStages collapse-table"
onClick="collapseTable('collapse-aggregated-allSkippedStages',
'aggregated-allSkippedStages')">
<h4>
<span class="collapse-table-arrow arrow-open"></span>
<a>Skipped Stages ({skippedStages.size})</a>
</h4>
</span> ++
<div class="aggregated-allSkippedStages collapsible-table">
{skippedStagesTable.toNodeSeq}
</div>
}

private def statusName(status: StageStatus): String = status match {
case StageStatus.ACTIVE => "active"
case StageStatus.COMPLETE => "completed"
case StageStatus.FAILED => "failed"
case StageStatus.PENDING => "pending"
case StageStatus.SKIPPED => "skipped"
}

private def stageTag(status: StageStatus): String = s"${statusName(status)}Stage"

private def headerDescription(status: StageStatus): String = statusName(status).capitalize

private def summaryContent(appSummary: AppSummary, status: StageStatus, size: Int): String = {
if (status == StageStatus.COMPLETE && appSummary.numCompletedStages != size) {
s"${appSummary.numCompletedStages}, only showing $size"
} else {
s"$size"
}
if (shouldShowFailedStages) {
content ++=
<span id ="failed" class="collapse-aggregated-allFailedStages collapse-table"
onClick="collapseTable('collapse-aggregated-allFailedStages',
'aggregated-allFailedStages')">
<h4>
<span class="collapse-table-arrow arrow-open"></span>
<a>Failed Stages ({numFailedStages})</a>
</h4>
</span> ++
<div class="aggregated-allFailedStages collapsible-table">
{failedStagesTable.toNodeSeq}
</div>
}

private def summary(appSummary: AppSummary, status: StageStatus, size: Int): Elem = {
val summary =
<li>
<a href={s"#${statusName(status)}"}>
<strong>{headerDescription(status)} Stages:</strong>
</a>
{summaryContent(appSummary, status, size)}
</li>

if (status == StageStatus.COMPLETE) {
summary % Attribute(None, "id", Text("completed-summary"), Null)
} else {
summary
}
UIUtils.headerSparkPage("Stages for All Jobs", content, parent)
}

private def table(
appSummary: AppSummary,
status: StageStatus,
stagesTable: StageTableBase,
size: Int): NodeSeq = {
val classSuffix = s"${statusName(status).capitalize}Stages"
<span id={statusName(status)}
class={s"collapse-aggregated-all$classSuffix collapse-table"}
onClick={s"collapseTable('collapse-aggregated-all$classSuffix'," +
s" 'aggregated-all$classSuffix')"}>
<h4>
<span class="collapse-table-arrow arrow-open"></span>
<a>{headerDescription(status)} Stages ({summaryContent(appSummary, status, size)})</a>
</h4>
</span> ++
<div class={s"aggregated-all$classSuffix collapsible-table"}>
{stagesTable.toNodeSeq}
</div>
}
}