Skip to content
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,8 @@ 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)
//remove the scheme from the url if it exists since Hadoop does not expect scheme
appMasterRequest.setTrackingUrl(uiAddress.replaceAll("^http(\\w)*://", ""))
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

resourceManager.registerApplicationMaster(appMasterRequest)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")) {
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Author

Choose a reason for hiding this comment

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

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

uiAddress = sc.ui.appUIAddress
}
registerAM(uiAddress, securityMgr)
try {
userThread.join()
} finally {
Expand Down