Skip to content

Conversation

@kayousterhout
Copy link
Contributor

It would be great to fix this for 1.3. since the fix is surgical and it helps understandability for users.

cc @shivaram @pwendell

@SparkQA
Copy link

SparkQA commented Mar 1, 2015

Test build #28136 has finished for PR 4839 at commit 84d617c.

  • This patch fails to build.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Mar 1, 2015

Test build #28141 has finished for PR 4839 at commit f346b49.

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

Copy link
Member

Choose a reason for hiding this comment

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

PS I noticed when using TaskInfo recently that it has gettingResult and finished property that does this same check. I used them instead of also comparing the times to 0.

@shivaram
Copy link
Contributor

shivaram commented Mar 1, 2015

I'm not familiar enough with the UI code, but one more problem I have noticed is that when the task is in the getting results stage, the UI shows the time as execution time and only after the task finishes do you see the getting results time. Is it possible to update the time waiting for results to be fetched while the task is in progress ?

@kayousterhout
Copy link
Contributor Author

@shivaram Fixed this, and @srowen thanks for pointing out the issue with the GET_RESULT status!! Shivaram, did you run this patch on your cluster? Or was your comment just based on the old code?

@shivaram
Copy link
Contributor

shivaram commented Mar 2, 2015

No it was based on the old code. Didn't get a chance to run this on the cluster yet.

@SparkQA
Copy link

SparkQA commented Mar 2, 2015

Test build #28155 has finished for PR 4839 at commit 3ab012c.

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

@kayousterhout
Copy link
Contributor Author

@shivaram @pwendell Does this look ok? Would be good to get into the next RC, just because this is pretty broken right now.

@shivaram
Copy link
Contributor

shivaram commented Mar 4, 2015

Functionality-wise this looks good to me.

@srowen
Copy link
Member

srowen commented Mar 6, 2015

Although I think this missed 1.3, as far as I'm concerned you can merge it into master, at least.

@andrewor14
Copy link
Contributor

LGTM I'm merging this into master 1.3 thanks

asfgit pushed a commit that referenced this pull request Mar 24, 2015
It would be great to fix this for 1.3. since the fix is surgical and it helps understandability for users.

cc shivaram pwendell

Author: Kay Ousterhout <[email protected]>

Closes #4839 from kayousterhout/SPARK-6088 and squashes the following commits:

3ab012c [Kay Ousterhout] Update getting result time incrementally, correctly set GET_RESULT status
f346b49 [Kay Ousterhout] Typos
748ea6b [Kay Ousterhout] Fixed build failure
84d617c [Kay Ousterhout] [SPARK-6088] Correct how tasks that get remote results are shown in the UI.

(cherry picked from commit 6948ab6)
Signed-off-by: Andrew Or <[email protected]>
@asfgit asfgit closed this in 6948ab6 Mar 24, 2015
@kayousterhout kayousterhout deleted the SPARK-6088 branch April 12, 2017 00:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants