-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-10652][SPARK-10742][Streaming] Set meaningful job descriptions for all streaming jobs #8791
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@zsxwing Can you take a look at the changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
else is missing.
|
Test build #42562 has finished for PR 8791 at commit
|
|
Test build #42573 has finished for PR 8791 at commit
|
|
@zsxwing I addressed comments and made another update. Now the job description supports having links in them, and that is used to link to the batch details page. |
|
Test build #42574 has finished for PR 8791 at commit
|
|
Test build #42592 has finished for PR 8791 at commit
|
|
Test build #42599 has finished for PR 8791 at commit
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks it's better to add a new method like
private[streaming] def getStartSite: String = startSite.get()
rather than exposing AtomicReference. In addition, this one won't break MiMa tests.
|
LGTM except my comment about |
|
Test build #42659 has finished for PR 8791 at commit
|
|
This is blocked on #8781 . Once that is merged in both master and 1.5, I will finish this PR by adding unit tests. |
|
@andrewor14 Please take a look. |
|
Test build #42797 has finished for PR 8791 at commit
|
|
Test build #42815 has finished for PR 8791 at commit
|
|
Test build #42825 has finished for PR 8791 at commit
|
|
Test build #42826 has finished for PR 8791 at commit
|
|
retest this please |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think both /link and link are called relative links. But /link is called root-relative link. Maybe we should use Links in job descriptions must be root-relative:\n here?
|
Test build #42840 has finished for PR 8791 at commit
|
|
Test build #42849 has finished for PR 8791 at commit
|
|
@andrewor14 Could you take a look at this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is assuming the UI is the only consumer of this? I suppose that's OK since no one uses JobLogger anymore
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whoever uses it will get the data as text. Only the UI would actually process it as a link. Nothing breaks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like this caused a cosmetic issue in the tooltips though: #11845
EDIT: not quite tooltip, but the event timeline display. Also could have been subsequent to this change. Worth a look at the new issue in any event
|
core parts LGTM |
|
LGTM |
|
Thanks @andrewor14 @zsxwing, will merge this to master and 1.5 when tests pass. |
|
Test build #42878 has finished for PR 8791 at commit
|
…ns for all streaming jobs Here is the screenshot after adding the job descriptions to threads that run receivers and the scheduler thread running the batch jobs. ## All jobs page * Added job descriptions with links to relevant batch details page  ## All stages page * Added stage descriptions with links to relevant batch details page  ## Streaming batch details page * Added the +details link  Author: Tathagata Das <[email protected]> Closes #8791 from tdas/SPARK-10652. (cherry picked from commit 5548a25) Signed-off-by: Tathagata Das <[email protected]>
Here is the screenshot after adding the job descriptions to threads that run receivers and the scheduler thread running the batch jobs.
All jobs page
All stages page
Streaming batch details page