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

Phabricator commented on HIVE-2691:
-----------------------------------

cwsteinbach has requested changes to the revision "HIVE-2691 [jira] Specify 
location of log4j configuration files via configuration properties".

INLINE COMMENTS
  common/src/java/org/apache/hadoop/hive/common/LogUtils.java:48 This method 
should not take an input parameter. Instead, look the config value up directly 
using HiveConf.
  common/src/java/org/apache/hadoop/hive/common/LogUtils.java:52 If the logging 
conf file is specified using the configuration property, then you have to 
assume that the value is a canonical path, and you need to look it up on the 
local filesystem, not from the classpath.
  common/src/java/org/apache/hadoop/hive/common/LogUtils.java:50 The default 
value for the two new conf properties is "". Checking for null will not catch 
this.
  ql/src/java/org/apache/hadoop/hive/ql/exec/ExecDriver.java:551 Please move 
this code to LogUtils.initHiveExecLog4j()
  ql/src/java/org/apache/hadoop/hive/ql/exec/ExecDriver.java:557 The default 
value of HiveConf.ConfVars.HIVE_EXEC_LOG4J_FILE is "". We should also check to 
make sure that the file actually exists. If it doesn't, we should use the 
default log conf from the classpath instead, and probably log a message 
explaining that we weren't able to find the conf file specified by HiveConf.
  ql/src/java/org/apache/hadoop/hive/ql/exec/ExecDriver.java:566 The javadoc 
for PropertyConfigurator.configure(String) is a little unclear about whether or 
not it can handle canonical paths that specify locations which aren't on the 
classpath, and since it doesn't return any error information I'm not convinced 
that this is actually working. Please try setting the log4j.debug variable 
(mentioned in the javadoc) to see if you can get any diagnostic information 
about where it's loading the logging configuration from.

  
http://logging.apache.org/log4j/1.2/apidocs/org/apache/log4j/PropertyConfigurator.html


  conf/hive-default.xml.template:1201 Both descriptions should make the 
following clear:

  * If the property is not set, then logging will be initialized using 
hive-log4j.properties (or hive-exec-log4j.properties) found on the classpath.

  * If the property is set, the value must be a valid URI (java.net.URI, e.g. 
"file:///tmp/my-logging.properties"), which you can then extract a URL from and 
pass to PropertyConfigurator.configure(URL).
  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java:63 I'm pretty sure 
that these logging statements are silently failing since this block of code 
runs before HiveConf.init() is called.

  I think the cleanest solution is to add getters to HiveConf that provide 
access to the hiveDefaultURl and hiveSiteURL variables, and then log the same 
information in LogUtils.initHiveLog4j() after logging has been initialized.
  common/src/java/org/apache/hadoop/hive/common/LogUtils.java:32 Change the 
visibility to private.
  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java:840 I'm reversing 
what I said earlier. I think this code should be moved back to CliDriver, 
ExecDriver, etc, and initHiveLog4j/initHiveExecLog4j should take a HiveConf 
object as input.

REVISION DETAIL
  https://reviews.facebook.net/D1203

                
> Specify location of log4j configuration files via configuration properties
> --------------------------------------------------------------------------
>
>                 Key: HIVE-2691
>                 URL: https://issues.apache.org/jira/browse/HIVE-2691
>             Project: Hive
>          Issue Type: New Feature
>          Components: Configuration, Logging
>            Reporter: Carl Steinbach
>            Assignee: Zhenxiao Luo
>         Attachments: HIVE-2691.D1131.1.patch, HIVE-2691.D1203.1.patch
>
>
> Oozie needs to be able to override the default location of the log4j 
> configuration
> files from the Hive command line, e.g:
> {noformat}
> hive -hiveconf hive.log4j.file=/home/carl/hive-log4j.properties -hiveconf 
> hive.log4j.exec.file=/home/carl/hive-exec-log4j.properties
> {noformat}

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

Reply via email to