You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@helix.apache.org by GitBox <gi...@apache.org> on 2022/09/07 17:23:21 UTC

[GitHub] [helix] jiajunwang commented on a diff in pull request #2201: Downgraded Deletion Error Logs to Avoid Error Spam Logging

jiajunwang commented on code in PR #2201:
URL: https://github.com/apache/helix/pull/2201#discussion_r965103623


##########
helix-core/src/main/java/org/apache/helix/manager/zk/ZkBaseDataAccessor.java:
##########
@@ -726,7 +726,7 @@ public boolean remove(String path, int options) {
       // case, we try to delete recursively
       _zkClient.delete(path);
     } catch (ZkException e) {
-      LOG.debug("Failed to delete {} with opts {}, err: {}. Try recursive delete", path, options,
+      LOG.info("Failed to delete {} with opts {}, err: {}. Try recursive delete", path, options,

Review Comment:
   I'm confused here.
   If set the log level to info, then orignal code will NOT print debug log, but the new change will be printing more logs. It contradicts to the description.
   
   > A log request of level p in a logger with level q is enabled if p >= q. This rule is at the heart of log4j. It assumes that levels are ordered. For the standard levels, we have ALL < DEBUG < INFO < WARN < ERROR < FATAL < OFF.
   Here, in the original code p == debug and q == info. So the log won't be printed.
   
   Debug to Info is considered upgrade instead of downgrade.



##########
helix-core/src/main/java/org/apache/helix/manager/zk/ZkBaseDataAccessor.java:
##########
@@ -726,7 +726,7 @@ public boolean remove(String path, int options) {
       // case, we try to delete recursively
       _zkClient.delete(path);
     } catch (ZkException e) {
-      LOG.debug("Failed to delete {} with opts {}, err: {}. Try recursive delete", path, options,
+      LOG.info("Failed to delete {} with opts {}, err: {}. Try recursive delete", path, options,

Review Comment:
   I'm confused here.
   If set the log level to info, then orignal code will NOT print debug log, but the new change will be printing more logs. It contradicts to the description.
   
   > A log request of level p in a logger with level q is enabled if p >= q. This rule is at the heart of log4j. It assumes that levels are ordered. For the standard levels, we have ALL < DEBUG < INFO < WARN < ERROR < FATAL < OFF.
   
   Here, in the original code p == debug and q == info. So the log won't be printed.
   
   Debug to Info is considered upgrade instead of downgrade.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org