Skip to content
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

[SPARK-23121][core] Fix for ui becoming unaccessible for long running streaming apps #20330

Closed
wants to merge 7 commits into from
Closed

[SPARK-23121][core] Fix for ui becoming unaccessible for long running streaming apps #20330

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
94d50b4
Fixed issue caused by the store cleaning up old stages
smurakozi Jan 19, 2018
d60ae4f
JobPage doesn't break if operationGraphForJob is not in the store for…
smurakozi Jan 19, 2018
832378d
lastStageNameAndDescription uses store.lastStageAttempt
smurakozi Jan 19, 2018
6525ef4
Changed message in case of missing DAG visualization info
smurakozi Jan 19, 2018
d5fdabb
Minor changes requested by reviewer
smurakozi Jan 19, 2018
f19d3a1
Using job.name if lastStageDescription is missing
smurakozi Jan 19, 2018
c733ac9
Fixed defaulting to job.name in lastStageNameAndDescription
smurakozi Jan 20, 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
24 changes: 13 additions & 11 deletions core/src/main/scala/org/apache/spark/ui/jobs/AllJobsPage.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ import org.apache.spark.util.Utils

/** Page showing list of all ongoing and recently finished jobs */
private[ui] class AllJobsPage(parent: JobsTab, store: AppStatusStore) extends WebUIPage("") {

import ApiHelper._

private val JOBS_LEGEND =
<div class="legend-area"><svg width="150px" height="85px">
<rect class="succeeded-job-legend"
Expand Down Expand Up @@ -65,10 +68,9 @@ private[ui] class AllJobsPage(parent: JobsTab, store: AppStatusStore) extends We
}.map { job =>
val jobId = job.jobId
val status = job.status
val jobDescription = store.lastStageAttempt(job.stageIds.max).description
val displayJobDescription = jobDescription
.map(UIUtils.makeDescription(_, "", plainText = true).text)
.getOrElse("")
val (_, lastStageDescription) = lastStageNameAndDescription(store, job)
val jobDescription = UIUtils.makeDescription(lastStageDescription, "", plainText = true).text

val submissionTime = job.submissionTime.get.getTime()
val completionTime = job.completionTime.map(_.getTime()).getOrElse(System.currentTimeMillis())
val classNameByStatus = status match {
Expand All @@ -80,7 +82,7 @@ private[ui] class AllJobsPage(parent: JobsTab, store: AppStatusStore) extends We

// The timeline library treats contents as HTML, so we have to escape them. We need to add
// extra layers of escaping in order to embed this in a Javascript string literal.
val escapedDesc = Utility.escape(displayJobDescription)
val escapedDesc = Utility.escape(jobDescription)
val jsEscapedDesc = StringEscapeUtils.escapeEcmaScript(escapedDesc)
val jobEventJsonAsStr =
s"""
Expand Down Expand Up @@ -403,6 +405,8 @@ private[ui] class JobDataSource(
sortColumn: String,
desc: Boolean) extends PagedDataSource[JobTableRowData](pageSize) {

import ApiHelper._

// Convert JobUIData to JobTableRowData which contains the final contents to show in the table
// so that we can avoid creating duplicate contents during sorting the data
private val data = jobs.map(jobRow).sorted(ordering(sortColumn, desc))
Expand All @@ -427,23 +431,21 @@ private[ui] class JobDataSource(
val formattedDuration = duration.map(d => UIUtils.formatDuration(d)).getOrElse("Unknown")
val submissionTime = jobData.submissionTime
val formattedSubmissionTime = submissionTime.map(UIUtils.formatDate).getOrElse("Unknown")
val lastStageAttempt = store.lastStageAttempt(jobData.stageIds.max)
val lastStageDescription = lastStageAttempt.description.getOrElse("")
val (lastStageName, lastStageDescription) = lastStageNameAndDescription(store, jobData)

val formattedJobDescription =
UIUtils.makeDescription(lastStageDescription, basePath, plainText = false)
val jobDescription = UIUtils.makeDescription(lastStageDescription, basePath, plainText = false)
Copy link
Contributor

Choose a reason for hiding this comment

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

No need to check for empty description here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

lastStageDescription may be empty, but it will not cause problems, makeDescription will handle it properly, just like in the version before lastStageAttempt was used:

  val jobDescription = UIUtils.makeDescription(jobData.description.getOrElse(""), 
  basePath, plainText = false)

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure, but don't you want the same behavior as above here (falling back to the job name)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've moved this logic to lastStageNameAndDescription, so it's uniform.


val detailUrl = "%s/jobs/job?id=%s".format(basePath, jobData.jobId)

new JobTableRowData(
jobData,
lastStageAttempt.name,
lastStageName,
lastStageDescription,
duration.getOrElse(-1),
formattedDuration,
submissionTime.map(_.getTime()).getOrElse(-1L),
formattedSubmissionTime,
formattedJobDescription,
jobDescription,
detailUrl
)
}
Expand Down
10 changes: 8 additions & 2 deletions core/src/main/scala/org/apache/spark/ui/jobs/JobPage.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -336,8 +336,14 @@ private[ui] class JobPage(parent: JobsTab, store: AppStatusStore) extends WebUIP
content ++= makeTimeline(activeStages ++ completedStages ++ failedStages,
store.executorList(false), appStartTime)

content ++= UIUtils.showDagVizForJob(
jobId, store.operationGraphForJob(jobId))
val operationGraphContent = store.asOption(store.operationGraphForJob(jobId)) match {
case Some(operationGraph) => UIUtils.showDagVizForJob(jobId, operationGraph)
case None =>
<div id="no-info">
<p>No DAG visualization information to display for job {jobId}</p>
</div>
}
content ++= operationGraphContent

if (shouldShowActiveStages) {
content ++= <h4 id="active">Active Stages ({activeStages.size})</h4> ++
Expand Down
12 changes: 9 additions & 3 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 @@ -23,12 +23,10 @@ import java.util.concurrent.TimeUnit
import javax.servlet.http.HttpServletRequest

import scala.collection.mutable.{HashMap, HashSet}
import scala.xml.{Elem, Node, Unparsed}
import scala.xml.{Node, Unparsed}

import org.apache.commons.lang3.StringEscapeUtils

import org.apache.spark.SparkConf
import org.apache.spark.internal.config._
import org.apache.spark.scheduler.TaskLocality
import org.apache.spark.status._
import org.apache.spark.status.api.v1._
Expand Down Expand Up @@ -1002,4 +1000,12 @@ private object ApiHelper {
}
}

def lastStageNameAndDescription(store: AppStatusStore, job: JobData): (String, String) = {
store.asOption(store.lastStageAttempt(job.stageIds.max)) match {
case Some(lastStageAttempt) =>
(lastStageAttempt.name, lastStageAttempt.description.getOrElse(job.name))
case None => ("", "")
Copy link
Contributor

Choose a reason for hiding this comment

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

Before, you were doing if (lastStageDescription.isEmpty) job.name else blah at the call site.

Now, when the last stage is not in the store, the call site is getting an empty string as the description, instead of using the job name.

Copy link
Contributor

Choose a reason for hiding this comment

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

This would probably be simpler:

val stage = store.asOption(...)
(stage.map(_.name).getOrElse(""), stage.map(_.description.getOrElse(job.name)))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed, thanks for catching.

}
}

}