You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by jimjh <gi...@git.apache.org> on 2014/11/13 08:27:27 UTC
[GitHub] spark pull request: SPARK-2624 add datanucleus jars to the contain...
GitHub user jimjh opened a pull request:
https://github.com/apache/spark/pull/3238
SPARK-2624 add datanucleus jars to the container in yarn-cluster
If `spark-submit` finds the datanucleus jars, it adds them to the driver's classpath, but does not add it to the container.
This patch modifies the yarn deployment class to copy all `datanucleus-*` jars found in `[spark-home]/libs` to the container.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/jimjh/spark SPARK-2624
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/3238.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #3238
----
commit 84e6cbae2c7f11f33c55d089645fd949fb88f3c3
Author: Jim Lim <ji...@quixey.com>
Date: 2014-11-13T07:25:26Z
SPARK-2624 add datanucleus jars to the container in yarn-cluster
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-2624 add datanucleus jars to the contain...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/3238#issuecomment-64972493
Hey @jimjh I'll look at this later today. Just a heads up we are in the middle of the 1.2 release right now, and depending on how much behavior this PR adds we may not pull it in for the release.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-2624 add datanucleus jars to the contain...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3238#issuecomment-63437350
[Test build #23541 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/23541/consoleFull) for PR 3238 at commit [`d28d8e9`](https://github.com/apache/spark/commit/d28d8e94aeac977a6ff10fd04e8a1a9b10dbb44a).
* This patch **passes all tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-2624 add datanucleus jars to the contain...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/3238#discussion_r21121693
--- Diff: yarn/common/src/main/scala/org/apache/spark/deploy/yarn/ClientBase.scala ---
@@ -584,6 +624,19 @@ private[spark] object ClientBase extends Logging {
}
/**
+ * Find the user-defined provided jars directory if configured, or return SPARK_HOME/lib if not.
+ *
+ * This method first looks for $CONF_SPARK_DATANUCLEUS_DIR inside the SparkConf, then looks for
+ * Spark home inside the the SparkConf and the user environment.
+ */
+ private def sparkDatanucleusJars(conf: SparkConf): Option[String] = {
--- End diff --
more like `dataNucleusJarsDir` or `dataNucleusJarsLocation`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-2624 add datanucleus jars to the contain...
Posted by tgravescs <gi...@git.apache.org>.
Github user tgravescs commented on the pull request:
https://github.com/apache/spark/pull/3238#issuecomment-63476674
I'm referring to other jars that could be the same situation as datanucleus in the future. They should be included in the Uber jar for some reason or another can't be. If we run into another case like that then we would have to add yet another config. I would prefer this config to be generic so that we can just put a jar in there in the future if needed.
If we think this is a short term workaround then I'm fine leaving it with the current name, but can you also file a follow on jira to investigate a better solution?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-2624 add datanucleus jars to the contain...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the pull request:
https://github.com/apache/spark/pull/3238#issuecomment-64266666
LGTM. There's some code duplication here that is not really your fault, and we could try to clean up in a separate change.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-2624 add datanucleus jars to the contain...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/3238#discussion_r20377414
--- Diff: yarn/common/src/main/scala/org/apache/spark/deploy/yarn/ClientBase.scala ---
@@ -223,6 +224,29 @@ private[spark] trait ClientBase extends Logging {
}
}
}
+
+ /**
+ * Do the same for datanucleus jars, if they exist in spark home. Find all datanucleus-* jars,
+ * copy them to the remote fs, and add them to the class path.
+ */
+ val sparkHomeOpt = sparkConf.getOption("spark.home").orElse(sys.env.get("SPARK_HOME"))
+ for (sparkHome <- sparkHomeOpt) {
+ val libs = sparkHome + Path.SEPARATOR + "lib"
+ val jars = new File(libs).listFiles(new FilenameFilter() {
+ override def accept(dir: File, name: String) = name.startsWith("datanucleus-")
+ })
+ // copy to remote and add to classpath
+ jars.foreach { jar =>
--- End diff --
No, the jars aren't included because there's an interference with the metadata file. Even outside of Yarn we manually add them to the classpath in addition to the assembly jar.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-2624 add datanucleus jars to the contain...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/3238#discussion_r21121229
--- Diff: docs/running-on-yarn.md ---
@@ -132,6 +132,18 @@ Most of the configs are the same for Spark on YARN as for other deployment modes
The maximum number of threads to use in the application master for launching executor containers.
</td>
</tr>
+<tr>
+ <td><code>spark.yarn.datanucleus.dir</code></td>
+ <td>(none)</td>
+ <td>
+ The location of the DataNucleus jars, in case overriding the default location is desired.
+ By default, Spark on YARN will use the DataNucleus jars installed at
+ <code>{sparkHome}/lib</code>, but the jars can also be in a world-readable location on HDFS.
--- End diff --
I think elsewhere we use `$SPARK_HOME/lib`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-2624 add datanucleus jars to the contain...
Posted by tgravescs <gi...@git.apache.org>.
Github user tgravescs commented on the pull request:
https://github.com/apache/spark/pull/3238#issuecomment-63100910
Yeah it looks like its apache licensed: http://www.datanucleus.org/documentation/license.html
It would be much nicer if it could be in in the assembly jar. Otherwise we would have to add something similar to spark.yarn.jar but for other dependency jars. Or change it to ship to a tarball or something else like that.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-2624 add datanucleus jars to the contain...
Posted by tgravescs <gi...@git.apache.org>.
Github user tgravescs commented on the pull request:
https://github.com/apache/spark/pull/3238#issuecomment-62900827
Aren't the datanucleus jars only needed for the hive sql support? Where is this being handled in standalone mode?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-2624 add datanucleus jars to the contain...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3238#issuecomment-63430802
[Test build #23541 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/23541/consoleFull) for PR 3238 at commit [`d28d8e9`](https://github.com/apache/spark/commit/d28d8e94aeac977a6ff10fd04e8a1a9b10dbb44a).
* This patch merges cleanly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-2624 add datanucleus jars to the contain...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3238#issuecomment-64104012
[Test build #23752 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/23752/consoleFull) for PR 3238 at commit [`16df17c`](https://github.com/apache/spark/commit/16df17cbb3f8c9b33c064d81f1e982e8727085f8).
* This patch **passes all tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-2624 add datanucleus jars to the contain...
Posted by jimjh <gi...@git.apache.org>.
Github user jimjh commented on the pull request:
https://github.com/apache/spark/pull/3238#issuecomment-63230766
I am still inclined to go with (2), with a variant of what @vanzin is suggesting. (Yes, I am hoping to get this out for 1.2.0. Do we have a date?)
We could have a `spark.yarn.datanucleus.dir` config option. If this is not set, the launcher would look for datanucleus jars in `SPARK_HOME`; otherwise, it would look for datanucleus jars in `spark.yarn.datanucleus.dir`. This mirrors what is currently being done for `__spark__.jar` (see [here](https://github.com/apache/spark/blob/master/yarn/common/src/main/scala/org/apache/spark/deploy/yarn/ClientBase.scala#L573) and [here](https://github.com/apache/spark/blob/master/yarn/common/src/main/scala/org/apache/spark/deploy/yarn/ClientBase.scala#L172).)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-2624 add datanucleus jars to the contain...
Posted by tgravescs <gi...@git.apache.org>.
Github user tgravescs commented on the pull request:
https://github.com/apache/spark/pull/3238#issuecomment-65141005
@andrewor14 see my comments from 14 days ago about generalizing. If this isn't going into 1.2 I would much rather go back to investigating just having an Uber jar.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-2624 add datanucleus jars to the contain...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/3238#issuecomment-65360572
I just had a discussion with @marmbrus and I think these jars are actually pretty important. For this reason I think it would be best if we pull this into 1.2. The existing approach looks fine to me as an intermediate solution. @jimjh would you have a chance to address the comments soon? I think @pwendell is ready to cut another RC in the next day or two.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-2624 add datanucleus jars to the contain...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3238#issuecomment-64318717
[Test build #23818 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/23818/consoleFull) for PR 3238 at commit [`fe95125`](https://github.com/apache/spark/commit/fe95125733308187d8225f1f2b77fd0692a91aa8).
* This patch **passes all tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-2624 add datanucleus jars to the contain...
Posted by jimjh <gi...@git.apache.org>.
Github user jimjh commented on the pull request:
https://github.com/apache/spark/pull/3238#issuecomment-64312996
@vanzin Thanks for the review!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-2624 add datanucleus jars to the contain...
Posted by jimjh <gi...@git.apache.org>.
Github user jimjh commented on the pull request:
https://github.com/apache/spark/pull/3238#issuecomment-63085403
[datanucleus](http://www.datanucleus.org/documentation/usage.html) uses the Apache 2 license.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-2624 add datanucleus jars to the contain...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/3238#discussion_r20329191
--- Diff: yarn/common/src/main/scala/org/apache/spark/deploy/yarn/ClientBase.scala ---
@@ -223,6 +224,29 @@ private[spark] trait ClientBase extends Logging {
}
}
}
+
+ /**
+ * Do the same for datanucleus jars, if they exist in spark home. Find all datanucleus-* jars,
+ * copy them to the remote fs, and add them to the class path.
+ */
+ val sparkHomeOpt = sparkConf.getOption("spark.home").orElse(sys.env.get("SPARK_HOME"))
+ for (sparkHome <- sparkHomeOpt) {
+ val libs = sparkHome + Path.SEPARATOR + "lib"
+ val jars = new File(libs).listFiles(new FilenameFilter() {
+ override def accept(dir: File, name: String) = name.startsWith("datanucleus-")
+ })
+ // copy to remote and add to classpath
+ jars.foreach { jar =>
--- End diff --
Hmm... this feels a little sub-optimal to me. For all others jars, the user has the option of either uploading them, or making them available on HDFS to avoid the cost of uploading them as part of every job. Perhaps the same should be done here?
Also, I don't expect you to know the answer to this question, but why aren't these jars included in the assembly jar when the Hive profile is enabled? That seems like it would be the best solution for everybody and would make this change unnecessary...
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-2624 add datanucleus jars to the contain...
Posted by jimjh <gi...@git.apache.org>.
Github user jimjh commented on the pull request:
https://github.com/apache/spark/pull/3238#issuecomment-63502170
Created! https://issues.apache.org/jira/browse/SPARK-4474
I will try to find time to add docs and write a test for this today.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-2624 add datanucleus jars to the contain...
Posted by jimjh <gi...@git.apache.org>.
Github user jimjh commented on the pull request:
https://github.com/apache/spark/pull/3238#issuecomment-63086308
@andrewor14 yep I tested it on a YARN cluster on EMR. I instantiated and printed a class from the datanucleus package, using spark assemblies before and after the fix.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-2624 add datanucleus jars to the contain...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/3238#discussion_r21121780
--- Diff: yarn/common/src/main/scala/org/apache/spark/deploy/yarn/ClientBase.scala ---
@@ -551,6 +584,13 @@ private[spark] object ClientBase extends Logging {
// Internal config to propagate the location of the user's jar to the driver/executors
val CONF_SPARK_USER_JAR = "spark.yarn.user.jar"
+ // Location of the datanucleus jars
+ val CONF_SPARK_DATANUCLEUS_DIR = "spark.yarn.datanucleus.dir"
+
+ // Internal config to propagate the locations of datanucleus jars found to add to the
+ // classpath of the executors
--- End diff --
add comma-delimited
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-2624 add datanucleus jars to the contain...
Posted by jimjh <gi...@git.apache.org>.
Github user jimjh commented on the pull request:
https://github.com/apache/spark/pull/3238#issuecomment-62917939
Yes, going by the comments [here](https://github.com/apache/spark/blob/master/bin/compute-classpath.sh#L111), the datanucleus jars are used for Hive support.
What mechanism does spark-standalone use to distribute jars and manage classpaths?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-2624 add datanucleus jars to the contain...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3238#issuecomment-62977570
[Test build #23333 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/23333/consoleFull) for PR 3238 at commit [`84e6cba`](https://github.com/apache/spark/commit/84e6cbae2c7f11f33c55d089645fd949fb88f3c3).
* This patch merges cleanly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-2624 add datanucleus jars to the contain...
Posted by jimjh <gi...@git.apache.org>.
Github user jimjh commented on the pull request:
https://github.com/apache/spark/pull/3238#issuecomment-63087228
That was added in [this commit](https://github.com/apache/spark/commit/cf0a8f0204bb8acdaf441b03c924c278fef08e28).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-2624 add datanucleus jars to the contain...
Posted by jimjh <gi...@git.apache.org>.
Github user jimjh commented on the pull request:
https://github.com/apache/spark/pull/3238#issuecomment-63430856
Added `spark.yarn.datanucleus.dir`. If not specified, looks for datanucleus under `SPARK_HOME/lib`.
I tried using the option with an absolute path, a `file://` URI, and a `hdfs://` URI.
@tgravescs datanucleus might be a specific unique case, where we want to distribute the jars with spark _but_ not include it in the uber jar; for other jars, the user can just use the `--jars` option with `spark-submit`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-2624 add datanucleus jars to the contain...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3238#issuecomment-64101814
[Test build #23752 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/23752/consoleFull) for PR 3238 at commit [`16df17c`](https://github.com/apache/spark/commit/16df17cbb3f8c9b33c064d81f1e982e8727085f8).
* This patch merges cleanly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-2624 add datanucleus jars to the contain...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/3238#discussion_r20814231
--- Diff: yarn/common/src/main/scala/org/apache/spark/deploy/yarn/ClientBase.scala ---
@@ -684,6 +737,13 @@ private[spark] object ClientBase extends Logging {
addUserClasspath(args, sparkConf, env)
}
+ // Add datanucleus jars to classpath
+ for (entries <- sparkConf.getOption(CONF_SPARK_DATANUCLEUS_JARS)) {
--- End diff --
Ah, I see what you're doing. You're manually adding all the datanucleus jars to the classpath by propagating the config. I wonder if, for the sake of simplicity, piggybacking on `cachedSecondaryJarLinks` is good enough.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-2624 add datanucleus jars to the contain...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/3238#discussion_r20821470
--- Diff: yarn/common/src/main/scala/org/apache/spark/deploy/yarn/ClientBase.scala ---
@@ -684,6 +737,13 @@ private[spark] object ClientBase extends Logging {
addUserClasspath(args, sparkConf, env)
}
+ // Add datanucleus jars to classpath
+ for (entries <- sparkConf.getOption(CONF_SPARK_DATANUCLEUS_JARS)) {
--- End diff --
In fact, nevermind. These jars need to be in Spark's classpath, not the user's classpath, so `cachedSecondaryJarLinks` wouldn't work for this after my change goes in.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-2624 add datanucleus jars to the contain...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/3238#discussion_r21121480
--- Diff: yarn/common/src/main/scala/org/apache/spark/deploy/yarn/ClientBase.scala ---
@@ -584,6 +624,19 @@ private[spark] object ClientBase extends Logging {
}
/**
+ * Find the user-defined provided jars directory if configured, or return SPARK_HOME/lib if not.
+ *
+ * This method first looks for $CONF_SPARK_DATANUCLEUS_DIR inside the SparkConf, then looks for
+ * Spark home inside the the SparkConf and the user environment.
+ */
+ private def sparkDatanucleusJars(conf: SparkConf): Option[String] = {
--- End diff --
more like `sparkDatanucleusJarsLocation`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-2624 add datanucleus jars to the contain...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/3238#issuecomment-65360604
@jimjh Also did you have a chance to test this on a real yarn cluster?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-2624 add datanucleus jars to the contain...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/3238#discussion_r20813704
--- Diff: yarn/common/src/main/scala/org/apache/spark/deploy/yarn/ClientBase.scala ---
@@ -22,6 +22,7 @@ import java.net.{InetAddress, UnknownHostException, URI, URISyntaxException}
import scala.collection.JavaConversions._
import scala.collection.mutable.{HashMap, ListBuffer, Map}
import scala.util.{Try, Success, Failure}
+import java.io.{File, FilenameFilter}
--- End diff --
nit: import should be grouped with other `java.*` imports.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-2624 add datanucleus jars to the contain...
Posted by jimjh <gi...@git.apache.org>.
Github user jimjh commented on the pull request:
https://github.com/apache/spark/pull/3238#issuecomment-63109218
@andrewor14 what do you think?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-2624 add datanucleus jars to the contain...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/3238#issuecomment-64318723
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/23818/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-2624 add datanucleus jars to the contain...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3238#issuecomment-64154161
[Test build #23772 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/23772/consoleFull) for PR 3238 at commit [`6c31fe0`](https://github.com/apache/spark/commit/6c31fe0d42cce98542c316197e25f1697041820c).
* This patch merges cleanly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-2624 add datanucleus jars to the contain...
Posted by jimjh <gi...@git.apache.org>.
Github user jimjh commented on the pull request:
https://github.com/apache/spark/pull/3238#issuecomment-65913114
Yea I should have been more careful. I agree that we should figure out a proper solution.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-2624 add datanucleus jars to the contain...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/3238#issuecomment-62851973
Can one of the admins verify this patch?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-2624 add datanucleus jars to the contain...
Posted by jimjh <gi...@git.apache.org>.
Github user jimjh commented on the pull request:
https://github.com/apache/spark/pull/3238#issuecomment-64963171
@vanzin ping!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-2624 add datanucleus jars to the contain...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/3238#discussion_r21121349
--- Diff: yarn/common/src/main/scala/org/apache/spark/deploy/yarn/ClientBase.scala ---
@@ -223,10 +224,42 @@ private[spark] trait ClientBase extends Logging {
}
}
}
+
if (cachedSecondaryJarLinks.nonEmpty) {
sparkConf.set(CONF_SPARK_YARN_SECONDARY_JARS, cachedSecondaryJarLinks.mkString(","))
}
+ /**
+ * Do the same for datanucleus jars, if they exist in spark home. Find all datanucleus-* jars,
+ * copy them to the remote fs, and add them to the class path.
--- End diff --
We should elaborate on why we need to do this. Without context I would be totally confused why we need to treat these jars differently (and only in cluster mode)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-2624 add datanucleus jars to the contain...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/3238#issuecomment-63086439
Sorry, I must be mixing this up with Ganglia. I'm 0 for 2 recently.
Well I found this comment in `compute-classpath.sh`:
```
# When Hive support is needed, Datanucleus jars must be included on the classpath.
# Datanucleus jars do not work if only included in the uber jar as plugin.xml metadata is lost.
# Both sbt and maven will populate "lib_managed/jars/" with the datanucleus jars when Spark is
# built with Hive, so first check if the datanucleus jars exist, and then ensure the current Spark
# assembly is built for Hive, before actually populating the CLASSPATH with the jars.
# Note that this check order is faster (by up to half a second) in the case where Hive is not used.
```
Hm, I wonder if the underlying issue can be solved with the assembly plugin if it's just a question of preserving some `plugin.xml` file? I don't know the issue.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-2624 add datanucleus jars to the contain...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/3238#issuecomment-63122630
According to the comment in `compute-classpath.sh` we currently don't have a solution to include it in the assembly jar. We have two options (1) do some build magic to make it happen, or (2) hack around it for Yarn based on the existing solution, as we have done in this PR. I haven't looked into (1) that much, but it would require some exploration as to whether there is a good way to preserve the metadata. I think there has been a lot of discussion behind the existing solution, so maybe this alternative is less trivial. In particular, if this is targeted for 1.2, then I'm more inclined to just go with (2).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-2624 add datanucleus jars to the contain...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/3238#issuecomment-62988129
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/23333/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-2624 add datanucleus jars to the contain...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/3238#issuecomment-62976870
add to whitelist. Standalone mode requires all distributed jars to be visible to all workers, so there is no equivalent to the distributed cache used in Yarn.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-2624 add datanucleus jars to the contain...
Posted by tgravescs <gi...@git.apache.org>.
Github user tgravescs commented on the pull request:
https://github.com/apache/spark/pull/3238#issuecomment-63317118
I would be more inclined to make the config name not specific to datanucleus. That way if we have similar issue in the future we can just drop another jar in there.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-2624 add datanucleus jars to the contain...
Posted by jimjh <gi...@git.apache.org>.
Github user jimjh commented on a diff in the pull request:
https://github.com/apache/spark/pull/3238#discussion_r20367567
--- Diff: yarn/common/src/main/scala/org/apache/spark/deploy/yarn/ClientBase.scala ---
@@ -223,6 +224,29 @@ private[spark] trait ClientBase extends Logging {
}
}
}
+
+ /**
+ * Do the same for datanucleus jars, if they exist in spark home. Find all datanucleus-* jars,
+ * copy them to the remote fs, and add them to the class path.
+ */
+ val sparkHomeOpt = sparkConf.getOption("spark.home").orElse(sys.env.get("SPARK_HOME"))
+ for (sparkHome <- sparkHomeOpt) {
+ val libs = sparkHome + Path.SEPARATOR + "lib"
+ val jars = new File(libs).listFiles(new FilenameFilter() {
+ override def accept(dir: File, name: String) = name.startsWith("datanucleus-")
+ })
+ // copy to remote and add to classpath
+ jars.foreach { jar =>
--- End diff --
@vanzin Yea that would be an optimization. How would you suggest I allow that option?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-2624 add datanucleus jars to the contain...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3238#issuecomment-64157988
[Test build #23772 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/23772/consoleFull) for PR 3238 at commit [`6c31fe0`](https://github.com/apache/spark/commit/6c31fe0d42cce98542c316197e25f1697041820c).
* This patch **passes all tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-2624 add datanucleus jars to the contain...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/3238#issuecomment-64104013
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/23752/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-2624 add datanucleus jars to the contain...
Posted by tgravescs <gi...@git.apache.org>.
Github user tgravescs commented on a diff in the pull request:
https://github.com/apache/spark/pull/3238#discussion_r20507053
--- Diff: yarn/common/src/main/scala/org/apache/spark/deploy/yarn/ClientBase.scala ---
@@ -551,6 +574,9 @@ private[spark] object ClientBase extends Logging {
// Internal config to propagate the location of the user's jar to the driver/executors
val CONF_SPARK_USER_JAR = "spark.yarn.user.jar"
+ // Location of the datanucleus jars
+ val CONF_SPARK_DATANUCLEUS_DIR = "spark.yarn.datanucleus.dir"
--- End diff --
please document
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-2624 add datanucleus jars to the contain...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/3238#issuecomment-65751457
Hey @jimjh I reverted this patch after all. It seems risky to push this into 1.2 this late in the release cycle. Let's try to figure out a correct solution for 1.3.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-2624 add datanucleus jars to the contain...
Posted by jimjh <gi...@git.apache.org>.
Github user jimjh commented on the pull request:
https://github.com/apache/spark/pull/3238#issuecomment-64154021
Hey @andrewor14 I am ready. `spark.yarn.datanucleus.dir` can be used to specify the directory containing the datanucleus jars, and `spark.yarn.datanucleus.jars` (internal) is used to propagate the actual list of jars. Tested on a YARN cluster with `local:/`, `hdfs:/`, and `file:/`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-2624 add datanucleus jars to the contain...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/3238#discussion_r21121180
--- Diff: docs/running-on-yarn.md ---
@@ -132,6 +132,18 @@ Most of the configs are the same for Spark on YARN as for other deployment modes
The maximum number of threads to use in the application master for launching executor containers.
</td>
</tr>
+<tr>
+ <td><code>spark.yarn.datanucleus.dir</code></td>
+ <td>(none)</td>
+ <td>
+ The location of the DataNucleus jars, in case overriding the default location is desired.
--- End diff --
I would add a note that explains why this warrants its own config, something about how data nucleus jars cannot be packaged into the Spark assembly jar because of metadata conflicts.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-2624 add datanucleus jars to the contain...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the pull request:
https://github.com/apache/spark/pull/3238#issuecomment-65141491
I thought about making this a generic "add all the jars in this directory to the dist cache and to the app's classpath". This would make sense for regular application dependencies - all jars would be added to the app's classpath (similar to --jar). But the datanucleus jars are "special", they have to be in the main class path so that Spark classes pick them up.
I think making this generic is a little dangerous since we shouldn't be encouraging people to add things to Spark's classpath, and should encourage the use of things like "userClassPathFirst" once we're comfortable that it works properly.
Another thing about the datanucleus jars is that most people actually shouldn't need them; Hive's preferred way to connect to the metastore is through the metastore server, and that doesn't require these jars. But I don't know how Spark SQL is generally deployed these days, so maybe that doesn't apply here.
In light of these I'd rather keep this as specific to these jars as possible, with the added comment that Tom made of investigating whether it's possible to add these to the uber jar somehow. That would be the best solution.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-2624 add datanucleus jars to the contain...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/3238#discussion_r20813987
--- Diff: yarn/common/src/main/scala/org/apache/spark/deploy/yarn/ClientBase.scala ---
@@ -223,10 +224,42 @@ private[spark] trait ClientBase extends Logging {
}
}
}
+
if (cachedSecondaryJarLinks.nonEmpty) {
sparkConf.set(CONF_SPARK_YARN_SECONDARY_JARS, cachedSecondaryJarLinks.mkString(","))
}
+ /**
+ * Do the same for datanucleus jars, if they exist in spark home. Find all datanucleus-* jars,
+ * copy them to the remote fs, and add them to the class path.
+ */
+ for (libsDir <- sparkDatanucleusJars(sparkConf)) {
+ val libsURI = new URI(libsDir)
+ val jarLinks = ListBuffer.empty[String]
+ if (libsURI.getScheme != LOCAL_SCHEME) {
+ val localURI = getQualifiedLocalPath(libsURI).toUri()
+ val jars = FileSystem.get(localURI, hadoopConf).listFiles(new Path(localURI.getPath), false)
+ while (jars.hasNext) {
+ val jar = jars.next()
+ val name = jar.getPath.getName
+ if (name.startsWith("datanucleus-")) {
+ // copy to remote and add to classpath
+ val src = jar.getPath
+ val destPath = copyFileToRemote(dst, src, replication)
+ distCacheMgr.addResource(fs, hadoopConf, destPath,
+ localResources, LocalResourceType.FILE, name, statCache)
+ jarLinks += name
--- End diff --
Don't you need to add these jars to `cachedSecondaryJarLinks` like in the other cases?
This probably works now since, IIRC, the classpath has a wildcard `$PWD/*` entry in it, but I've removed that in https://github.com/apache/spark/pull/3233.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-2624 add datanucleus jars to the contain...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the pull request:
https://github.com/apache/spark/pull/3238#issuecomment-63130947
I wonder if using `XmlAppendingTransformer` in the shade plugin would help; it doesn't look like the different plugin.xml files actually conflict with one another, seems like you just need all of the content available. (There are some conflicts in the actual root `<plugin>` tag, which is the part that might not work.)
Anyway, that's probably for a different change, though, if it's desired.
As for the dist cache option, you could have a `spark.yarn.datanucleus.cache.dir` config option, and add all the jars in that directory to the dist cache instead of looking for them locally. The launcher could populate the directory if no jars are there, although that could result in races (multiple jobs launching at the same time trying to upload files).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-2624 add datanucleus jars to the contain...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/3238#issuecomment-65140058
Hey @jimjh the overall approach makes sense. However I think having a config that is explicitly only for data nucleus jars is a little too specific. Is it possible to generalize this in any way? Currently we already have `spark.yarn.secondary.jars`. IIUC the reason why we can't just use this directly is that this adds the jars to the user application's class path, not Spark's. I wonder if there's an easy way to generalize that. @vanzin any thoughts?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-2624 add datanucleus jars to the contain...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3238#issuecomment-64313146
[Test build #23818 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/23818/consoleFull) for PR 3238 at commit [`fe95125`](https://github.com/apache/spark/commit/fe95125733308187d8225f1f2b77fd0692a91aa8).
* This patch merges cleanly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-2624 add datanucleus jars to the contain...
Posted by jimjh <gi...@git.apache.org>.
Github user jimjh commented on the pull request:
https://github.com/apache/spark/pull/3238#issuecomment-64721913
Are we good to merge?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-2624 add datanucleus jars to the contain...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/3238#issuecomment-64157991
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/23772/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-2624 add datanucleus jars to the contain...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3238#issuecomment-62988124
[Test build #23333 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/23333/consoleFull) for PR 3238 at commit [`84e6cba`](https://github.com/apache/spark/commit/84e6cbae2c7f11f33c55d089645fd949fb88f3c3).
* This patch **passes all tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-2624 add datanucleus jars to the contain...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/3238#discussion_r20343870
--- Diff: yarn/common/src/main/scala/org/apache/spark/deploy/yarn/ClientBase.scala ---
@@ -223,6 +224,29 @@ private[spark] trait ClientBase extends Logging {
}
}
}
+
+ /**
+ * Do the same for datanucleus jars, if they exist in spark home. Find all datanucleus-* jars,
+ * copy them to the remote fs, and add them to the class path.
+ */
+ val sparkHomeOpt = sparkConf.getOption("spark.home").orElse(sys.env.get("SPARK_HOME"))
+ for (sparkHome <- sparkHomeOpt) {
+ val libs = sparkHome + Path.SEPARATOR + "lib"
+ val jars = new File(libs).listFiles(new FilenameFilter() {
+ override def accept(dir: File, name: String) = name.startsWith("datanucleus-")
+ })
+ // copy to remote and add to classpath
+ jars.foreach { jar =>
--- End diff --
Isn't it because of licensing? Datanucleus is
LGPL or similar?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-2624 add datanucleus jars to the contain...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/3238#issuecomment-63437361
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/23541/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org