SPARK-2150: Provide direct link to finished application UI in yarn resou...#1094
SPARK-2150: Provide direct link to finished application UI in yarn resou...#1094rahulsinghaliitd wants to merge 1 commit into
Conversation
|
Can one of the admins verify this patch? |
|
Thanks for filing this; this is something I had on my radar but hadn't gotten down to it yet. While I also want the feature, I think we need a better approach here. For one, I'm not a big fan of using the log directory name as the application's URL in the history server; it's not very discoverable, since it's generated internally by the event logger. Also, slightly unrelated, but I have upcoming changes that make those names even more ugly, so they're not a very user-friendly thing to use as a public-facing URL. I've actually been playing with this and have some changes that use the actual application ID, when it's available (such as in the case of Yarn), to serve the application from the history server. I think that's a better approach. But if some committer wants to check this in while I work on that, it would not cause (many) conflicts with my change. While we're here, you'd need to make a similar change in the yarn/alpha project, and also in yarn-client mode (ExecutorLauncher.scala in both projects). |
|
Hi @vanzin, I don't have any preference as to how the URL is formatted and I think that changing the URL can be a separate activity. I am hoping this can be committed so that we atleast have the functionality. Thanks for pointing out the changes, I had also realized the same and I am currently in the process of testing them. They should be available soon. |
|
This change looks good to me. |
|
@rahulsinghaliitd that bug is about linking to the running web ui of the driver in yarn-client mode. Slightly different than this one, which is about setting the link after the application is finished. |
|
@vanzin I was only referring to how the UI URL is passed around. I have used the longer way of passing it around using command line arguments whereas the other change uses spark conf by simply setting it as another property. |
|
@rahulsinghaliitd ah, good point. Passing as a SparkConf property should work now that I fixed some things in the yarn-cluster backend. |
|
Latest patch LGTM. |
|
(Aside from rebasing to fix the merge conflicts.) |
|
Am I supposed to resolve the conflicts or will they be resolved by the admin who merges this change? |
|
Generally we ask for the PR submitter to resolve conflicts. You just have to rebase your branch on top of master, resolve conflicts, and force push it into your repo, the PR will be updated automatically. |
|
Is someone working on this? or facing some problem? |
There was a problem hiding this comment.
I think it would be better to add a routine to the eventLogger to just give us the name of the directory rather then us splitting it and it possibly breaking in the future.
|
This PR conflicts with pr1112. I would like to put that one in first and then upmerge this. |
There was a problem hiding this comment.
If we are adding this we should also use it in this file to set the path.
|
OK, thank you for your reply. |
|
@rahulsinghaliitd can you please upmerge |
|
@tgravescs updated according to your comments and rebased to current HEAD of master branch. Thanks for following up on this PR. |
There was a problem hiding this comment.
I think it makes more sense to do this the other way. have the UI_PATH_PREFIX /history and add in the "/" wherever its needed.
|
@tgravescs updated according to your comments and rebased to current HEAD of master branch. |
There was a problem hiding this comment.
typo, need the other " in the default case.
There was a problem hiding this comment.
woah, thanks for catching that!
|
Jenkins, test this please |
|
QA tests have started for PR 1094. This patch merges cleanly. |
|
QA results for PR 1094: |
|
@rahulsinghaliitd sorry for this dragging on I forgot to kick jenkins. Looks like its complaining about scalastyle. Can you please update. /home/jenkins/workspace/SparkPullRequestBuilder@2/core/src/main/scala/org/apache/spark/deploy/history/FsHistoryProvider.scala message=File line length exceeds 100 characters line=172 |
|
Jenkins, this is okay to test |
…source manager UI Use the event logger directory to provide a direct link to finished application UI in yarn resourcemanager UI.
|
@tgravescs No problem at all. Updated patch. |
|
@tgravescs There does not seem to be a maven goal to check scalastyle, isn't this something we want? |
|
I've only used the sbt/sbt scalastyle target. I'm not sure if there is a reason it wasn't implemented in maven. you could file a jira for it and get comments from the folks who did it. |
|
Jenkins, test this please |
|
QA tests have started for PR 1094. This patch merges cleanly. |
|
QA results for PR 1094: |
|
+1 thanks @rahulsinghaliitd ! |
…sou... ...rce manager UI Use the event logger directory to provide a direct link to finished application UI in yarn resourcemanager UI. Author: Rahul Singhal <rahul.singhal@guavus.com> Closes apache#1094 from rahulsinghaliitd/SPARK-2150 and squashes the following commits: 95f230c [Rahul Singhal] SPARK-2150: Provide direct link to finished application UI in yarn resource manager UI
* [CARMEL-6305] Support to show partitions for Delta table * Fix unit tests
Co-authored-by: Egor Krivokon <>
7 participants
...rce manager UI
Use the event logger directory to provide a direct link to finished
application UI in yarn resourcemanager UI.