[jira] [Commented] (MAPREDUCE-6052) Support overriding log4j.properties per job

["Junping Du (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/MAPREDUCE-6052?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14192957#comment-14192957 ] 

Junping Du commented on MAPREDUCE-6052:
---------------------------------------

bq. In copyAndConfigureFiles, the following code has bee executed before. We don't need to invoke them again in addLog4jToDistributedCache. and we can combine addLog4jToDistributedCache and copyLog4jPropertyFile into one method.
Let's keep it here. We do can have flexibility in future for switching job working directory or changing replication number. Also, combining two methods don't have significant benefits, let's along existing copyAndConfigureFiles() except we have strong justification for change.

bq. The same bunch of code is already executed before in copyAndConfigureFiles(3 args). No need to repeat in copyLog4jPropertyFile, and mapredSysPerms can be passed as an arg.
It is safety to keep checking directory existing before using it given the possible refactoring or other operations in parallel.

bq. FileSystem localFs = FileSystem.getLocal(conf); only make sense to the if condition is true.
Nice catch! However, given this is a very minor one and this is a critical issue in 2.6 release which is pretty closed to end. Can we refactor it later?

bq. I still have one question about a corner case:
That's a good question. In your case, there will be a classic scenario:  two files with the same name on classpath (one is loading in conf directory by default, the other is ), which one would take effective? It actually depends on which file appear on classpath earlier (http://stackoverflow.com/questions/6644440/java-which-of-multiple-resources-on-classpath-jvm-takes), but not sure if all JVMs conform the same spec on this. So the best way to do (it is also what we should recommend to user/customer) is to use a different name (like: log4j.properties in most cases) other than the default one.

> Support overriding log4j.properties per job
> -------------------------------------------
>
>                 Key: MAPREDUCE-6052
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-6052
>             Project: Hadoop Map/Reduce
>          Issue Type: Bug
>    Affects Versions: 2.5.0
>            Reporter: Junping Du
>            Assignee: Junping Du
>         Attachments: MAPREDUCE-6052-v2.patch, MAPREDUCE-6052-v3.patch, MAPREDUCE-6052-v4.patch, MAPREDUCE-6052.patch
>
>
> For current MR application, the "log4j.configuration" is hard coded to container-log4j.properties within each node. We still need flexibility to override it per job like what we do in MRV1.
> {code}
>   public static void addLog4jSystemProperties(
>       String logLevel, long logSize, int numBackups, List<String> vargs) {
>     vargs.add("-Dlog4j.configuration=container-log4j.properties");
> {code}



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)