virajjasani commented on a change in pull request #2219:
URL: https://github.com/apache/hbase/pull/2219#discussion_r471012031
##########
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:
> I'm also worried log stack at info level is confusing since the format
is like an exception.
Even though it is Exception, but we do want to log which Exception caused
Connection closure (once in a while) right? If so, then keeping threadname at
info level and stacktrace at debug somehow doesn't fit well IMHO.
If we know for certain that threadname is useful to get sufficient info,
then we should not log stacktrace at all, and if stacktrace is required for
debugging, then it should be at the same level as threadname.
> Level debug is inconvenient since we have to re-run the user application
to reproduce it.
This is exactly applicable to stacktrace also. If application is running
with `info` level and if threadname is not sufficient to know the cause, then
we can't get stacktrace either because our application was running with `info`
level at that time and stacktrace is present at `debug` level in code.
`¯\_(ツ)_/¯`
----------------------------------------------------------------
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:
[email protected]