virajjasani commented on a change in pull request #2219:
URL: https://github.com/apache/hbase/pull/2219#discussion_r471075141



##########
File path: 
hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncConnectionImpl.java
##########
@@ -199,6 +199,10 @@ public void close() {
     if (!closed.compareAndSet(false, true)) {
       return;
     }
+    LOG.info("Connection has been closed by {}.", 
Thread.currentThread().getName());

Review comment:
       > Most of the times where this log message would have helped me in the 
last ~6 months the thread name and an unambiguous "we closed the connection" 
would suffice.
   
   That's perfectly fine, if thread name is enough for prod env and if we have 
a way to repro in dev env, we can let stacktrace be at `debug`. The only thing 
I was concerned about was whether not finding stacktrace in prod env would 
still keep things unclear for user. However, here I believe once we get the 
thread name, we can find recent logs of that thread to find more.
   
   > I generally am against non-error related stack traces at log levels above 
DEBUG.
   
   +1 for this unless the non-error trace is useful for detailed debugging, in 
which case maybe we can write something specifically to indicate to not treat 
this as erroneous condition.
   However, for our current PR, it seems thread-name is most likely going to 
provide sufficient info.




----------------------------------------------------------------
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.

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


Reply via email to