-
Notifications
You must be signed in to change notification settings - Fork 28.9k
[SPARK-10812][YARN] Spark hadoop util support switching to yarn #8911
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
[SPARK-10812][YARN] Spark hadoop util support switching to yarn #8911
Conversation
|
Test build #42993 has finished for PR 8911 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.
LGTM. Only trivial things to suggest like a space after 'try'
|
Test build #43031 has started for PR 8911 at commit |
|
There's code in different places that set Other than that, looks sane. This is another piece of code that will need some serious thought when trying to fix the "Spark doesn't allow multiple contexts" issue, though. |
|
@vanzin so in my own code (where I do try and switch between yarn and non yarn mode) I clear the SPARK_YARN_MODE as done in the test. I could update SparkContext to explicitly clear SPARK_YARN_MODE if its being launched with a non yarn mode client if you think that would be helpful for people? |
|
Yeah, having the Spark code clean up after itself is easier because it means people don't have to remember to do it, and it doesn't need to be documented. |
|
Makes sense, do you think I should put that change in the SparkContext (on startup of non-yarn client or stop of any client) or in the yarnclient stop code? |
|
I did a cursory lookup for where it is set, and I think the places that need to be changed are Doing it in |
|
@vanzin so it seems like if I do it in SparkContext shutdown that should be sufficient for all cases? |
|
I don't think so. |
|
ah that makes sense, I guess I forgot the Client was a public API. |
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.
To be super paranoid, I'd do this before the previous line.
|
LGTM aside from two minor things. |
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.
While we're here, comparing ...getClass === classOf[...] would be better IMO. Fewer magic strings.
|
Test build #43035 has finished for PR 8911 at commit
|
|
@vanzin updated with the suggested changes :) |
|
LGTM, merging to master. Thanks! |
While this is likely not a huge issue for real production systems, for test systems which may setup a Spark Context and tear it down and stand up a Spark Context with a different master (e.g. some local mode & some yarn mode) tests this cane be an issue. Discovered during work on spark-testing-base on Spark 1.4.1, but seems like the logic that triggers it is present in master (see SparkHadoopUtil object). A valid work around for users encountering this issue is to fork a different JVM, however this can be heavy weight. ``` [info] SampleMiniClusterTest: [info] Exception encountered when attempting to run a suite with class name: com.holdenkarau.spark.testing.SampleMiniClusterTest *** ABORTED *** [info] java.lang.ClassCastException: org.apache.spark.deploy.SparkHadoopUtil cannot be cast to org.apache.spark.deploy.yarn.YarnSparkHadoopUtil [info] at org.apache.spark.deploy.yarn.YarnSparkHadoopUtil$.get(YarnSparkHadoopUtil.scala:163) [info] at org.apache.spark.deploy.yarn.Client.prepareLocalResources(Client.scala:257) [info] at org.apache.spark.deploy.yarn.Client.createContainerLaunchContext(Client.scala:561) [info] at org.apache.spark.deploy.yarn.Client.submitApplication(Client.scala:115) [info] at org.apache.spark.scheduler.cluster.YarnClientSchedulerBackend.start(YarnClientSchedulerBackend.scala:57) [info] at org.apache.spark.scheduler.TaskSchedulerImpl.start(TaskSchedulerImpl.scala:141) [info] at org.apache.spark.SparkContext.<init>(SparkContext.scala:497) [info] at com.holdenkarau.spark.testing.SharedMiniCluster$class.setup(SharedMiniCluster.scala:186) [info] at com.holdenkarau.spark.testing.SampleMiniClusterTest.setup(SampleMiniClusterTest.scala:26) [info] at com.holdenkarau.spark.testing.SharedMiniCluster$class.beforeAll(SharedMiniCluster.scala:103) ``` Author: Holden Karau <[email protected]> Closes #8911 from holdenk/SPARK-10812-spark-hadoop-util-support-switching-to-yarn. (cherry picked from commit d8d50ed)
|
I'm running into this issue when running tests using pytest on pyspark with version 1.4.1. Is there a workaround I can use in pyspark in the meantime before we're able to upgrade to 1.5.2/1.6 to benefit from this fix? |
|
@stevenmanton that question probably belongs more on the user list - but I'd say maybe just don't use yarn mode for your tests. |
|
Thanks @holdenk. It ended up being a simple fix. I'll follow up with mailing list for any other questions. |
While this is likely not a huge issue for real production systems, for test systems which may setup a Spark Context and tear it down and stand up a Spark Context with a different master (e.g. some local mode & some yarn mode) tests this cane be an issue. Discovered during work on spark-testing-base on Spark 1.4.1, but seems like the logic that triggers it is present in master (see SparkHadoopUtil object). A valid work around for users encountering this issue is to fork a different JVM, however this can be heavy weight. ``` [info] SampleMiniClusterTest: [info] Exception encountered when attempting to run a suite with class name: com.holdenkarau.spark.testing.SampleMiniClusterTest *** ABORTED *** [info] java.lang.ClassCastException: org.apache.spark.deploy.SparkHadoopUtil cannot be cast to org.apache.spark.deploy.yarn.YarnSparkHadoopUtil [info] at org.apache.spark.deploy.yarn.YarnSparkHadoopUtil$.get(YarnSparkHadoopUtil.scala:163) [info] at org.apache.spark.deploy.yarn.Client.prepareLocalResources(Client.scala:257) [info] at org.apache.spark.deploy.yarn.Client.createContainerLaunchContext(Client.scala:561) [info] at org.apache.spark.deploy.yarn.Client.submitApplication(Client.scala:115) [info] at org.apache.spark.scheduler.cluster.YarnClientSchedulerBackend.start(YarnClientSchedulerBackend.scala:57) [info] at org.apache.spark.scheduler.TaskSchedulerImpl.start(TaskSchedulerImpl.scala:141) [info] at org.apache.spark.SparkContext.<init>(SparkContext.scala:497) [info] at com.holdenkarau.spark.testing.SharedMiniCluster$class.setup(SharedMiniCluster.scala:186) [info] at com.holdenkarau.spark.testing.SampleMiniClusterTest.setup(SampleMiniClusterTest.scala:26) [info] at com.holdenkarau.spark.testing.SharedMiniCluster$class.beforeAll(SharedMiniCluster.scala:103) ``` Author: Holden Karau <[email protected]> Closes apache#8911 from holdenk/SPARK-10812-spark-hadoop-util-support-switching-to-yarn.
While this is likely not a huge issue for real production systems, for test systems which may setup a Spark Context and tear it down and stand up a Spark Context with a different master (e.g. some local mode & some yarn mode) tests this cane be an issue. Discovered during work on spark-testing-base on Spark 1.4.1, but seems like the logic that triggers it is present in master (see SparkHadoopUtil object). A valid work around for users encountering this issue is to fork a different JVM, however this can be heavy weight.