You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2023/01/10 22:56:16 UTC
[GitHub] [ozone] DaveTeng0 opened a new pull request, #4169: HDDS-7097. Container scanner log output lacks useful information
DaveTeng0 opened a new pull request, #4169:
URL: https://github.com/apache/ozone/pull/4169
## What changes were proposed in this pull request?
1. Add log for the exception object when container corruption happens to better identify the failure.
2. Remove the stack trace of the call to KeyValueContainer#markContainerUnhealthy (not very useful).
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-7097
## How was this patch tested?
Manually test in local cluster to verify the log message.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] DaveTeng0 commented on pull request #4169: HDDS-7097. Container scanner log output lacks useful information
Posted by GitBox <gi...@apache.org>.
DaveTeng0 commented on PR #4169:
URL: https://github.com/apache/ozone/pull/4169#issuecomment-1386319707
> > Manually test in local cluster to verify the log message.
>
> Please provide example output.
Sure! I'll add in the PR description!
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] sodonnel commented on a diff in pull request #4169: HDDS-7097. Container scanner log output lacks useful information
Posted by GitBox <gi...@apache.org>.
sodonnel commented on code in PR #4169:
URL: https://github.com/apache/ozone/pull/4169#discussion_r1066777565
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueContainerCheck.java:
##########
@@ -401,6 +401,6 @@ private void handleCorruption(IOException e) {
String errStr =
"Corruption detected in container: [" + containerID + "] ";
String logMessage = errStr + "Exception: [" + e.getMessage() + "]";
- LOG.error(logMessage);
+ LOG.error(logMessage, e);
Review Comment:
You could clean this up further by removing the two intermediate strings:
```
LOG.error("Corruption detected in container: [{}]. Exception: {}", containerID, e.getMessage, e);
```
Does logging the full stack trace via the last exception parameter not also log the e.getMessage() too?
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] DaveTeng0 commented on pull request #4169: HDDS-7097. Container scanner log output lacks useful information
Posted by GitBox <gi...@apache.org>.
DaveTeng0 commented on PR #4169:
URL: https://github.com/apache/ozone/pull/4169#issuecomment-1378002814
(cc. @errose28 )
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] DaveTeng0 commented on a diff in pull request #4169: HDDS-7097. Container scanner log output lacks useful information
Posted by GitBox <gi...@apache.org>.
DaveTeng0 commented on code in PR #4169:
URL: https://github.com/apache/ozone/pull/4169#discussion_r1072966360
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueContainerCheck.java:
##########
@@ -401,6 +401,6 @@ private void handleCorruption(IOException e) {
String errStr =
"Corruption detected in container: [" + containerID + "] ";
String logMessage = errStr + "Exception: [" + e.getMessage() + "]";
- LOG.error(logMessage);
+ LOG.error(logMessage, e);
Review Comment:
Oops yes it does contain e.getMessage() in the exception object! I'll remove the redundant e.getMessage()! Thanks for the check Stephen!
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] errose28 commented on a diff in pull request #4169: HDDS-7097. Container scanner log output lacks useful information
Posted by GitBox <gi...@apache.org>.
errose28 commented on code in PR #4169:
URL: https://github.com/apache/ozone/pull/4169#discussion_r1074003948
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueContainerCheck.java:
##########
@@ -398,9 +396,7 @@ private void loadContainerData() throws IOException {
}
private void handleCorruption(IOException e) {
- String errStr =
- "Corruption detected in container: [" + containerID + "] ";
- String logMessage = errStr + "Exception: [" + e.getMessage() + "]";
- LOG.error(logMessage);
+ LOG.error("Marking Container [{}] UNHEALTHY as it failed metadata check." +
+ " Exception: {}", containerID, e);
Review Comment:
```suggestion
LOG.error("Corruption detected in container [{}]. Marking it UNHEALTHY.", containerID, e);
```
Technically the metadata or the data (the actual blocks) may have been corrupted when this method is called. The exception should tell us which one it is. Also we can let the logger handle the exception printing directly without needing to pass it as a string format arg.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai commented on a diff in pull request #4169: HDDS-7097. Container scanner log output lacks useful information
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on code in PR #4169:
URL: https://github.com/apache/ozone/pull/4169#discussion_r1066779837
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueContainer.java:
##########
@@ -338,7 +337,7 @@ public void markContainerUnhealthy() throws StorageContainerException {
}
LOG.warn("Moving container {} to state {} from state:{} Trace:{}",
containerData.getContainerPath(), containerData.getState(),
- prevState, StringUtils.getStackTrace(Thread.currentThread()));
+ prevState);
Review Comment:
` Trace:{}` is no longer needed.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] errose28 merged pull request #4169: HDDS-7097. Container scanner log output lacks useful information
Posted by "errose28 (via GitHub)" <gi...@apache.org>.
errose28 merged PR #4169:
URL: https://github.com/apache/ozone/pull/4169
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] DaveTeng0 commented on a diff in pull request #4169: HDDS-7097. Container scanner log output lacks useful information
Posted by GitBox <gi...@apache.org>.
DaveTeng0 commented on code in PR #4169:
URL: https://github.com/apache/ozone/pull/4169#discussion_r1072965478
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueContainer.java:
##########
@@ -338,7 +337,7 @@ public void markContainerUnhealthy() throws StorageContainerException {
}
LOG.warn("Moving container {} to state {} from state:{} Trace:{}",
containerData.getContainerPath(), containerData.getState(),
- prevState, StringUtils.getStackTrace(Thread.currentThread()));
+ prevState);
Review Comment:
Ohh!! Sure yes it should be removed! Thanks for the check attila!
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] DaveTeng0 commented on a diff in pull request #4169: HDDS-7097. Container scanner log output lacks useful information
Posted by GitBox <gi...@apache.org>.
DaveTeng0 commented on code in PR #4169:
URL: https://github.com/apache/ozone/pull/4169#discussion_r1082808619
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueContainerCheck.java:
##########
@@ -398,9 +396,7 @@ private void loadContainerData() throws IOException {
}
private void handleCorruption(IOException e) {
- String errStr =
- "Corruption detected in container: [" + containerID + "] ";
- String logMessage = errStr + "Exception: [" + e.getMessage() + "]";
- LOG.error(logMessage);
+ LOG.error("Marking Container [{}] UNHEALTHY as it failed metadata check." +
+ " Exception: {}", containerID, e);
Review Comment:
ohh i see~ ok! Yeah will update the string format!!
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org