SPARK-5217 Spark UI should report pending stages during job execution on AllStagesPage. #4043

ScrapCodes · 2015-01-14T11:20:58Z

This is a first step towards having time remaining estimates for queued and running jobs. See SPARK-5216

SparkQA · 2015-01-14T11:22:39Z

Test build #25544 has started for PR 4043 at commit d335f8f.

This patch merges cleanly.

SparkQA · 2015-01-14T11:37:39Z

Test build #25545 has started for PR 4043 at commit f6dc584.

This patch merges cleanly.

SparkQA · 2015-01-14T12:31:20Z

Test build #25544 has finished for PR 4043 at commit d335f8f.

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

AmplabJenkins · 2015-01-14T12:31:24Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/25544/
Test PASSed.

SparkQA · 2015-01-14T12:46:34Z

Test build #25545 has finished for PR 4043 at commit f6dc584.

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

AmplabJenkins · 2015-01-14T12:46:38Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/25545/
Test PASSed.

squito · 2015-01-14T23:41:30Z

lgtm.

I was going to suggest that pending stages should be sorted with oldest submission time first, not reversed ... but I guess we want the completed stages sorted with oldest last, and probably makes sense to keep those tables consistent with each other.

ScrapCodes · 2015-01-16T06:09:14Z

Hi Imran, Thanks for taking a look. @pwendell please take a look ?

pwendell · 2015-01-16T06:38:50Z

I'm not sure this can be merged as-is. The state clean-up here is based on the assumption that every stage that is pending will at some later time be submitted. Is that definitely true? What happens if a stage is aborted or failed, won't its dependent stages remain pending indefinitely for the entire history of the application? I think at a minimum you need to make sure you remove all associated stages with a given job if the job ends. There may also be other corner cases I'm not thinking of.

A second assumption this makes is that the job start event will always occur before the stage is submitted. Is that definitely true? It would be good to dig through the reporting API and make sure that is a safe assumption. I think the guarantees of the listener around event ordering are pretty minimal.

Finally, what about putting pending stages after active stages in the display page? My concern is you may have dozens or more pending stages in production jobs, I think people will be frustrated if they open the UI and they have to scroll way down every time they need to e.g. refresh the page. It's a big odd to go (active -> pending -> completed), but I think better for usability.

ScrapCodes · 2015-01-16T07:14:48Z

Thanks Patrick !
Have two questions inline.

I'm not sure this can be merged as-is. The state clean-up here is based on the assumption that every stage that is pending will at some later time be submitted. Is that definitely true? What happens if a stage is aborted or failed, won't its dependent stages remain pending indefinitely for the entire history of the application? I think at a minimum you need to make sure you remove all associated stages with a given job if the job ends. There may also be other corner cases I'm not thinking of.

If I am understanding this correctly, "All stages for a particular job Id on jobEnd event - should be cleaned up". I am certainly missing something, is this not already achieved by this ?

A second assumption this makes is that the job start event will always occur before the stage is submitted. Is that definitely true? It would be good to dig through the reporting API and make sure that is a safe assumption. I think the guarantees of the listener around event ordering are pretty minimal.

This can lead to an anomaly that a stage will appear in progress in both active stages section and pending section. But it will still be obvious that - that stage is in progress. Is this safe to ignore, or is it a concern that should be addressed at the cost of minute complexity ?

Finally, what about putting pending stages after active stages in the display page? My concern is you may have dozens or more pending stages in production jobs, I think people will be frustrated if they open the UI and they have to scroll way down every time they need to e.g. refresh the page. It's a big odd to go (active -> pending -> completed), but I think better for usability.

This is thoughtful, I am going to incorporate this asap.

[EDIT]
One more thing to look at is ordering of stages since they are stored in a HashMap. Let me see if ordering can make sense somehow.

pwendell · 2015-01-16T07:52:24Z

Ah I see - so on the first point, the issue may be covered by the code you referenced. For some reason the diff originally rendered in a way where I didn't notice that.

For the second issue. My concern is that this line could throw an exception:
https://github.com/apache/spark/pull/4043/files#diff-1f32bcb61f51133bd0959a4177a066a5R263

I.e. if you had a stage submitted, but you never received the job started event. It could be that you can assume it, but it would be good to just note the ordering assumptions around the listener events on that line, so that if it gets changed later and someone gets a no-such-elemnt-exception. It's possible to debug it.

ScrapCodes · 2015-01-16T07:57:27Z

Good point, in map in scala remove returns an Option and does not throw exception. However in lists what you said holds. See https://github.com/scala/scala/blob/2.11.x/src/library/scala/collection/mutable/LinkedHashMap.scala#L76

SparkQA · 2015-01-16T08:02:35Z

Test build #25644 has started for PR 4043 at commit c7b3332.

This patch merges cleanly.

pwendell · 2015-01-16T08:03:34Z

@ScrapCodes Rather than change this to +LinkedHashMap+ can you just check if it contains it before removing it? It might not be obvious to developers that +remove+ has that specific behavior. I think it's better to just be explicit.

ScrapCodes · 2015-01-16T08:11:30Z

LinkedHashMap was introduced to maintain the insertion order of stagesIds(Hasmap will lead to arbitrary order), all map(s) in scala has return an Option on remove behavior. Even in java calling remove on inexistent key returns null. Do you still want me to change it? Also may be changing the name pendingStages to pendingStagesMap will avoid that confusion ? and should the same apply to - already existing field activeStages ?

pwendell · 2015-01-16T08:13:56Z

ah I see - if the existing remove call is safe, then I think it's fine.

ScrapCodes · 2015-01-16T08:43:18Z

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

      val now = System.currentTimeMillis

      val activeStagesTable =
        new StageTableBase(activeStages.sortBy(_.submissionTime).reverse,
          parent.basePath, parent.listener, isFairScheduler = parent.isFairScheduler,
          killEnabled = parent.killEnabled)
+      val pendingStagesTable =
+        new StageTableBase(pendingStages.sortBy(_.submissionTime),


@pwendell Can I use Sorting.stableSort here ?

Why not just keep the sorting the same as it was? I think the submission time is unlikely to be tied in most cases. It would be good to just make it consistent with the existing ones.

I was thinking about usability, like the most interesting stage which is next to be executed will appear at last. Anyway will change it.

SparkQA · 2015-01-16T09:16:34Z

Test build #25644 has finished for PR 4043 at commit c7b3332.

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

AmplabJenkins · 2015-01-16T09:16:38Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/25644/
Test PASSed.

pwendell · 2015-01-16T09:30:00Z

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

@@ -37,12 +37,18 @@ private[ui] class AllStagesPage(parent: StagesTab) extends WebUIPage("") {
      val numCompletedStages = listener.numCompletedStages
      val failedStages = listener.failedStages.reverse.toSeq
      val numFailedStages = listener.numFailedStages
+      val pendingStages = listener.pendingStages.values.toSeq


can you move this up with activeStages to make the declarations grouped properly?

SparkQA · 2015-01-16T11:22:38Z

Test build #25656 has started for PR 4043 at commit 15cdda4.

This patch merges cleanly.

SparkQA · 2015-01-16T12:33:04Z

Test build #25656 has finished for PR 4043 at commit 15cdda4.

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

AmplabJenkins · 2015-01-16T12:33:08Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/25656/
Test PASSed.

pwendell · 2015-01-18T03:35:01Z

@ScrapCodes mind bringing up to date? The current form LGTM

… on AllStagesPage.

…pleted->failed. And changed pending stages to not reverse sort.

SparkQA · 2015-01-19T07:07:43Z

Test build #25748 has started for PR 4043 at commit 3b11803.

This patch merges cleanly.

ScrapCodes · 2015-01-19T07:09:00Z

@pwendell - patch updated to latest master.

SparkQA · 2015-01-19T08:16:25Z

Test build #25748 has finished for PR 4043 at commit 3b11803.

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

AmplabJenkins · 2015-01-19T08:16:29Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/25748/
Test PASSed.

ScrapCodes force-pushed the SPARK-5216/5217-show-waiting-stages branch from d335f8f to f6dc584 Compare January 14, 2015 11:34

ScrapCodes reviewed Jan 16, 2015
View reviewed changes

pwendell reviewed Jan 16, 2015
View reviewed changes

ScrapCodes added 3 commits January 19, 2015 12:16

SPARK-5217 Spark UI should report pending stages during job execution…

c19d82a

… on AllStagesPage.

Switched to Linked hashmap, changed the order to active->pending->com…

0992842

…pleted->failed. And changed pending stages to not reverse sort.

Review feedback.

3b11803

ScrapCodes force-pushed the SPARK-5216/5217-show-waiting-stages branch from 15cdda4 to 3b11803 Compare January 19, 2015 07:06

asfgit closed this in 851b6a9 Jan 19, 2015

ScrapCodes deleted the SPARK-5216/5217-show-waiting-stages branch June 3, 2015 06:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SPARK-5217 Spark UI should report pending stages during job execution on AllStagesPage. #4043

SPARK-5217 Spark UI should report pending stages during job execution on AllStagesPage. #4043

ScrapCodes commented Jan 14, 2015

SparkQA commented Jan 14, 2015

SparkQA commented Jan 14, 2015

SparkQA commented Jan 14, 2015

AmplabJenkins commented Jan 14, 2015

SparkQA commented Jan 14, 2015

AmplabJenkins commented Jan 14, 2015

squito commented Jan 14, 2015

ScrapCodes commented Jan 16, 2015

pwendell commented Jan 16, 2015

ScrapCodes commented Jan 16, 2015

pwendell commented Jan 16, 2015

ScrapCodes commented Jan 16, 2015

SparkQA commented Jan 16, 2015

pwendell commented Jan 16, 2015

ScrapCodes commented Jan 16, 2015

pwendell commented Jan 16, 2015

ScrapCodes Jan 16, 2015

pwendell Jan 16, 2015

ScrapCodes Jan 16, 2015

SparkQA commented Jan 16, 2015

AmplabJenkins commented Jan 16, 2015

pwendell Jan 16, 2015

SparkQA commented Jan 16, 2015

SparkQA commented Jan 16, 2015

AmplabJenkins commented Jan 16, 2015

pwendell commented Jan 18, 2015

SparkQA commented Jan 19, 2015

ScrapCodes commented Jan 19, 2015

SparkQA commented Jan 19, 2015

AmplabJenkins commented Jan 19, 2015

SPARK-5217 Spark UI should report pending stages during job execution on AllStagesPage. #4043

SPARK-5217 Spark UI should report pending stages during job execution on AllStagesPage. #4043

Conversation

ScrapCodes commented Jan 14, 2015

SparkQA commented Jan 14, 2015

SparkQA commented Jan 14, 2015

SparkQA commented Jan 14, 2015

AmplabJenkins commented Jan 14, 2015

SparkQA commented Jan 14, 2015

AmplabJenkins commented Jan 14, 2015

squito commented Jan 14, 2015

ScrapCodes commented Jan 16, 2015

pwendell commented Jan 16, 2015

ScrapCodes commented Jan 16, 2015

pwendell commented Jan 16, 2015

ScrapCodes commented Jan 16, 2015

SparkQA commented Jan 16, 2015

pwendell commented Jan 16, 2015

ScrapCodes commented Jan 16, 2015

pwendell commented Jan 16, 2015

ScrapCodes Jan 16, 2015

Choose a reason for hiding this comment

pwendell Jan 16, 2015

Choose a reason for hiding this comment

ScrapCodes Jan 16, 2015

Choose a reason for hiding this comment

SparkQA commented Jan 16, 2015

AmplabJenkins commented Jan 16, 2015

pwendell Jan 16, 2015

Choose a reason for hiding this comment

SparkQA commented Jan 16, 2015

SparkQA commented Jan 16, 2015

AmplabJenkins commented Jan 16, 2015

pwendell commented Jan 18, 2015

SparkQA commented Jan 19, 2015

ScrapCodes commented Jan 19, 2015

SparkQA commented Jan 19, 2015

AmplabJenkins commented Jan 19, 2015