[GitHub] spark pull request: SPARK-1252. On YARN, use container-log4j.prope...

[asfgit]

Github user asfgit closed the pull request at:

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


---
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.
---

[GitHub] spark pull request: SPARK-1252. On YARN, use container-log4j.prope...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/148#issuecomment-39762310
  

Refer to this link for build results: 
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/13847/


---
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.
---

[GitHub] spark pull request: SPARK-1252. On YARN, use container-log4j.prope...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/148#issuecomment-39762308
  
Merged build finished. 


---
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.
---

[GitHub] spark pull request: SPARK-1252. On YARN, use container-log4j.prope...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

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

[GitHub] spark pull request: SPARK-1252. On YARN, use container-log4j.prope...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

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

[GitHub] spark pull request: SPARK-1252. On YARN, use container-log4j.prope...

[sryza]

Github user sryza commented on the pull request:

https://github.com/apache/spark/pull/148#issuecomment-39756329
  
Thanks Tom.  I just rebased on master.

> Note the other option would be to change the 
conf/log4j.properties.template to be more like hadoops. 
I don't have an opinion on this, but happy to make the change if you think 
it's the right thing.


---
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.
---

[GitHub] spark pull request: SPARK-1252. On YARN, use container-log4j.prope...

[tgravescs]

Github user tgravescs commented on the pull request:

https://github.com/apache/spark/pull/148#issuecomment-39732013
  
Note the other option would be to change the conf/log4j.properties.template 
to be more like hadoops.  


---
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.
---

[GitHub] spark pull request: SPARK-1252. On YARN, use container-log4j.prope...

[tgravescs]

Github user tgravescs commented on the pull request:

https://github.com/apache/spark/pull/148#issuecomment-39731322
  
Changes look good.   Sandy can you merge up to the latest master and I'll 
commit.


---
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.
---

[GitHub] spark pull request: SPARK-1252. On YARN, use container-log4j.prope...

[sryza]

Github user sryza commented on the pull request:

https://github.com/apache/spark/pull/148#issuecomment-39700608
  
Right, (b) is different than the original intent of the PR.  The reason for 
copying Spark's log4j instead of Hadoop's was the concern brought up by 
@tgravescs earlier:

> The one downside to making the yarn one default is that we now get 
different looking logs if the user just uses the spark one. In the default most 
things go to syslog file, and if I just put in the conf/log4j.properties by 
copying the template I won't get a syslog file and most things will be in 
stdout right? 

This applies to the master too.


---
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.
---

[GitHub] spark pull request: SPARK-1252. On YARN, use container-log4j.prope...

[mridulm]

Github user mridulm commented on the pull request:

https://github.com/apache/spark/pull/148#issuecomment-39653130
  
Just to be clear, the current status of the patch seems to be :
a) If user specified logging config - use that.
b) If missing, use a spark config built into the jar.

(b) seems to be different from the original intent of the PR, but I guess 
if we cant merge the container logging config with ours, it cant be helped.
What about simply copying the existing hadoop log4j container config into 
(b) and expanding the variables as was done in the first PR when (a) is missing 
? (it would have the nice property of logs going to syslog in that case, no ? 
Or wont that not work ?)
Also, this is distinct from what happens in the master ? Or applies there 
too ?


---
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.
---

[GitHub] spark pull request: SPARK-1252. On YARN, use container-log4j.prope...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/148#issuecomment-39611680
  
All automated tests passed.
Refer to this link for build results: 
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/13773/


---
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.
---

[GitHub] spark pull request: SPARK-1252. On YARN, use container-log4j.prope...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/148#issuecomment-39611678
  
Merged build finished. All automated tests 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.
---

[GitHub] spark pull request: SPARK-1252. On YARN, use container-log4j.prope...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

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

[GitHub] spark pull request: SPARK-1252. On YARN, use container-log4j.prope...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

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

[GitHub] spark pull request: SPARK-1252. On YARN, use container-log4j.prope...

[sryza]

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

https://github.com/apache/spark/pull/148#discussion_r11310525
  
--- Diff: yarn/alpha/src/main/resources/log4j-spark-container.properties ---
@@ -0,0 +1,24 @@
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License. See accompanying LICENSE file.
+
+# Set everything to be logged to the console
+log4j.rootCategory=INFO, console
+log4j.appender.console=org.apache.log4j.ConsoleAppender
+log4j.appender.console.target=System.err
+log4j.appender.console.layout=org.apache.log4j.PatternLayout
--- End diff --

I tried putting them in common initially and they weren't showing up in the 
YARN jar.  I just tried mucking around with Maven a bit and was able to get 
this to work, so I'm uploading a new patch.  I'm not sure how to do the same in 
sbt though.


---
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.
---

[GitHub] spark pull request: SPARK-1252. On YARN, use container-log4j.prope...

[tgravescs]

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

https://github.com/apache/spark/pull/148#discussion_r11307961
  
--- Diff: yarn/alpha/src/main/resources/log4j-spark-container.properties ---
@@ -0,0 +1,24 @@
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License. See accompanying LICENSE file.
+
+# Set everything to be logged to the console
+log4j.rootCategory=INFO, console
+log4j.appender.console=org.apache.log4j.ConsoleAppender
+log4j.appender.console.target=System.err
+log4j.appender.console.layout=org.apache.log4j.PatternLayout
--- End diff --

do we need to have 2 different log4j properties files or can they go in 
common?


---
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.
---

[GitHub] spark pull request: SPARK-1252. On YARN, use container-log4j.prope...

[pwendell]

Github user pwendell commented on the pull request:

https://github.com/apache/spark/pull/148#issuecomment-39591912
  
@tgravescs mind taking a look at 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] spark pull request: SPARK-1252. On YARN, use container-log4j.prope...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

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

[GitHub] spark pull request: SPARK-1252. On YARN, use container-log4j.prope...

[sryza]

Github user sryza commented on the pull request:

https://github.com/apache/spark/pull/148#issuecomment-38866826
  
I uploaded a patch with a new approach.  In it, we use neither Hadoop's 
log4j.properties nor Hadoop's container-log4j.properties. I added a 
log4j-spark-container.properties file (with the same conf as 
conf/log4j.properties.template) that gets included in the Spark yarn jar.  This 
is used by default, but if the user specifies a SPARK_LOG4J_CONF, that will get 
used instead.

I think this should address the issues that y'all have pointed out with the 
original approach, but let me know if I'm missing anything.


---
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.
---

[GitHub] spark pull request: SPARK-1252. On YARN, use container-log4j.prope...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

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

[GitHub] spark pull request: SPARK-1252. On YARN, use container-log4j.prope...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

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

[GitHub] spark pull request: SPARK-1252. On YARN, use container-log4j.prope...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/148#issuecomment-38765113
  
Merged build finished.


---
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.
---

[GitHub] spark pull request: SPARK-1252. On YARN, use container-log4j.prope...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/148#issuecomment-38765114
  
One or more automated tests failed
Refer to this link for build results: 
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/13490/


---
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.
---

[GitHub] spark pull request: SPARK-1252. On YARN, use container-log4j.prope...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

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

[GitHub] spark pull request: SPARK-1252. On YARN, use container-log4j.prope...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

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

[GitHub] spark pull request: SPARK-1252. On YARN, use container-log4j.prope...

[tgravescs]

Github user tgravescs commented on the pull request:

https://github.com/apache/spark/pull/148#issuecomment-37934593
  
I agree it could be an improvement.  I just don't want to cause confusion 
when their logs go to a different file when they turn debug on.  I'm not sure I 
have any other good ideas on ways around it.  Unless we just document (or put 
lines in the log4j.properties.template for yarn) that on yarn to get the same 
behavior use org.apache.hadoop.yarn.ContainerLogAppender.

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

[GitHub] spark pull request: SPARK-1252. On YARN, use container-log4j.prope...

[sryza]

Github user sryza commented on the pull request:

https://github.com/apache/spark/pull/148#issuecomment-37841517
  
@tgravescs separating out log4j logs from stdout seems like a possible 
improvement to me, but if you think it will cause confusion, we can still use 
container-log4j.properties and use JVM props to configure it to log to stdout.


---
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.
---

[GitHub] spark pull request: SPARK-1252. On YARN, use container-log4j.prope...

[tgravescs]

Github user tgravescs commented on the pull request:

https://github.com/apache/spark/pull/148#issuecomment-37825208
  
As pointed out the SPARK_LOG4J_CONF env variable can be used to set the 
logging for spark on yarn.  

The one downside to making the yarn one default is that we now get 
different looking logs if the user just uses the spark one.  In the default 
most things go to syslog file, and if I just put in the conf/log4j.properties 
by copying the template I won't get a syslog file and most things will be in 
stdout right?  


---
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.
---

[GitHub] spark pull request: SPARK-1252. On YARN, use container-log4j.prope...

[mridulm]

Github user mridulm commented on the pull request:

https://github.com/apache/spark/pull/148#issuecomment-37774742
  
Check for use of SPARK_LOG4J_CONF in yarn/
I think primarily in
yarn/common/src/main/scala/org/apache/spark/deploy/yarn/ClientBase.scala
(The code has changed quite a bit since I wrote it, so I can only give
pointers unfortunately)


On Sun, Mar 16, 2014 at 1:32 PM, Patrick Wendell
wrote:

> It's unfortunate that Hadoop publishes the log4j.properties file directly
> in it's jars. This is exactly why we've avoided doing this in Spark 
because
> it creates a weird situation where you can't easily control logging
> preferences.
>
> @sryza  @mridulm  -
> I think what we need is a precedence order here:
>
>- If a user explicitly gives a log4j.properties, use that 
(@mridul- I didn't actually know we allowed this in 
the YARN launcher, where is the
>code relevant to that?)
>- If a user jar has a log4j.properties file in it, we should use that.
>- If not, we can use the YARN cluster defaults when running on YARN.
>
> --
> Reply to this email directly or view it on 
GitHub
> .
>


---
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.
---

[GitHub] spark pull request: SPARK-1252. On YARN, use container-log4j.prope...

[pwendell]

Github user pwendell commented on the pull request:

https://github.com/apache/spark/pull/148#issuecomment-37769373
  
It's unfortunate that Hadoop publishes the log4j.properties file directly 
in it's jars. This is exactly why we've avoided doing this in Spark because it 
creates a weird situation where you can't easily control logging preferences.

@sryza @mridulm - I think what we need is a precedence order here:
- If a user explicitly gives a log4j.properties, use that (@mridul - I 
didn't actually know we allowed this in the YARN launcher, where is the code 
relevant to that?)
- If a user jar has a log4j.properties file in it, we should use that.
- If not, we can use the YARN cluster defaults when running on 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] spark pull request: SPARK-1252. On YARN, use container-log4j.prope...

[mridulm]

Github user mridulm commented on the pull request:

https://github.com/apache/spark/pull/148#issuecomment-37766354
  
Sure, or merge it with the user provided logging config even if provided
(which could be quite tricky, maybe not a good idea).
I just want to ensure that users have the ability to customize and/or
override logging config for their specific app.


On Sun, Mar 16, 2014 at 11:55 AM, Sandy Ryza 
wrote:

> @pwendell  Spark's build excluding
> log4j.properties is not enough to keep it off the executor classpath.
> Executor classpaths include the Hadoop jars as installed locally on the
> cluster machines. And those include a log4j.properties.
>
> @mridulm  My bad, I hadn't noticed that there
> was a way to pass a log4j.properties. In that case, I think the best thing
> would be to use container-log4j.properties by default, but, if the client
> gives one, to use that instead. What do you think?
>
> --
> Reply to this email directly or view it on 
GitHub
> .
>


---
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.
---

[GitHub] spark pull request: SPARK-1252. On YARN, use container-log4j.prope...

[sryza]

Github user sryza commented on the pull request:

https://github.com/apache/spark/pull/148#issuecomment-37766301
  
@pwendell Spark's build excluding log4j.properties is not enough to keep it 
off the executor classpath.  Executor classpaths include the Hadoop jars as 
installed locally on the cluster machines.  And those include a 
log4j.properties.

@mridulm My bad, I hadn't noticed that there was a way to pass a 
log4j.properties.  In that case, I think the best thing would be to use 
container-log4j.properties by default, but, if the client gives one, to use 
that instead.  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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] spark pull request: SPARK-1252. On YARN, use container-log4j.prope...

[mridulm]

Github user mridulm commented on the pull request:

https://github.com/apache/spark/pull/148#issuecomment-37764986
  
Ah ok !
But wont that not be a function of how the user builds his jar ? It is much 
more easier to modify a config file than to rebuild a jar (particularly when 
the staging node does not have the source :-) )


---
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.
---

[GitHub] spark pull request: SPARK-1252. On YARN, use container-log4j.prope...

[pwendell]

Github user pwendell commented on the pull request:

https://github.com/apache/spark/pull/148#issuecomment-37764878
  
@mridulm I'm referring to the application jar, not the Spark assembly. The 
spark assembly always excludes log4j.properties intentionally.


---
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.
---