You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by JoshRosen <gi...@git.apache.org> on 2014/08/08 02:21:59 UTC
[GitHub] spark pull request: [SPARK-2913] Place our log4j.properties at the...
GitHub user JoshRosen opened a pull request:
https://github.com/apache/spark/pull/1844
[SPARK-2913] Place our log4j.properties at the head of the classpath.
This addresses SPARK-2913, an issue where changes to log4j.properties didn't take effect in our default Spark EC2 configuration because Hadoop's configuration directory would end up ahead of Spark's on the computed CLASSPATH.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/JoshRosen/spark SPARK-2913
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/1844.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 #1844
----
commit de5bf18a1978230f8c32b6a4c514ce4b592e8d50
Author: Josh Rosen <jo...@apache.org>
Date: 2014-08-08T00:19:22Z
[SPARK-2913] Place our log4j.properties at the head of the classpath.
----
---
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-2913] Place our log4j.properties at the...
Posted by JoshRosen <gi...@git.apache.org>.
Github user JoshRosen commented on the pull request:
https://github.com/apache/spark/pull/1844#issuecomment-52822418
I'm going to close this for now and revisit it later; there's some more complexity RE: the log4j.properties file that takes precedence when launching workers.
---
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-2913] Place our log4j.properties at the...
Posted by shivaram <gi...@git.apache.org>.
Github user shivaram commented on the pull request:
https://github.com/apache/spark/pull/1844#issuecomment-51643912
Agree about updating the documentation.
I don't know if application jars take precedence when you use SparkSubmit. On the executor side I think JVMs are always launched using `spark-class` which means that they should use `compute-classpath.sh` -- Also my guess is that since logging is initialized very early on, it might be too late if the application's log4j.properties is added to the class loader when doing `addJar` or something like that.
@aarondav @pwendell could probably throw more light on this.
---
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-2913] Place our log4j.properties at the...
Posted by JoshRosen <gi...@git.apache.org>.
Github user JoshRosen commented on the pull request:
https://github.com/apache/spark/pull/1844#issuecomment-51555625
Any thoughts on this approach of doing it in Spark vs spark-ec2? The current configuration docs just tell users "edit Spark's log4j.properties" without any classpath-ordering caveats. I think it will be really confusing to users if these instructions don't work out of the box in the most common configurations.
The only downside that I see here is that users might want to place some other log4j.properties further ahead on the classpath. In that case, they could just symlink the other file into Spark's log4j.properties location.
---
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-2913] Place our log4j.properties at the...
Posted by JoshRosen <gi...@git.apache.org>.
Github user JoshRosen closed the pull request at:
https://github.com/apache/spark/pull/1844
---
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-2913] Place our log4j.properties at the...
Posted by aarondav <gi...@git.apache.org>.
Github user aarondav commented on the pull request:
https://github.com/apache/spark/pull/1844#issuecomment-51553531
There was also this PR I made approximately forever-ago to accomplish a similar goal, but just for spark-ec2: https://github.com/mesos/spark-ec2/pull/56/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-2913] Place our log4j.properties at the...
Posted by shivaram <gi...@git.apache.org>.
Github user shivaram commented on the pull request:
https://github.com/apache/spark/pull/1844#issuecomment-51563228
One thing thats good to keep in mind that often applications written on top of Spark may also have their own log4j.properties and core-site.xml etc. So putting Spark's conf before everything else could make it hard to override for users.
Changing this in spark-ec2 however should be fine as that is a custom setup just for trying out Spark.
---
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-2913] Place our log4j.properties at the...
Posted by JoshRosen <gi...@git.apache.org>.
Github user JoshRosen commented on the pull request:
https://github.com/apache/spark/pull/1844#issuecomment-51624727
@shivaram Good points. We probably want Spark's log4j.properties to be used for interactive shells and when launching Spark standalone masters and workers, but I can see cases where the user might prefer if spark-submit used the log4j.properties file from their application JAR.
Even if we decide to only change the code in spark-ec2, I think we should still update the documentation to be clearer about possible sources of log4j.properties (and maybe explain how to debug these issues with `SPARK_PRINT_LAUNCH_COMMAND=1`).
I can look into this, but do you know whether the application JAR takes precedence over the other classpath entries so that its log4j.properties file will always be preferred?
---
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-2913] Place our log4j.properties at the...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/1844#issuecomment-51553054
QA results for PR 1844:<br>- This patch PASSES unit tests.<br>- This patch merges cleanly<br>- This patch adds no public classes<br><br>For more information see test ouptut:<br>https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18161/consoleFull
---
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-2913] Place our log4j.properties at the...
Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on the pull request:
https://github.com/apache/spark/pull/1844#issuecomment-51562742
I think it's reasonable to handle this specially in spark itself - @aarondav 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-2913] Place our log4j.properties at the...
Posted by sryza <gi...@git.apache.org>.
Github user sryza commented on the pull request:
https://github.com/apache/spark/pull/1844#issuecomment-51653564
On the executor side, framework jars come first unless spark.files.userClassPathFirst is set to true.
At least for Spark on YARN, executors are not launched with spark-class.
---
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-2913] Place our log4j.properties at the...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/1844#issuecomment-51550134
QA tests have started for PR 1844. This patch merges cleanly. <br>View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18161/consoleFull
---
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