[SPARK-3286] - Cannot view ApplicationMaster UI when Yarn’s url scheme i... #2276

benoyantony · 2014-09-04T17:26:00Z

...s https

…e is https

AmplabJenkins · 2014-09-04T17:27:08Z

Can one of the admins verify this patch?

tgravescs · 2014-09-04T17:37:38Z

yarn/common/src/main/scala/org/apache/spark/deploy/yarn/ApplicationMaster.scala

@@ -188,7 +188,7 @@ private[spark] class ApplicationMaster(args: ApplicationMasterArguments,
    if (sc == null) {
      finish(FinalApplicationStatus.FAILED, "Timed out waiting for SparkContext.")
    } else {
-      registerAM(sc.ui.appUIHostPort)
+      registerAM(sc.ui.appUIAddress)


Pulling over my comment from the previous pr - #2206

This won't work with yarn alpha.

yes so unfortunately I went and verified that hadoop 0.23 can't have the scheme. It automatically adds the scheme itself (just supports http://).

This is already calling down into stable/alpha versions of YarnRMClientImpl.register. We could potentially pass whole uri, including scheme and then have the alpha version strip off the scheme.

Also you modified the call in runDriver to registerAM which covers client mode, I assume the same change needs to happen in runExecutorLauncher for yarn client mode.

Added code to strip off the scheme in alpha version.

Not sure if I need to modify runExecutorLauncher.

tgravescs · 2014-09-05T16:04:51Z

yarn/alpha/src/main/scala/org/apache/spark/deploy/yarn/YarnRMClientImpl.scala

@@ -96,7 +96,7 @@ private class YarnRMClientImpl(args: ApplicationMasterArguments) extends YarnRMC
    // Users can then monitor stderr/stdout on that node if required.
    appMasterRequest.setHost(Utils.localHostName())
    appMasterRequest.setRpcPort(0)
-    appMasterRequest.setTrackingUrl(uiAddress)
+    appMasterRequest.setTrackingUrl(uiAddress.replaceAll("^http(\\w)*://", ""))


I would rather this done with something more reliable like URI class and just removing the scheme if it has one.

Also can you add a comment about we are removing it because hadoop doesn't handle the scheme.

benoyantony · 2014-09-05T17:01:25Z

Sure. I'll do both.
Does Alpha corresponds to Hadoop versions before YARN-1203 ? As you know, before YARN-1203, we cannot pass AM URLS with scheme.

vanzin · 2014-09-05T17:06:53Z

No, alpha means pre-branch-2 hadoop (I think, Hadoop branching is not exactly an exact science). Anyway, there are stable releases without YARN-1203. So that probably should be handled.

If there isn't an API to figure out the Yarn version, I'd use reflection to detect a method that was added after YARN-1203 (preferrably around this API), and only apply the fix if the method is available.

…e is https

SparkQA · 2014-09-05T23:41:09Z

Can one of the admins verify this patch?

benoyantony · 2014-09-05T23:48:47Z

Thanks .
I have taken a different approach.
I check if the HTTPS is enabled for YARN by looking at yarn.http.policy. This is a new config parameter introduced after YARN-1203. If yarn.http.policy is HTTPS_ONLY, then appUIAddress is passed. This means that for most cases, we pass appUIHostPort as before.

With this implementation, I prefer not to use URI to remove the scheme from uiAddress for the alpha version of YarnRMClientImpl since there is a good chance of passing uiAddress without scheme and URI construction with throw exception.

tgravescs · 2014-09-09T14:33:20Z

yarn/common/src/main/scala/org/apache/spark/deploy/yarn/ApplicationMaster.scala

@@ -189,7 +189,11 @@ private[spark] class ApplicationMaster(args: ApplicationMasterArguments,
    if (sc == null) {
      finish(FinalApplicationStatus.FAILED, "Timed out waiting for SparkContext.")
    } else {
-      registerAM(sc.ui.appUIHostPort, securityMgr)
+        var uiAddress = sc.ui.appUIHostPort
+        if (yarnConf.get("yarn.http.policy").equals("HTTPS_ONLY")) {


I'm not sure I like this as much. The reason I prefer stripping it off in alpha is the yarn alpha support will be deprecated at some point (hopefully fairly soon) and I would rather have it be optimized for the yarn stable case which is what most people are using.

Per @vanzin , alpha means pre-branch-2 hadoop. Before YARN-1203(Hadoop 2.2), we cannot pass AM URLS with scheme. There are stable releases before YARN-1203. So for stable releases without YARN-1203, we have to pass host:port.

There are 4 cases:
Alpha and yarn.http.policy !=HTTPS_ONLY : uiAddress = host:port
Alpha and yarn.http.policy ==HTTPS_ONLY : uiAddress = scheme:host:port (strip off scheme since alpha will choke on scheme.)

stable and yarn.http.policy !=HTTPS_ONLY : uiAddress = host:port
stable and yarn.http.policy ==HTTPS_ONLY : uiAddress = scheme:host:port (we are taking a bet that yarn.http.policy==HTTPS_ONLY occurs only for stable release after Yarn-1203 since this property was introduced by Yarn-1277 . Both these changes are released in Hadoop-2.2. )

So I think, we need to do the conditional logic for both stable and alpha.
We also need to strip off the scheme for alpha.
Please let me know , if there is a better solution.

Actually the yarn versioning thing is: Alpha is branch 0.23 and 2.x up til 2.1.x .
stable is 2.2.x and beyond

see http://spark.apache.org/docs/latest/building-with-maven.html#specifying-the-hadoop-version

since YARN-1203 went into 2.2 its fine to only special case in alpha but do the easy thing for stable.

I'll defer to @tgravescs on Hadoop branch trivia; I see the point of having different logic for stable and alpha, although I'm ok with the current approach too.

Thanks @tgravescs .
So all stable versions are post YARN-1203 and can handle urls with scheme.
Alpha versions cannot handle url with scheme.
Address with scheme is passed from the ApplicationMaster.scala.
There are only two cases.
Stable : pass the address with scheme.
Alpha : strips of the scheme from the address.

I have changed the code appropriately

vanzin · 2014-09-09T20:37:45Z

LGTM. getAuthority() strips a little more than what I'd like, but I don't think the UI address ever has anything else anyway.

tgravescs · 2014-09-10T16:54:56Z

+1, looks good. Thanks @benoyantony

benoyantony · 2014-09-10T17:28:52Z

Thank you.
Could one of you please add me as "Contributor" so that I can assign this jira to me ?

tgravescs · 2014-09-10T17:36:50Z

@pwendell can you add @benoyantony as contributor in jira.

JoshRosen · 2014-09-10T18:58:39Z

This broke the Jenkins PRB build because it introduced a style error:

=========================================================================
Running Scala style checks
=========================================================================
Scalastyle checks failed at following occurrences:
error file=/home/jenkins/workspace/NewSparkPullRequestBuilder/yarn/alpha/src/main/scala/org/apache/spark/deploy/yarn/YarnRMClientImpl.scala message=insert.a.single.space.after.comment.start.and.before.end.message line=101 column=4
java.lang.RuntimeException: exists error
    at scala.sys.package$.error(package.scala:27)

I'll push a hotfix now.

tgravescs · 2014-09-10T19:10:08Z

Thanks @JoshRosen. Sorry about that I forgot to trigger jenkins again. I keep having issues with it not triggering I forgot to double check this one.

JoshRosen · 2014-09-10T19:13:20Z

@tgravescs Don't sweat it, Jenkins has been flaky lately.

I'm spending the entire day today ironing out Jenkins issues and building my own alternative to the Jenkins GitHub pull request builder plugin; follow https://github.com/databricks/spark-pr-dashboard/compare/pull-request-builder to track my progress.

[SPARK-3286] - Cannot view ApplicationMaster UI when Yarn’s url schem…

f060c02

…e is https

tgravescs reviewed Sep 4, 2014
View reviewed changes

benoyantony mentioned this pull request Sep 4, 2014

[SPARK-3286] - Cannot view ApplicationMaster UI when Yarn’s url scheme is ... #2206

Closed

tgravescs reviewed Sep 5, 2014
View reviewed changes

[SPARK-3286] - Cannot view ApplicationMaster UI when Yarn’s url schem…

450c536

…e is https

rebasing from master

92127c9

tgravescs reviewed Sep 9, 2014
View reviewed changes

Benoy Antony added 2 commits September 9, 2014 12:06

Use address with scheme, but Allpha version removes the scheme

e82f94e

Use address with scheme, but Allpha version removes the scheme

c3d51ee

asfgit closed this in 6f7a768 Sep 10, 2014

asfgit pushed a commit that referenced this pull request Sep 10, 2014

[HOTFIX] Fix scala style issue introduced by #2276.

26503fd

staple mentioned this pull request Sep 10, 2014

[SPARK-2781][SQL] Check resolution of LogicalPlans in Analyzer. #1706

Closed

WangTaoTheTonic mentioned this pull request Oct 27, 2014

[SPARK-4098][YARN]use appUIAddress instead of appUIHostPort in yarn-client mode #2958

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-3286] - Cannot view ApplicationMaster UI when Yarn’s url scheme i... #2276

[SPARK-3286] - Cannot view ApplicationMaster UI when Yarn’s url scheme i... #2276

benoyantony commented Sep 4, 2014

AmplabJenkins commented Sep 4, 2014

tgravescs Sep 4, 2014

benoyantony Sep 4, 2014

tgravescs Sep 5, 2014

benoyantony commented Sep 5, 2014

vanzin commented Sep 5, 2014

SparkQA commented Sep 5, 2014

benoyantony commented Sep 5, 2014

tgravescs Sep 9, 2014

benoyantony Sep 9, 2014

tgravescs Sep 9, 2014

vanzin Sep 9, 2014

benoyantony Sep 9, 2014

vanzin commented Sep 9, 2014

tgravescs commented Sep 10, 2014

benoyantony commented Sep 10, 2014

tgravescs commented Sep 10, 2014

JoshRosen commented Sep 10, 2014

tgravescs commented Sep 10, 2014

JoshRosen commented Sep 10, 2014

[SPARK-3286] - Cannot view ApplicationMaster UI when Yarn’s url scheme i... #2276

[SPARK-3286] - Cannot view ApplicationMaster UI when Yarn’s url scheme i... #2276

Conversation

benoyantony commented Sep 4, 2014

AmplabJenkins commented Sep 4, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

benoyantony commented Sep 5, 2014

vanzin commented Sep 5, 2014

SparkQA commented Sep 5, 2014

benoyantony commented Sep 5, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vanzin commented Sep 9, 2014

tgravescs commented Sep 10, 2014

benoyantony commented Sep 10, 2014

tgravescs commented Sep 10, 2014

JoshRosen commented Sep 10, 2014

tgravescs commented Sep 10, 2014

JoshRosen commented Sep 10, 2014