You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by "cshannon (via GitHub)" <gi...@apache.org> on 2023/05/12 12:46:15 UTC
[GitHub] [accumulo] cshannon opened a new pull request, #3398: Include missing stack trace when logging error
cshannon opened a new pull request, #3398:
URL: https://github.com/apache/accumulo/pull/3398
Update the log message for InterruptedException inside the Tablet class to include the full stack trace as it is logged at the error level.
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] EdColeman commented on a diff in pull request #3398: Include missing stack trace when logging error
Posted by "EdColeman (via GitHub)" <gi...@apache.org>.
EdColeman commented on code in PR #3398:
URL: https://github.com/apache/accumulo/pull/3398#discussion_r1192451320
##########
server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java:
##########
@@ -984,7 +984,7 @@ synchronized void completeClose(boolean saveState, boolean completeClose) throws
activeScans.size());
this.wait(50);
} catch (InterruptedException e) {
- log.error(e.toString());
+ log.error("{}", e, e);
Review Comment:
```suggestion
log.error("Interrupted waiting to completeClose for extent {}, {}", extent, e, e);
```
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] ctubbsii commented on a diff in pull request #3398: Include missing stack trace when logging error
Posted by "ctubbsii (via GitHub)" <gi...@apache.org>.
ctubbsii commented on code in PR #3398:
URL: https://github.com/apache/accumulo/pull/3398#discussion_r1192904064
##########
server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java:
##########
@@ -984,7 +984,7 @@ synchronized void completeClose(boolean saveState, boolean completeClose) throws
activeScans.size());
this.wait(50);
} catch (InterruptedException e) {
- log.error(e.toString());
+ log.error("Interrupted waiting to completeClose for extent {}, {}", extent, e, e);
Review Comment:
With a custom message, it's no longer necessary to force the exception.toString to be part of the message. That's mostly a hack to satisfy the logging API when you don't have a custom message to satisfy the API's need for a string as the first parameter. Having the stack trace is sufficient, as it will already display the message.
```suggestion
log.error("Interrupted waiting to completeClose for extent {}", extent, e);
```
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] cshannon commented on a diff in pull request #3398: Include missing stack trace when logging error
Posted by "cshannon (via GitHub)" <gi...@apache.org>.
cshannon commented on code in PR #3398:
URL: https://github.com/apache/accumulo/pull/3398#discussion_r1192463596
##########
server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java:
##########
@@ -984,7 +984,7 @@ synchronized void completeClose(boolean saveState, boolean completeClose) throws
activeScans.size());
this.wait(50);
} catch (InterruptedException e) {
- log.error(e.toString());
+ log.error("{}", e, e);
Review Comment:
This is fine with me, it provides more context.
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] cshannon commented on pull request #3398: Include missing stack trace when logging error
Posted by "cshannon (via GitHub)" <gi...@apache.org>.
cshannon commented on PR #3398:
URL: https://github.com/apache/accumulo/pull/3398#issuecomment-1545693132
This is a minor change targeted for 2.1 to logging based on @ctubbsii comments in #3368 . I will go ahead and merge this as soon as the CI checks pass.
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] cshannon merged pull request #3398: Include missing stack trace when logging error
Posted by "cshannon (via GitHub)" <gi...@apache.org>.
cshannon merged PR #3398:
URL: https://github.com/apache/accumulo/pull/3398
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org