Skip to content
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,12 @@ private[spark] class AppStatusStore(
}.toSeq
}

def getJobIdsAssociatedWithStage(stageId: Int, stageAttemptId: Int): Seq[Int] = {
val stageKey = Array(stageId, stageAttemptId)
val jobIds = store.read(classOf[StageDataWrapper], stageKey).jobIds.toSeq
jobIds
}

def lastStageAttempt(stageId: Int): v1.StageData = {
val it = store.view(classOf[StageDataWrapper])
.index("stageId")
Expand Down
8 changes: 8 additions & 0 deletions core/src/main/scala/org/apache/spark/ui/jobs/StagePage.scala
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,8 @@ private[ui] class StagePage(parent: StagesTab, store: AppStatusStore) extends We
return UIUtils.headerSparkPage(request, stageHeader, content, parent)
}

val stageJobIds = parent.store.getJobIdsAssociatedWithStage(stageId, stageAttemptId)
Copy link
Member

Choose a reason for hiding this comment

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

In https://github.com/apache/spark/blob/master/core/src/main/scala/org/apache/spark/ui/jobs/StagePage.scala#L109 there is a query for the stage data already. We can reduce the query to the store here.

Copy link
Member

Choose a reason for hiding this comment

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

E.g. we can add a function to return the whole StageDataWrapper in AppStatusStore

Copy link
Author

Choose a reason for hiding this comment

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

Makes sense, I have fixed it. Request you to have a look.


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

val totalTasks = taskCount(stageData)
Expand Down Expand Up @@ -182,6 +184,12 @@ private[ui] class StagePage(parent: StagesTab, store: AppStatusStore) extends We
{Utils.bytesToString(stageData.diskBytesSpilled)}
</li>
}}
{if (!stageJobIds.isEmpty) {
Copy link
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

Copy link
Author

@pgandhi999 pgandhi999 Sep 28, 2018

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}
Copy link
Member

Choose a reason for hiding this comment

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

make it href link?

Copy link
Author

Choose a reason for hiding this comment

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

The problem here is that the stage could also have multiple job ids, in that case, we get a bunch of them. Do you want a generic link instead that will take you to the jobs page? Let me know what you think.

Copy link
Member

Choose a reason for hiding this comment

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

My suggestion is to map each job id as a href link.

Copy link
Author

Choose a reason for hiding this comment

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

Did that and updated the screenshot. Thank you.

</li>
}}
</ul>
</div>

Expand Down