Skip to content

[SPARK-24851][UI] Map a Stage ID to it's Associated Job ID #21809

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
pgandhi999 wants to merge 9 commits into from
Closed

[SPARK-24851][UI] Map a Stage ID to it's Associated Job ID #21809

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
7be0520
[SPARK-24851] : Map a Stage ID to it's Associated Job ID in UI
Jul 18, 2018
3151a62
[SPARK-24851] : Using both stage id as well as the attempt id to fetc…
Jul 23, 2018
a50e8b1
[SPARK-24851] : Avoiding extra store lookup and adding href links
Jul 25, 2018
d57e6dc
[SPARK-24851] : Fixing Scalastyle Tests
Jul 25, 2018
3a06b87
[SPARK-24851] : Adding exception handling part to avoid NoSuchElement…
Jul 26, 2018
52b08b2
[SPARK-24851] : Addressing reviews July 27
Jul 27, 2018
9eff537
[SPARK-24851] : Addressing Reviews August 9
Aug 9, 2018
3b73840
[SPARK-24851] : Addressing reviews September 28, 2018
Sep 28, 2018
0be099a
[SPARK-24851] : Addressing Reviews October 5, 2018
Oct 5, 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
9 changes: 6 additions & 3 deletions core/src/main/scala/org/apache/spark/status/AppStatusStore.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,10 +112,13 @@ private[spark] class AppStatusStore(
}
}

def stageAttempt(stageId: Int, stageAttemptId: Int, details: Boolean = false): v1.StageData = {
def stageAttempt(stageId: Int, stageAttemptId: Int,

@gengliangwang gengliangwang Jul 27, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Changing the return type to (StageData, jobIds) might be simpler.

@pgandhi999 pgandhi999 Jul 27, 2018

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done

details: Boolean = false): (v1.StageData, Seq[Int]) = {

@tgravescs tgravescs Aug 2, 2018

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.

indent , you might make it look like taskSummary

@pgandhi999 pgandhi999 Aug 9, 2018

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done, thanks!

val stageKey = Array(stageId, stageAttemptId)
val stage = store.read(classOf[StageDataWrapper], stageKey).info
if (details) stageWithDetails(stage) else stage
val stageDataWrapper: StageDataWrapper = store.read(classOf[StageDataWrapper], stageKey)

@tgravescs tgravescs Aug 2, 2018

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.

you don't really need the type here (: StageDataWrapper)

@pgandhi999 pgandhi999 Aug 9, 2018

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done, thanks!

val stage = if (details) stageWithDetails(stageDataWrapper.info) else stageDataWrapper.info
val jobIds = stageDataWrapper.jobIds

@tgravescs tgravescs Oct 4, 2018

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.

instead of having separate val just put this in the return:

(stage, stageDataWrapper.jobIds.toSeq)

@pgandhi999 pgandhi999 Oct 5, 2018

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done

(stage, jobIds.toSeq)
}

def taskCount(stageId: Int, stageAttemptId: Int): Long = {
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/scala/org/apache/spark/status/api/v1/StagesResource.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ private[v1] class StagesResource extends BaseAppResource {
@PathParam("stageAttemptId") stageAttemptId: Int,
@QueryParam("details") @DefaultValue("true") details: Boolean): StageData = withUI { ui =>
try {
ui.store.stageAttempt(stageId, stageAttemptId, details = details)
ui.store.stageAttempt(stageId, stageAttemptId, details = details)._1
} catch {
case _: NoSuchElementException =>
// Change the message depending on whether there are any attempts for the requested stage.
Expand Down
35 changes: 29 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 @@ -18,7 +18,7 @@
package org.apache.spark.ui.jobs

import java.net.URLEncoder
import java.util.Date
import java.util.{Date, NoSuchElementException}
import java.util.concurrent.TimeUnit
import javax.servlet.http.HttpServletRequest

Expand Down Expand Up @@ -105,15 +105,29 @@ private[ui] class StagePage(parent: StagesTab, store: AppStatusStore) extends We
val stageAttemptId = parameterAttempt.toInt

val stageHeader = s"Details for Stage $stageId (Attempt $stageAttemptId)"
val stageData = parent.store
.asOption(parent.store.stageAttempt(stageId, stageAttemptId, details = false))
.getOrElse {
var stageDataTuple: Tuple2[StageData, Seq[Int]] = null
try {

@tgravescs tgravescs Aug 2, 2018

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.

I think you could use options here instead of try and null checks to make this cleaner

@pgandhi999 pgandhi999 Aug 9, 2018

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done, thanks!

stageDataTuple = parent.store.stageAttempt(stageId, stageAttemptId, details = false)
} catch {
case e: NoSuchElementException => e.getMessage
}
var stageData: StageData = null
var stageJobIds: Seq[Int] = null
if (stageDataTuple == null) {
stageData = {

@gengliangwang gengliangwang Jul 27, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this code branch is unreachable.

@pgandhi999 pgandhi999 Jul 27, 2018

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

It is unreachable by the IDE but during runtime, the code does work, I have confirmed that.

val content =
<div id="no-info">
<p>No information to display for Stage {stageId} (Attempt {stageAttemptId})</p>
<p>No information to display for Stage
{stageId}
(Attempt
{stageAttemptId})</p>
</div>
return UIUtils.headerSparkPage(request, stageHeader, content, parent)
}
} else {
stageData = stageDataTuple._1
stageJobIds = stageDataTuple._2
}

val localitySummary = store.localitySummary(stageData.stageId, stageData.attemptId)

Expand Down Expand Up @@ -182,6 +196,15 @@ private[ui] class StagePage(parent: StagesTab, store: AppStatusStore) extends We
{Utils.bytesToString(stageData.diskBytesSpilled)}
</li>
}}
{if (!stageJobIds.isEmpty) {

@abellina abellina Sep 12, 2018

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.

This could throw an NPE if stageDataTuple is None

scala> var x:Seq[Int] = null
x: Seq[Int] = null

scala> x.isEmpty
java.lang.NullPointerException

@pgandhi999 pgandhi999 Sep 28, 2018 •
edited
Loading

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Since, we are returning from the code if it is None, it should never reach there as far as I can tell.

<li>
<strong>Associated Job Ids: </strong>
{stageJobIds.map(jobId => {val detailUrl = "%s/jobs/job/?id=%s".format(
UIUtils.prependBaseUri(request, parent.basePath), jobId)
<a href={s"${detailUrl}"}>{s"${jobId}"} &nbsp;&nbsp;</a>
})}
</li>
}}
</ul>
</div>

Expand Down Expand Up @@ -1047,7 +1070,7 @@ private[ui] object ApiHelper {
}

def lastStageNameAndDescription(store: AppStatusStore, job: JobData): (String, String) = {
val stage = store.asOption(store.stageAttempt(job.stageIds.max, 0))
val stage = store.asOption(store.stageAttempt(job.stageIds.max, 0)._1)
(stage.map(_.name).getOrElse(""), stage.flatMap(_.description).getOrElse(job.name))
}

Expand Down