[SPARK-1825] Make Windows Spark client work fine with Linux YARN cluster

[tsudukim]

Modified environment strings and path separators to platform-independent style if possible.

[SparkQA]

Test build #25204 has started for PR 3943 at commit 3d03d35.

• This patch merges cleanly.

[SparkQA]

Test build #25204 has finished for PR 3943 at commit 3d03d35.

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

[AmplabJenkins]

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/25204/
Test PASSed.

[andrewor14]

Hey @tsudukim have you tested this on a real YARN cluster? Also, does this work across YARN versions, say 2.2 through 2.5?

[tsudukim]

thanks @andrewor14 for following this.

I have tested it on two YARN clusters: 2.3 and 2.5. Both has 1 master and 3 slaves.
From Linux client, it works fine on both clusters. From Windows client, it still doesn't work on 2.3 cluster but works fine on 2.5 cluster.

This patch is implemented as to work across YARN versions. When we apply this patch on old YARN clusters (2.2, 2.3), the behaviour doesn't change from now because just the function which the current code uses ($()) is called by reflection. Only when we apply this patch on new YARN clusters (2.4, 2.5), cross-plaform version of function ($$()) is called. But I think someone else should test this on their variety of clusters as well because it depends on the environment strongly.

[sarutak]

@tsudukim Recently, YARN module was refactored so could you rebase your change?

[tgravescs]

@tsudukim sorry for the delay in looking at this, could you rebase to latest master?

[vanzin]

This is probably not a big deal, since this is not in any hot path, but it seems wasteful to look for the right method every time this method is called instead of caching the method to be called. I'd do this:

private lazy val expandMethod = Try(classOf[Environment].getMethod("$$"))
  .getOrElse(classOf[Environment].getMethod("$"))

Then your method becomes just:

def expandEnvironment(environment: Environment): String = \
  expandMethod.invoke(environment).asInstanceOf[String]

[vanzin]

@tsudukim were you able to actually run the unit tests on Windows? I tried that before but failed...

[tsudukim]

@sarutak @tgravescs @vanzin Thank you for your comments!
Though I don't have enogh time in these several days, I'm going to do it until next week. Sorry for the delay.

[aniketbhatnagar]

@tsudukim Would you have time this week to look at this please? I was hoping for this fix to make it in 1.2.1!

[SparkQA]

Test build #26316 has started for PR 3943 at commit ec4b865.

• This patch merges cleanly.

[tsudukim]

@vanzin Actually, test for org.apache.spark.deploy.yarn.* fails in Windows even in master branch.
I just ignored only that error and checked some new errors are not caused by my patch. (of course new error was not reported)

@aniketbhatnagar Sorry for the delay. I managed to do it, but I don't think this patch goes into 1.2.1. 1.3 might be good.

[SparkQA]

Test build #26316 has finished for PR 3943 at commit ec4b865.

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

[AmplabJenkins]

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/26316/
Test PASSed.

[andrewor14]

you missed one Utils.isWindows

[andrewor14]

Ok LGTM aside from the minor comments I pointed out. I will fix these myself when I merge this into master. Thanks a lot @tsudukim