rahulrane50 commented on code in PR #2201:
URL: https://github.com/apache/helix/pull/2201#discussion_r964451422
##########
zookeeper-api/src/main/java/org/apache/helix/zookeeper/zkclient/ZkClient.java:
##########
@@ -2073,7 +2073,7 @@ public Object call() throws Exception {
record(path, null, startT, ZkClientMonitor.AccessType.WRITE);
} catch (Exception e) {
recordFailure(path, ZkClientMonitor.AccessType.WRITE);
- LOG.warn("zkclient {}, Failed to delete path {}! ", _uid, path, e);
+ LOG.debug("zkclient {}, Failed to delete path {}! ", _uid, path, e);
Review Comment:
Umm is this correct? There are 3 parameters for 2 positional brackets.
##########
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:
neat : I understand the overall intention of changing this to INFO level but
I would be very cautious about this change since this is very core method which
is used in many places. Any small changes in logging here would flood logs in
specific scenarios. Also could you help me understand why was the "debug" log
noise in the first place. It shouldn't even show up in logs unless debug
logging is enabled right? (I assume debug logging is disabled by default)
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
