-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.apache.org/r/7/#review2
-----------------------------------------------------------


Most of the messages you changed are things we rely on to post-mortem issues. 
Many of these logs were added based on experiences running production systems 
and helping users (and admins) identify what's going on in a running system. 
Generally these are only output when something significant changes 
(conn/session est/close) and are therefore not seen frequently.

Alternately (vs this patch) I think you should look at configuring your log4j 
configuration in log4j.properties. You can pretty much get what you want (turn 
off info messages) by either setting the appender threshold to "WARN" or by 
setting the threshold on a per-class basis.

See: http://logging.apache.org/log4j/1.2/manual.html specifically:

# Print only messages of level WARN or above in the package com.foo.
log4j.logger.com.foo=WARN

You can set this parameter for a specific class, such as 
log4j.logger.org.apache.zookeeper.ZooKeeper (etc...)



/src/java/main/org/apache/zookeeper/ClientCnxn.java
<http://reviews.apache.org/r/7/#comment1>

    This is very useful to see in some cases. (however not as critical as some 
of the other changes I've highlighted, and it should def not be trace).
    
    There have definitely been some cases in production systems where all you 
have is an INFO level log that this has helped to debug. I'd be hesitant to 
remove this, esp given it's typically only output when the client is shut down.



/src/java/main/org/apache/zookeeper/ClientCnxn.java
<http://reviews.apache.org/r/7/#comment2>

    This is a critical message, it should not be changed.



/src/java/main/org/apache/zookeeper/ClientCnxn.java
<http://reviews.apache.org/r/7/#comment3>

    similar to line 744.
    
    Connection establishment and session establishment are two critical 
messages for understanding client state.



/src/java/main/org/apache/zookeeper/ClientCnxn.java
<http://reviews.apache.org/r/7/#comment4>

    Nice to know this if/when clients are unable to connect to the server. At 
least you know they are trying.



/src/java/main/org/apache/zookeeper/ClientCnxn.java
<http://reviews.apache.org/r/7/#comment5>

    again not as critical, but very useful in production env.



/src/java/main/org/apache/zookeeper/Environment.java
<http://reviews.apache.org/r/7/#comment6>

    Not a good idea. This is critical for debugging issues.



/src/java/main/org/apache/zookeeper/ZooKeeper.java
<http://reviews.apache.org/r/7/#comment7>

    this is important for verifying that the user is establishing the 
connection, it's only output at session creation time. I wouldn't remove it 
(actually it was added/improved based on user feedback).



/src/java/main/org/apache/zookeeper/ZooKeeper.java
<http://reviews.apache.org/r/7/#comment8>

    same here



/src/java/main/org/apache/zookeeper/ZooKeeper.java
<http://reviews.apache.org/r/7/#comment9>

    ditto


- Patrick


On None, Anthony Urso wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.apache.org/r/7/
> -----------------------------------------------------------
> 
> Review request for zookeeper.
> 
> 
> Summary
> -------
> 
> Lower log levels in ZK client for trace/debug messages.
> 
> 
> Diffs
> -----
> 
>   /src/java/main/org/apache/zookeeper/ClientCnxn.java 1027676 
>   /src/java/main/org/apache/zookeeper/Environment.java 1027676 
>   /src/java/main/org/apache/zookeeper/ZooKeeper.java 1027676 
> 
> Diff: http://reviews.apache.org/r/7/diff
> 
> 
> Testing
> -------
> 
> Manual functional testing.
> 
> 
> Thanks,
> 
> Anthony
> 
>

Reply via email to