[SPARK-21503][UI]: Spark UI shows incorrect task status for a killed Executor Process #18707

pgandhi999 · 2017-07-21T21:22:17Z

The executor tab on Spark UI page shows task as completed when an executor process that is running that task is killed using the kill command.
Added the case ExecutorLostFailure which was previously not there, thus, the default case would be executed in which case, task would be marked as completed. This case will consider all those cases where executor connection to Spark Driver was lost due to killing the executor process, network connection etc.

How was this patch tested?

Manually Tested the fix by observing the UI change before and after.
Before:

After:

Please review http://spark.apache.org/contributing.html before opening a pull request.

Added the case ExecutorLostFailure which was previously not there, thus, the default case would be executed in which case, task would be marked as completed.

tgravescs · 2017-07-21T21:37:24Z

ok to test

zsxwing · 2017-07-21T22:58:42Z

core/src/main/scala/org/apache/spark/ui/exec/ExecutorsTab.scala

          return
        case _: ExceptionFailure =>
          taskSummary.tasksFailed += 1
+        case _: ExecutorLostFailure =>


Looks like we can use info.successful?

Hi,
I have replaced most of the task failed cases with info.successful. Tested it as well. Have also written unit test to simulate my issue. Thank you.

SparkQA · 2017-07-22T00:39:17Z

Test build #79850 has finished for PR 18707 at commit 172fc20.

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

SparkQA · 2017-07-25T17:56:41Z

Test build #79937 has finished for PR 18707 at commit 81422e0.

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

Apache Spark Pull Request - July 26, 2017

…es not create SparkContext Added a flag to check whether user has initialized Spark Context. If it is true, then we let Application Master unregister with Resource Manager else we do not.

SparkQA · 2017-07-26T21:43:55Z

Test build #79977 has finished for PR 18707 at commit f454c89.

This patch fails Scala style tests.
This patch merges cleanly.
This patch adds no public classes.

… that does not create SparkContext" This reverts commit f454c89. "Merged another issue to this one by mistake"

SparkQA · 2017-07-27T00:26:41Z

Test build #79976 has finished for PR 18707 at commit 55c6c37.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
throw new AnalysisException(s\"UDF class $className doesn't implement any UDF interface\")
throw new AnalysisException(s\"It is invalid to implement multiple UDF interfaces, UDF class $className\")
throw new AnalysisException(s\"UDF class with $n type arguments is not supported.\")
throw new AnalysisException(s\"Can not instantiate class $className, please make sure it has public non argument constructor\")
case e: ClassNotFoundException => throw new AnalysisException(s\"Can not load class $className, please make sure it is on the classpath\")

SparkQA · 2017-07-27T01:02:03Z

Test build #79978 has finished for PR 18707 at commit 6b7d5c6.

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

jiangxb1987 · 2017-07-28T07:39:59Z

core/src/main/scala/org/apache/spark/ui/exec/ExecutorsTab.scala

          // first attempt of this task. This may not be 100% accurate because the first attempt
          // could have failed half-way through. The correct fix would be to keep track of the
          // metrics added by each attempt, but this is much more complicated.
          return


nit:

if (taskEnd.reason == Resubmitted) { return ; }

I have made the required changes and verified them. Thank you.

jiangxb1987 · 2017-07-28T07:48:51Z

core/src/main/scala/org/apache/spark/ui/exec/ExecutorsTab.scala

+      if (info.successful) {
+        taskSummary.tasksComplete += 1
+      } else {
+        taskSummary.tasksFailed += 1


I'm not sure this keeps the previous behavior, cc @zsxwing

I went through the list of possible task failure reasons, and I found info.successful to be updated for all except Resubmitted case. If you think there is a possibility of any other reason that has been overlooked by me, do let me know.

Spark - July 28, 2017

[SPARK-21503]- Making Changes as per comments: Removed match case statement and replaced it with an if clause.

SparkQA · 2017-07-28T18:26:03Z

Test build #80026 has finished for PR 18707 at commit bc41664.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
class OneVsRestParams(HasFeaturesCol, HasLabelCol, HasWeightCol, HasPredictionCol):
class ArrowWriter(

SparkQA · 2017-07-28T19:10:51Z

Test build #80027 has finished for PR 18707 at commit e46126f.

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

umehrot2 · 2017-07-28T23:55:29Z

core/src/test/scala/org/apache/spark/ui/StagePageSuite.scala

        jobListener.onTaskEnd(
          SparkListenerTaskEnd(0, 0, "result", Success, taskInfo, taskMetrics))
    }
+    (3 to 4).foreach {


I don't see a test case which uses this method to test that the task is correctly displayed as failed. In that case, is this test suite modification needed ?

You are right. I did try to simulate a similar unit test for Executors Tab UI, but it needs creating a SparkUI object which cannot be accessed from StagePageSuite. Hence, I am reverting my unit test for the time being.

Spark - August 1, 2017

[SPARK-21503]: Reverting Unit Test Code - Not needed.

SparkQA · 2017-08-01T17:03:38Z

Test build #80119 has finished for PR 18707 at commit 9b3cebc.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
case class UnresolvedSubqueryColumnAliases(
sealed trait FrameType
sealed trait SpecialFrameBoundary extends Expression with Unevaluable
sealed trait WindowFrame extends Expression with Unevaluable

SparkQA · 2017-08-01T18:56:30Z

Test build #80122 has finished for PR 18707 at commit 7f03341.

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

jiangxb1987 · 2017-08-06T14:29:46Z

LGTM, cc @zsxwing @cloud-fan

cloud-fan · 2017-08-07T16:14:58Z

core/src/main/scala/org/apache/spark/ui/exec/ExecutorsTab.scala

+      if (taskEnd.reason == Resubmitted) {
+        return
+      }
+      if (info.successful) {


what's the relationship between info.successful and taskEnd.reason?

def successful: Boolean = finished && !failed && !killed

So if the task state shows finished and it has neither failed nor been killed, then info.successful is true. Now, there could be multiple reasons for a failed or killed task, and taskEnd.reason lists those. However, if task state is SUCCESS, so is the reason and thus, info.successful will be true in case of SUCCESS and false for all the other cases.

cloud-fan · 2017-08-07T17:04:30Z

core/src/main/scala/org/apache/spark/ui/exec/ExecutorsTab.scala

-          // could have failed half-way through. The correct fix would be to keep track of the
-          // metrics added by each attempt, but this is much more complicated.
-          return
-        case _: ExceptionFailure =>


can we fix the problem by changing this line to case _: TaskFailedReason?

So, I added info.successful check at the suggestion of @zsxwing and @jiangxb1987 . I feel this covers all the failed and killed cases which TaskFailedReason might have overlooked. However, if you feel otherwise, please let me know.

tgravescs · 2017-08-08T17:09:11Z

change LGTM

@cloud-fan I don't see any issues with the current approach, it was suggested earlier, but I think your suggestion would work also. Functionally I'm not seeing any difference. Did I miss something or are you ok with this way?

…xecutor Process The executor tab on Spark UI page shows task as completed when an executor process that is running that task is killed using the kill command. Added the case ExecutorLostFailure which was previously not there, thus, the default case would be executed in which case, task would be marked as completed. This case will consider all those cases where executor connection to Spark Driver was lost due to killing the executor process, network connection etc. ## How was this patch tested? Manually Tested the fix by observing the UI change before and after. Before: <img width="1398" alt="screen shot-before" src="https://user-images.githubusercontent.com/22228190/28482929-571c9cea-6e30-11e7-93dd-728de5cdea95.png"> After: <img width="1385" alt="screen shot-after" src="https://user-images.githubusercontent.com/22228190/28482964-8649f5ee-6e30-11e7-91bd-2eb2089c61cc.png"> Please review http://spark.apache.org/contributing.html before opening a pull request. Author: pgandhi <[email protected]> Author: pgandhi999 <[email protected]> Closes #18707 from pgandhi999/master. (cherry picked from commit f016f5c) Signed-off-by: Wenchen Fan <[email protected]>

cloud-fan · 2017-08-09T05:47:58Z

I was just asking about different fixes, I'm ok with the current fix.

thanks, merging to master/2.2!

…xecutor Process The executor tab on Spark UI page shows task as completed when an executor process that is running that task is killed using the kill command. Added the case ExecutorLostFailure which was previously not there, thus, the default case would be executed in which case, task would be marked as completed. This case will consider all those cases where executor connection to Spark Driver was lost due to killing the executor process, network connection etc. ## How was this patch tested? Manually Tested the fix by observing the UI change before and after. Before: <img width="1398" alt="screen shot-before" src="https://user-images.githubusercontent.com/22228190/28482929-571c9cea-6e30-11e7-93dd-728de5cdea95.png"> After: <img width="1385" alt="screen shot-after" src="https://user-images.githubusercontent.com/22228190/28482964-8649f5ee-6e30-11e7-91bd-2eb2089c61cc.png"> Please review http://spark.apache.org/contributing.html before opening a pull request. Author: pgandhi <[email protected]> Author: pgandhi999 <[email protected]> Closes apache#18707 from pgandhi999/master. (cherry picked from commit f016f5c) Signed-off-by: Wenchen Fan <[email protected]>

[SPARK-21503]: Fixed the issue

172fc20

Added the case ExecutorLostFailure which was previously not there, thus, the default case would be executed in which case, task would be marked as completed.

pgandhi999 changed the title ~~[SPARK-21503][UI]: Fixed the issue~~ [SPARK-21503][UI]: Spark UI shows incorrect task status for a killed Executor Process Jul 21, 2017

zsxwing reviewed Jul 21, 2017

View reviewed changes

[SPARK-21503][UI]: Adding changes as per comments

81422e0

pgandhi999 and others added 2 commits July 26, 2017 16:26

Merge pull request #1 from apache/master

55c6c37

Apache Spark Pull Request - July 26, 2017

[SPARK-21541]: Spark Logs show incorrect job status for a job that do…

f454c89

…es not create SparkContext Added a flag to check whether user has initialized Spark Context. If it is true, then we let Application Master unregister with Resource Manager else we do not.

Revert "[SPARK-21541]: Spark Logs show incorrect job status for a job…

6b7d5c6

… that does not create SparkContext" This reverts commit f454c89. "Merged another issue to this one by mistake"

jiangxb1987 reviewed Jul 28, 2017

View reviewed changes

pgandhi999 and others added 2 commits July 28, 2017 10:24

Merge pull request #2 from apache/master

bc41664

Spark - July 28, 2017

[SPARK-21503]- Making Changes as per comments

e46126f

[SPARK-21503]- Making Changes as per comments: Removed match case statement and replaced it with an if clause.

umehrot2 reviewed Jul 28, 2017

View reviewed changes

pgandhi999 and others added 2 commits August 1, 2017 08:58

Merge pull request #4 from apache/master

9b3cebc

Spark - August 1, 2017

[SPARK-21503]: Reverting Unit Test Code

7f03341

[SPARK-21503]: Reverting Unit Test Code - Not needed.

cloud-fan reviewed Aug 7, 2017

View reviewed changes

asfgit closed this in f016f5c Aug 9, 2017

[SPARK-21503][UI]: Spark UI shows incorrect task status for a killed Executor Process #18707

[SPARK-21503][UI]: Spark UI shows incorrect task status for a killed Executor Process #18707

Uh oh!

Conversation

pgandhi999 commented Jul 21, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

How was this patch tested?

Uh oh!

tgravescs commented Jul 21, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Jul 22, 2017

Uh oh!

SparkQA commented Jul 25, 2017

Uh oh!

SparkQA commented Jul 26, 2017

Uh oh!

SparkQA commented Jul 27, 2017

Uh oh!

SparkQA commented Jul 27, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Jul 28, 2017

Uh oh!

SparkQA commented Jul 28, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Aug 1, 2017

Uh oh!

SparkQA commented Aug 1, 2017

Uh oh!

jiangxb1987 commented Aug 6, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tgravescs commented Aug 8, 2017

Uh oh!

cloud-fan commented Aug 9, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

pgandhi999 commented Jul 21, 2017 •

edited

Loading