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

smurakozi · 2018-01-19T12:54:40Z

What changes were proposed in this pull request?

The allJobs and the job pages attempt to use stage attempt and DAG visualization from the store, but for long running jobs they are not guaranteed to be retained, leading to exceptions when these pages are rendered.

To fix it store.lastStageAttempt(stageId) and store.operationGraphForJob(jobId) are wrapped in store.asOption and default values are used if the info is missing.

How was this patch tested?

Manual testing of the UI, also using the test command reported in SPARK-23121:

./bin/spark-submit --class org.apache.spark.examples.streaming.HdfsWordCount ./examples/jars/spark-examples_2.11-2.4.0-SNAPSHOT.jar /spark

Closes #20287

… a jobid

smurakozi · 2018-01-19T13:04:00Z

@guoxiaolongzte could you please check if this change fixes the issue you have observed?

smurakozi · 2018-01-19T13:29:43Z

cc @jiangxb1987, @srowen, @vanzin

smurakozi · 2018-01-19T13:33:38Z

core/src/main/scala/org/apache/spark/ui/jobs/AllJobsPage.scala

+      val (_, lastStageDescription) = lastStageNameAndDescription(store, job)
+      val displayJobDescription =
+        if (lastStageDescription.isEmpty) {
+          job.name


Using job.name instead of "" to behave more like the pre-2.3 version:
https://github.com/smurakozi/spark/blob/772e4648d95bda3353723337723543c741ea8476/core/src/main/scala/org/apache/spark/ui/jobs/AllJobsPage.scala#L70

srowen · 2018-01-19T14:15:05Z

Jenkins add to whitelist

SparkQA · 2018-01-19T17:36:27Z

Test build #86389 has finished for PR 20330 at commit 6525ef4.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

vanzin · 2018-01-19T17:43:32Z

core/src/main/scala/org/apache/spark/ui/jobs/AllJobsPage.scala

-        .getOrElse("")
+      val (_, lastStageDescription) = lastStageNameAndDescription(store, job)
+      val displayJobDescription =
+        if (lastStageDescription.isEmpty) {


nit: I generally prefer the opposite check.

if (data is good) do something with data else fallback to something else

vanzin · 2018-01-19T17:44:15Z

core/src/main/scala/org/apache/spark/ui/jobs/AllJobsPage.scala


-    val formattedJobDescription =
-      UIUtils.makeDescription(lastStageDescription, basePath, plainText = false)
+    val jobDescription = UIUtils.makeDescription(lastStageDescription, basePath, plainText = false)


No need to check for empty description here?

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)

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

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

vanzin · 2018-01-19T17:44:39Z

core/src/main/scala/org/apache/spark/ui/jobs/JobPage.scala

+    val operationGraphContent = store.asOption(store.operationGraphForJob(jobId)) match {
+      case Some(operationGraph) => UIUtils.showDagVizForJob(jobId, operationGraph)
+      case None =>
+              <div id="no-info">


Indentation is off.

vanzin · 2018-01-19T17:45:11Z

core/src/main/scala/org/apache/spark/ui/jobs/StagePage.scala

@@ -18,7 +18,7 @@
 package org.apache.spark.ui.jobs

 import java.net.URLEncoder
-import java.util.Date
+import java.util.{Collections, Date}


New import is unused?

vanzin · 2018-01-19T17:45:39Z

core/src/main/scala/org/apache/spark/ui/jobs/StagePage.scala

@@ -31,6 +31,7 @@ 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


Just use JobData since it's already imported?

SparkQA · 2018-01-19T18:12:40Z

Test build #4063 has finished for PR 20330 at commit 6525ef4.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2018-01-20T00:05:27Z

Test build #86399 has finished for PR 20330 at commit d5fdabb.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

vanzin · 2018-01-20T00:23:10Z

core/src/main/scala/org/apache/spark/ui/jobs/StagePage.scala

+    store.asOption(store.lastStageAttempt(job.stageIds.max)) match {
+      case Some(lastStageAttempt) =>
+        (lastStageAttempt.name, lastStageAttempt.description.getOrElse(job.name))
+      case None => ("", "")


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.

This would probably be simpler:

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

Fixed, thanks for catching.

vanzin · 2018-01-20T00:23:43Z

You could also add 'Closes #20287' to the PR description to close the other PR for the same bug automatically.

SparkQA · 2018-01-20T03:04:12Z

Test build #86404 has finished for PR 20330 at commit f19d3a1.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

jiangxb1987 · 2018-01-20T06:35:07Z

cc @gengliangwang

SparkQA · 2018-01-21T00:07:07Z

Test build #86417 has finished for PR 20330 at commit c733ac9.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

gengliangwang

LGTM. I check the other calls of lastStageAttempt and all looks OK.

vanzin · 2018-01-22T18:36:05Z

Merging to master / 2.3.

… streaming apps ## What changes were proposed in this pull request? The allJobs and the job pages attempt to use stage attempt and DAG visualization from the store, but for long running jobs they are not guaranteed to be retained, leading to exceptions when these pages are rendered. To fix it `store.lastStageAttempt(stageId)` and `store.operationGraphForJob(jobId)` are wrapped in `store.asOption` and default values are used if the info is missing. ## How was this patch tested? Manual testing of the UI, also using the test command reported in SPARK-23121: ./bin/spark-submit --class org.apache.spark.examples.streaming.HdfsWordCount ./examples/jars/spark-examples_2.11-2.4.0-SNAPSHOT.jar /spark Closes #20287 Author: Sandor Murakozi <[email protected]> Closes #20330 from smurakozi/SPARK-23121. (cherry picked from commit 446948a) Signed-off-by: Marcelo Vanzin <[email protected]>

smurakozi · 2018-01-23T07:34:03Z

Thanks for your help @vanzin, @gengliangwang, @jiangxb1987

smurakozi added 4 commits January 19, 2018 11:59

Fixed issue caused by the store cleaning up old stages

94d50b4

JobPage doesn't break if operationGraphForJob is not in the store for…

d60ae4f

… a jobid

lastStageNameAndDescription uses store.lastStageAttempt

832378d

Changed message in case of missing DAG visualization info

6525ef4

smurakozi commented Jan 19, 2018

View reviewed changes

vanzin reviewed Jan 19, 2018

View reviewed changes

smurakozi added 2 commits January 19, 2018 20:46

Minor changes requested by reviewer

d5fdabb

Using job.name if lastStageDescription is missing

f19d3a1

vanzin reviewed Jan 20, 2018

View reviewed changes

Fixed defaulting to job.name in lastStageNameAndDescription

c733ac9

gengliangwang approved these changes Jan 22, 2018

View reviewed changes

asfgit closed this in 446948a Jan 22, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

smurakozi commented Jan 19, 2018 •

edited

Loading

smurakozi commented Jan 19, 2018

smurakozi commented Jan 19, 2018

smurakozi Jan 19, 2018

srowen commented Jan 19, 2018

SparkQA commented Jan 19, 2018

vanzin Jan 19, 2018

vanzin Jan 19, 2018

smurakozi Jan 19, 2018

vanzin Jan 19, 2018

smurakozi Jan 19, 2018

vanzin Jan 19, 2018

vanzin Jan 19, 2018

vanzin Jan 19, 2018

SparkQA commented Jan 19, 2018

SparkQA commented Jan 20, 2018

vanzin Jan 20, 2018

vanzin Jan 20, 2018

smurakozi Jan 20, 2018

vanzin commented Jan 20, 2018

SparkQA commented Jan 20, 2018

jiangxb1987 commented Jan 20, 2018

SparkQA commented Jan 21, 2018

gengliangwang left a comment

vanzin commented Jan 22, 2018

smurakozi commented Jan 23, 2018

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

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

Conversation

smurakozi commented Jan 19, 2018 • edited Loading

What changes were proposed in this pull request?

How was this patch tested?

smurakozi commented Jan 19, 2018

smurakozi commented Jan 19, 2018

Choose a reason for hiding this comment

srowen commented Jan 19, 2018

SparkQA commented Jan 19, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SparkQA commented Jan 19, 2018

SparkQA commented Jan 20, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vanzin commented Jan 20, 2018

SparkQA commented Jan 20, 2018

jiangxb1987 commented Jan 20, 2018

SparkQA commented Jan 21, 2018

gengliangwang left a comment

Choose a reason for hiding this comment

vanzin commented Jan 22, 2018

smurakozi commented Jan 23, 2018

smurakozi commented Jan 19, 2018 •

edited

Loading