[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[tgravescs]

Github user tgravescs commented on the pull request:

https://github.com/apache/spark/pull/9118#issuecomment-149587427
  
@vundela  Does SPARK_LOG4J_CONF still work? 


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[vundela]

Github user vundela commented on a diff in the pull request:

https://github.com/apache/spark/pull/9118#discussion_r42503914
  
--- Diff: yarn/src/main/scala/org/apache/spark/deploy/yarn/Client.scala ---
@@ -497,6 +497,19 @@ private[spark] class Client(
*/
   private def createConfArchive(): File = {
 val hadoopConfFiles = new HashMap[String, File]()
+
+// Uploading $SPARK_CONF_DIR/log4j.properties file to the distributed 
cache to make sure that
+// the executors will use the latest configurations instead of the 
default values. This is
+// required when user changes log4j.properties directly to set the log 
configurations. If
+// configuration file is provided through --files then executors will 
be taking configurations
+// from --files instead of $SPARK_CONF_DIR/log4j.properties.
--- End diff --

Yes, thats correct. 


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[vundela]

Github user vundela commented on the pull request:

https://github.com/apache/spark/pull/9118#issuecomment-149588730
  
@tgravescs  Yes, it still works and user see a deprecated message.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[tgravescs]

Github user tgravescs commented on a diff in the pull request:

https://github.com/apache/spark/pull/9118#discussion_r42503542
  
--- Diff: yarn/src/main/scala/org/apache/spark/deploy/yarn/Client.scala ---
@@ -497,6 +497,19 @@ private[spark] class Client(
*/
   private def createConfArchive(): File = {
 val hadoopConfFiles = new HashMap[String, File]()
+
+// Uploading $SPARK_CONF_DIR/log4j.properties file to the distributed 
cache to make sure that
+// the executors will use the latest configurations instead of the 
default values. This is
+// required when user changes log4j.properties directly to set the log 
configurations. If
+// configuration file is provided through --files then executors will 
be taking configurations
+// from --files instead of $SPARK_CONF_DIR/log4j.properties.
--- End diff --

Why does --files work?  Is it just because it gets put on classpath first?


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[markgrover]

Github user markgrover commented on the pull request:

https://github.com/apache/spark/pull/9118#issuecomment-149644490
  
I have one more noob question. Let's say I have a 4 node hadoop/yarn 
cluster and a 5th node, I am driving a yarn-client app from. I use puppet to 
set up my log4j configuration and it's set up differently on the 4 cluster 
nodes, vs. the 5th driver node. After this change, would the 5th node's log4j 
configuration always override my log4j configuration on the 4 executor nodes? 
Would that come as a surprise to some users?


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/spark/pull/9118


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[markgrover]

Github user markgrover commented on the pull request:

https://github.com/apache/spark/pull/9118#issuecomment-149657635
  
Got it, thanks!


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[vanzin]

Github user vanzin commented on the pull request:

https://github.com/apache/spark/pull/9118#issuecomment-149651459
  
@markgrover you can't have that scenario on YARN because when running Spark 
through YARN there is no "cluster configuration". Everything is configured 
based on the gateway node's configuration. You don't even need Spark jars 
available in the cluster.

The change LGTM, I'll merge to master.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[tgravescs]

Github user tgravescs commented on the pull request:

https://github.com/apache/spark/pull/9118#issuecomment-149608519
  
changes lgtm.  @vanzin are you ok with these?


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/9118#issuecomment-149257321
  
Merged build finished. Test FAILed.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/9118#issuecomment-149259270
  
Merged build started.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/9118#issuecomment-149259241
  
 Merged build triggered.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/9118#issuecomment-149260451
  
**[Test build #43929 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43929/consoleFull)**
 for PR 9118 at commit 
[`187eb99`](https://github.com/apache/spark/commit/187eb99def45670285047c51ad8269a998fb8ed9).


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/9118#issuecomment-149257320
  
Merged build started.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/9118#issuecomment-149257322
  
Test FAILed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43928/
Test FAILed.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/9118#issuecomment-149257298
  
 Merged build triggered.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[tgravescs]

Github user tgravescs commented on the pull request:

https://github.com/apache/spark/pull/9118#issuecomment-149309256
  
This isn't really a but as it was kind of designed this way.  Originally 
you had to either set SPARK_LOG4J_CONF (which  does SPARK_LOG4J_CONF still work 
properly?)  or you had to upload it with the --files.  

I'm definitely fine with having this happen automatically but we need to 
update the documentation at 
http://spark.apache.org/docs/latest/running-on-yarn.html.  It talks about using 
the --files and setting the java options. 

I assume --files still works because it will show up on the classpath first 
because it looks like it will still upload the one from the conf dir?



---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[vundela]

Github user vundela commented on the pull request:

https://github.com/apache/spark/pull/9118#issuecomment-149328542
  
@tgravescs 

Thanks for the review. Yes, it make sense to update the documentation. 


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/9118#issuecomment-149377264
  
Merged build started.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/9118#issuecomment-149377247
  
 Merged build triggered.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/9118#issuecomment-149363556
  
 Merged build triggered.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/9118#issuecomment-149364898
  
**[Test build #43943 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43943/consoleFull)**
 for PR 9118 at commit 
[`39c75a2`](https://github.com/apache/spark/commit/39c75a2c73cc49dda6c2450d1be9b96b2d71023e).


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/9118#issuecomment-149367985
  
**[Test build #43943 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43943/consoleFull)**
 for PR 9118 at commit 
[`39c75a2`](https://github.com/apache/spark/commit/39c75a2c73cc49dda6c2450d1be9b96b2d71023e).
 * This patch **fails MiMa 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/9118#issuecomment-149381570
  
Merged build finished. 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[vanzin]

Github user vanzin commented on a diff in the pull request:

https://github.com/apache/spark/pull/9118#discussion_r42434651
  
--- Diff: yarn/src/main/scala/org/apache/spark/deploy/yarn/Client.scala ---
@@ -497,6 +497,20 @@ private[spark] class Client(
*/
   private def createConfArchive(): File = {
 val hadoopConfFiles = new HashMap[String, File]()
+
+// Uploading $SPARK_CONF_DIR/log4j.properties file to the distributed 
cache to make sure that
+// the executors will use the latest configurations instead of the 
default values. This is
+// required when user changes log4j.properties directly to set the log 
configurations. If
+// configuration file is provided through --files then executors will 
be taking configurations
+// from --files instead of $SPARK_CONF_DIR/log4j.properties.
+
Option(Utils.getContextOrSparkClassLoader.getResource("log4j.properties"))
+  .map(_.getPath).map(path => {
--- End diff --

Technically, you should only look at this URL if its protocol is `file`, 
otherwise even if the path matches a local file, it wouldn't be correct.

Also, style:

.map { path =>
  // code
}




---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[vanzin]

Github user vanzin commented on a diff in the pull request:

https://github.com/apache/spark/pull/9118#discussion_r42434676
  
--- Diff: yarn/src/main/scala/org/apache/spark/deploy/yarn/Client.scala ---
@@ -497,6 +497,20 @@ private[spark] class Client(
*/
   private def createConfArchive(): File = {
 val hadoopConfFiles = new HashMap[String, File]()
+
+// Uploading $SPARK_CONF_DIR/log4j.properties file to the distributed 
cache to make sure that
+// the executors will use the latest configurations instead of the 
default values. This is
+// required when user changes log4j.properties directly to set the log 
configurations. If
+// configuration file is provided through --files then executors will 
be taking configurations
+// from --files instead of $SPARK_CONF_DIR/log4j.properties.
+
Option(Utils.getContextOrSparkClassLoader.getResource("log4j.properties"))
+  .map(_.getPath).map(path => {
+val file = new File(path)
+if(file.isFile && file.canRead) {
--- End diff --

style: `if (...) {`


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/9118#issuecomment-149377726
  
**[Test build #43952 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43952/consoleFull)**
 for PR 9118 at commit 
[`e1e474a`](https://github.com/apache/spark/commit/e1e474aff26d2b2b7ba117885f96493e9cb724a0).


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/9118#issuecomment-149363592
  
Merged build started.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/9118#issuecomment-149368037
  
Test FAILed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43943/
Test FAILed.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/9118#issuecomment-149368032
  
Merged build finished. Test FAILed.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/9118#issuecomment-149381336
  
**[Test build #43952 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43952/consoleFull)**
 for PR 9118 at commit 
[`e1e474a`](https://github.com/apache/spark/commit/e1e474aff26d2b2b7ba117885f96493e9cb724a0).
 * 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/9118#issuecomment-149381572
  
Test PASSed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43952/
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/9118#issuecomment-149401211
  
Merged build started.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/9118#issuecomment-149401495
  
**[Test build #43958 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43958/consoleFull)**
 for PR 9118 at commit 
[`b97a2b5`](https://github.com/apache/spark/commit/b97a2b5d8b1c477444d57dddccf6a30657008f96).


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/9118#issuecomment-149407006
  
**[Test build #43958 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43958/consoleFull)**
 for PR 9118 at commit 
[`b97a2b5`](https://github.com/apache/spark/commit/b97a2b5d8b1c477444d57dddccf6a30657008f96).
 * 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/9118#issuecomment-149407072
  
Merged build finished. 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/9118#issuecomment-149407073
  
Test PASSed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43958/
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[vanzin]

Github user vanzin commented on a diff in the pull request:

https://github.com/apache/spark/pull/9118#discussion_r42446420
  
--- Diff: yarn/src/main/scala/org/apache/spark/deploy/yarn/Client.scala ---
@@ -497,6 +497,19 @@ private[spark] class Client(
*/
   private def createConfArchive(): File = {
 val hadoopConfFiles = new HashMap[String, File]()
+
+// Uploading $SPARK_CONF_DIR/log4j.properties file to the distributed 
cache to make sure that
+// the executors will use the latest configurations instead of the 
default values. This is
+// required when user changes log4j.properties directly to set the log 
configurations. If
+// configuration file is provided through --files then executors will 
be taking configurations
+// from --files instead of $SPARK_CONF_DIR/log4j.properties.
+val log4jFileName = "log4j.properties"
+
Option(Utils.getContextOrSparkClassLoader.getResource(log4jFileName)).foreach {
+  url => if (url.getProtocol == "file") {
--- End diff --

style: `url =>` goes on the previous line.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/9118#issuecomment-149401196
  
 Merged build triggered.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/9118#issuecomment-149272877
  
Test PASSed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43929/
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/9118#issuecomment-149272263
  
**[Test build #43929 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43929/consoleFull)**
 for PR 9118 at commit 
[`187eb99`](https://github.com/apache/spark/commit/187eb99def45670285047c51ad8269a998fb8ed9).
 * 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/9118#issuecomment-149272876
  
Merged build finished. 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[srowen]

Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/9118#discussion_r42135926
  
--- Diff: yarn/src/main/scala/org/apache/spark/deploy/yarn/Client.scala ---
@@ -340,6 +340,14 @@ private[spark] class Client(
   "for alternatives.")
 }
 
+// Uploading $SPARK_CONF_DIR/log4j.properties file to the distributed 
cache to make sure that
+// the executors will use the latest configurations instead of the 
default values. This is
+// required when user changes log4j.properties directly to set the log 
configurations. If
+// configuration file is provided through --files then executors will 
be taking configurations
+// from --files instead of $SPARK_CONF_DIR/log4j.properties.
+val log4jConf = oldLog4jConf.orElse(
+  
Option(Utils.getContextOrSparkClassLoader.getResource("/log4j.properties")).map(_.toString))
--- End diff --

I might be over-estimating the intended scope, but this is not the only 
place that log4j.properties can occur. Generally callers can set 
-Dlog4j.configuration=... to set its 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[markgrover]

Github user markgrover commented on the pull request:

https://github.com/apache/spark/pull/9118#issuecomment-148409085
  
LGTM too, the tests seem to have passed as well.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[vanzin]

Github user vanzin commented on the pull request:

https://github.com/apache/spark/pull/9118#issuecomment-148443924
  
So, I thought about this a little bit more and I think it would be worth it 
to put that file in `createConfArchive` instead. That way, it's more efficient, 
since it's one less tiny file we're uploading to HDFS before starting the job. 
It also would maintain the current behavior of not overriding `--files`, since 
the configuration dir is added later in the classpath.

Could you try that? Thanks!


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[vanzin]

Github user vanzin commented on a diff in the pull request:

https://github.com/apache/spark/pull/9118#discussion_r42146188
  
--- Diff: yarn/src/main/scala/org/apache/spark/deploy/yarn/Client.scala ---
@@ -340,6 +340,14 @@ private[spark] class Client(
   "for alternatives.")
 }
 
+// Uploading $SPARK_CONF_DIR/log4j.properties file to the distributed 
cache to make sure that
+// the executors will use the latest configurations instead of the 
default values. This is
+// required when user changes log4j.properties directly to set the log 
configurations. If
+// configuration file is provided through --files then executors will 
be taking configurations
+// from --files instead of $SPARK_CONF_DIR/log4j.properties.
+val log4jConf = oldLog4jConf.orElse(
+  
Option(Utils.getContextOrSparkClassLoader.getResource("/log4j.properties")).map(_.toString))
--- End diff --

Sure, but we're trying to make the default case easier. If the user is 
messing with `log4j.configuration` then he probably knows what he's doing, and 
this change would not affect 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/9118#issuecomment-148163198
  
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[markgrover]

Github user markgrover commented on a diff in the pull request:

https://github.com/apache/spark/pull/9118#discussion_r42042756
  
--- Diff: yarn/src/main/scala/org/apache/spark/deploy/yarn/Client.scala ---
@@ -340,6 +340,14 @@ private[spark] class Client(
   "for alternatives.")
 }
 
+// Uploading $SPARK_CONF_DIR/log4j.properties file to the distributed 
cache to make sure that
+// the executor's will use the latest configurations instead of the 
default values. This is
+// required when user changes log4j.properties directly or use's UI 
(e.g., Cloudera Manager) to
+// set the log configurations. If configuration file is provided 
through --files then executors
+// will be taking configurations from --files instead of 
$SPARK_CONF_DIR/log4j.properties.
+val log4jConf =
+  
oldLog4jConf.orElse(Option(getClass.getResource("/log4j.properties").toString()))
--- End diff --

Could the output of getResource() be null?


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[vundela]

Github user vundela commented on a diff in the pull request:

https://github.com/apache/spark/pull/9118#discussion_r42043365
  
--- Diff: yarn/src/main/scala/org/apache/spark/deploy/yarn/Client.scala ---
@@ -340,6 +340,14 @@ private[spark] class Client(
   "for alternatives.")
 }
 
+// Uploading $SPARK_CONF_DIR/log4j.properties file to the distributed 
cache to make sure that
+// the executor's will use the latest configurations instead of the 
default values. This is
--- End diff --

Done


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[markgrover]

Github user markgrover commented on a diff in the pull request:

https://github.com/apache/spark/pull/9118#discussion_r42047771
  
--- Diff: yarn/src/main/scala/org/apache/spark/deploy/yarn/Client.scala ---
@@ -340,6 +340,14 @@ private[spark] class Client(
   "for alternatives.")
 }
 
+// Uploading $SPARK_CONF_DIR/log4j.properties file to the distributed 
cache to make sure that
+// the executor's will use the latest configurations instead of the 
default values. This is
+// required when user changes log4j.properties directly or use's UI 
(e.g., Cloudera Manager) to
+// set the log configurations. If configuration file is provided 
through --files then executors
+// will be taking configurations from --files instead of 
$SPARK_CONF_DIR/log4j.properties.
+val log4jConf =
+  
oldLog4jConf.orElse(Option(getClass.getResource("/log4j.properties").toString()))
--- End diff --

Right, and if so, calling toString() may lead to a NPE?


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[vanzin]

Github user vanzin commented on a diff in the pull request:

https://github.com/apache/spark/pull/9118#discussion_r42049285
  
--- Diff: yarn/src/main/scala/org/apache/spark/deploy/yarn/Client.scala ---
@@ -340,6 +340,14 @@ private[spark] class Client(
   "for alternatives.")
 }
 
+// Uploading $SPARK_CONF_DIR/log4j.properties file to the distributed 
cache to make sure that
+// the executors will use the latest configurations instead of the 
default values. This is
+// required when user changes log4j.properties directly to set the log 
configurations. If
+// configuration file is provided through --files then executors will 
be taking configurations
+// from --files instead of $SPARK_CONF_DIR/log4j.properties.
+val log4jConf =
+  
oldLog4jConf.orElse(Option(getClass.getResource("/log4j.properties").toString()))
--- End diff --

As Mark mentioned, this can throw a NPE.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[vundela]

Github user vundela commented on the pull request:

https://github.com/apache/spark/pull/9118#issuecomment-148163495
  
@vanzin  can you please review the pull request.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[markgrover]

Github user markgrover commented on a diff in the pull request:

https://github.com/apache/spark/pull/9118#discussion_r42042421
  
--- Diff: yarn/src/main/scala/org/apache/spark/deploy/yarn/Client.scala ---
@@ -340,6 +340,14 @@ private[spark] class Client(
   "for alternatives.")
 }
 
+// Uploading $SPARK_CONF_DIR/log4j.properties file to the distributed 
cache to make sure that
+// the executor's will use the latest configurations instead of the 
default values. This is
--- End diff --

Nit: executors


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[markgrover]

Github user markgrover commented on a diff in the pull request:

https://github.com/apache/spark/pull/9118#discussion_r42042383
  
--- Diff: yarn/src/main/scala/org/apache/spark/deploy/yarn/Client.scala ---
@@ -340,6 +340,14 @@ private[spark] class Client(
   "for alternatives.")
 }
 
+// Uploading $SPARK_CONF_DIR/log4j.properties file to the distributed 
cache to make sure that
+// the executor's will use the latest configurations instead of the 
default values. This is
+// required when user changes log4j.properties directly or use's UI 
(e.g., Cloudera Manager) to
--- End diff --

Nit: I'd prefer vendor neutral vocabulary here.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[vundela]

Github user vundela commented on a diff in the pull request:

https://github.com/apache/spark/pull/9118#discussion_r42043343
  
--- Diff: yarn/src/main/scala/org/apache/spark/deploy/yarn/Client.scala ---
@@ -340,6 +340,14 @@ private[spark] class Client(
   "for alternatives.")
 }
 
+// Uploading $SPARK_CONF_DIR/log4j.properties file to the distributed 
cache to make sure that
+// the executor's will use the latest configurations instead of the 
default values. This is
+// required when user changes log4j.properties directly or use's UI 
(e.g., Cloudera Manager) to
--- End diff --

Thanks for the suggestion, done. 


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[vundela]

GitHub user vundela opened a pull request:

https://github.com/apache/spark/pull/9118

[SPARK-11105][yarn]  Distribute log4j.properties to executors

Currently log4j.properties file is not uploaded to executor's which is 
leading them to use the default values. This fix will make sure that file is 
always uploaded to distributed cache so that executor will use the latest 
settings. 

If user specifies log configurations through --files then executors will be 
picking configs from --files instead of $SPARK_CONF_DIR/log4j.properties

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/vundela/spark master

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/spark/pull/9118.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 #9118


commit cc09f86b779f41cfa9f08e7a41198ccc0c4e61c9
Author: Srinivasa Reddy Vundela 
Date:   2015-10-14T19:04:41Z

[SPARK-11105] Distribute log4j.properties to executors




---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[vundela]

Github user vundela commented on a diff in the pull request:

https://github.com/apache/spark/pull/9118#discussion_r42043730
  
--- Diff: yarn/src/main/scala/org/apache/spark/deploy/yarn/Client.scala ---
@@ -340,6 +340,14 @@ private[spark] class Client(
   "for alternatives.")
 }
 
+// Uploading $SPARK_CONF_DIR/log4j.properties file to the distributed 
cache to make sure that
+// the executor's will use the latest configurations instead of the 
default values. This is
+// required when user changes log4j.properties directly or use's UI 
(e.g., Cloudera Manager) to
+// set the log configurations. If configuration file is provided 
through --files then executors
+// will be taking configurations from --files instead of 
$SPARK_CONF_DIR/log4j.properties.
+val log4jConf =
+  
oldLog4jConf.orElse(Option(getClass.getResource("/log4j.properties").toString()))
--- End diff --

It might, if the resource not found. 


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[vundela]

Github user vundela commented on a diff in the pull request:

https://github.com/apache/spark/pull/9118#discussion_r42058235
  
--- Diff: yarn/src/main/scala/org/apache/spark/deploy/yarn/Client.scala ---
@@ -340,6 +340,14 @@ private[spark] class Client(
   "for alternatives.")
 }
 
+// Uploading $SPARK_CONF_DIR/log4j.properties file to the distributed 
cache to make sure that
+// the executors will use the latest configurations instead of the 
default values. This is
+// required when user changes log4j.properties directly to set the log 
configurations. If
+// configuration file is provided through --files then executors will 
be taking configurations
+// from --files instead of $SPARK_CONF_DIR/log4j.properties.
+val log4jConf =
+  
oldLog4jConf.orElse(Option(getClass.getResource("/log4j.properties").toString()))
--- End diff --

Fixed.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[vundela]

Github user vundela commented on a diff in the pull request:

https://github.com/apache/spark/pull/9118#discussion_r42058190
  
--- Diff: yarn/src/main/scala/org/apache/spark/deploy/yarn/Client.scala ---
@@ -340,6 +340,14 @@ private[spark] class Client(
   "for alternatives.")
 }
 
+// Uploading $SPARK_CONF_DIR/log4j.properties file to the distributed 
cache to make sure that
+// the executor's will use the latest configurations instead of the 
default values. This is
+// required when user changes log4j.properties directly or use's UI 
(e.g., Cloudera Manager) to
+// set the log configurations. If configuration file is provided 
through --files then executors
+// will be taking configurations from --files instead of 
$SPARK_CONF_DIR/log4j.properties.
+val log4jConf =
+  
oldLog4jConf.orElse(Option(getClass.getResource("/log4j.properties").toString()))
--- End diff --

Thanks for catching, fixed it.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[vanzin]

Github user vanzin commented on a diff in the pull request:

https://github.com/apache/spark/pull/9118#discussion_r42063646
  
--- Diff: yarn/src/main/scala/org/apache/spark/deploy/yarn/Client.scala ---
@@ -340,6 +340,15 @@ private[spark] class Client(
   "for alternatives.")
 }
 
+// Uploading $SPARK_CONF_DIR/log4j.properties file to the distributed 
cache to make sure that
+// the executors will use the latest configurations instead of the 
default values. This is
+// required when user changes log4j.properties directly to set the log 
configurations. If
+// configuration file is provided through --files then executors will 
be taking configurations
+// from --files instead of $SPARK_CONF_DIR/log4j.properties.
+val log4jConfUrl = getClass.getResource("/log4j.properties")
+val log4jConf =
+  oldLog4jConf.orElse(Option(if (log4jConfUrl == null) null else 
log4jConfUrl.toString()))
--- End diff --

So, a more scala-y way of doing this would be:

val log4jConf = 
oldLog4jConf.orElse(Option(getClass.getResource("/log4j.properties")).map(_.toString))

Also, it might be better to use 
`Utils.getContextOrSparkClassLoader.getResource` instead, so that it picks up 
the user's log4j.properties it they embed it in their app's 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[vundela]

Github user vundela commented on a diff in the pull request:

https://github.com/apache/spark/pull/9118#discussion_r42067281
  
--- Diff: yarn/src/main/scala/org/apache/spark/deploy/yarn/Client.scala ---
@@ -340,6 +340,15 @@ private[spark] class Client(
   "for alternatives.")
 }
 
+// Uploading $SPARK_CONF_DIR/log4j.properties file to the distributed 
cache to make sure that
+// the executors will use the latest configurations instead of the 
default values. This is
+// required when user changes log4j.properties directly to set the log 
configurations. If
+// configuration file is provided through --files then executors will 
be taking configurations
+// from --files instead of $SPARK_CONF_DIR/log4j.properties.
+val log4jConfUrl = getClass.getResource("/log4j.properties")
+val log4jConf =
+  oldLog4jConf.orElse(Option(if (log4jConfUrl == null) null else 
log4jConfUrl.toString()))
--- End diff --

Done


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/9118#issuecomment-148239448
  
  [Test build #43756 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43756/consoleFull)
 for   PR 9118 at commit 
[`9669a32`](https://github.com/apache/spark/commit/9669a32f83c48e4cd1c1b7a7d39aad7737be68a2).


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/9118#issuecomment-148242697
  
  [Test build #43756 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43756/console)
 for   PR 9118 at commit 
[`9669a32`](https://github.com/apache/spark/commit/9669a32f83c48e4cd1c1b7a7d39aad7737be68a2).
 * 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/9118#issuecomment-148238862
  
 Merged build triggered.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/9118#issuecomment-148238885
  
Merged build started.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/9118#issuecomment-148242856
  
Test PASSed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/43756/
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/9118#issuecomment-148242853
  
Merged build finished. 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[vanzin]

Github user vanzin commented on the pull request:

https://github.com/apache/spark/pull/9118#issuecomment-148238683
  
LGTM pending tests.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-11105][yarn] Distribute log4j.propertie...

[vanzin]

Github user vanzin commented on the pull request:

https://github.com/apache/spark/pull/9118#issuecomment-148238617
  
ok to test


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org