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