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 2022/06/16 02:18:43 UTC

[GitHub] [ozone] swamirishi opened a new pull request, #3522: HDDS-6899: Fix Logging

swamirishi opened a new pull request, #3522:
URL: https://github.com/apache/ozone/pull/3522

   ## What changes were proposed in this pull request?
   
   Exception stacktrace put in debug level instead of warn or error level. 
   
   ## What is the link to the Apache JIRA
   https://issues.apache.org/jira/browse/HDDS-6899
   ## How was this patch tested?
   Logging Changes, no change in logic.


-- 
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] umamaheswararao merged pull request #3522: HDDS-6899. [EC] Remove warnings and errors from console during online reconstruction of data.

Posted by GitBox <gi...@apache.org>.
umamaheswararao merged PR #3522:
URL: https://github.com/apache/ozone/pull/3522


-- 
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] umamaheswararao commented on pull request #3522: HDDS-6899. [EC] Remove warnings and errors from console during online reconstruction of data.

Posted by GitBox <gi...@apache.org>.
umamaheswararao commented on PR #3522:
URL: https://github.com/apache/ozone/pull/3522#issuecomment-1160732846

   Please run the local checkstyle before posting the PR.


-- 
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] umamaheswararao commented on pull request #3522: HDDS-6899. [EC] Remove warnings and errors from console during online reconstruction of data.

Posted by GitBox <gi...@apache.org>.
umamaheswararao commented on PR #3522:
URL: https://github.com/apache/ozone/pull/3522#issuecomment-1157810578

   I have just re triggered the CI. Did you get chance to check how these messages appearing in this he console with these changes?


-- 
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] umamaheswararao commented on pull request #3522: HDDS-6899. [EC] Remove warnings and errors from console during online reconstruction of data.

Posted by GitBox <gi...@apache.org>.
umamaheswararao commented on PR #3522:
URL: https://github.com/apache/ozone/pull/3522#issuecomment-1161139032

   Congratulations @swamirishi for your first commit :-)
   Thanks for your contribution.


-- 
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] JyotinderSingh commented on pull request #3522: HDDS-6899. [EC] Remove warnings and errors from console while reconstructing data.

Posted by GitBox <gi...@apache.org>.
JyotinderSingh commented on PR #3522:
URL: https://github.com/apache/ozone/pull/3522#issuecomment-1157274475

   Hi @swamirishi, the title of the pull request should be the same as that of the Jira ticket. I have modified it to fit the requirement.


-- 
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] umamaheswararao commented on a diff in pull request #3522: HDDS-6899. [EC] Remove warnings and errors from console during online reconstruction of data.

Posted by GitBox <gi...@apache.org>.
umamaheswararao commented on code in PR #3522:
URL: https://github.com/apache/ozone/pull/3522#discussion_r899229798


##########
hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/XceiverClientGrpc.java:
##########
@@ -280,7 +280,10 @@ public ContainerCommandResponseProto sendCommand(
       // Re-interrupt the thread while catching InterruptedException
       Thread.currentThread().interrupt();
     } catch (ExecutionException e) {
-      LOG.error("Failed to execute command {}", processForDebug(request), e);
+      LOG.error("Failed to execute command {}." +
+                      "Exception Class: {}, Exception Message: {}",
+              request.getCmdType(), e.getClass().getName(), e.getMessage());

Review Comment:
   How about "if isDebugEnabled, we can log debug one, otherwise error?"
   in that case, debug logging also should have some details.



-- 
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] umamaheswararao commented on a diff in pull request #3522: HDDS-6899. [EC] Remove warnings and errors from console during online reconstruction of data.

Posted by GitBox <gi...@apache.org>.
umamaheswararao commented on code in PR #3522:
URL: https://github.com/apache/ozone/pull/3522#discussion_r901923356


##########
hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/XceiverClientGrpc.java:
##########
@@ -280,7 +280,13 @@ public ContainerCommandResponseProto sendCommand(
       // Re-interrupt the thread while catching InterruptedException
       Thread.currentThread().interrupt();
     } catch (ExecutionException e) {
-      LOG.error("Failed to execute command {}", processForDebug(request), e);
+      String message = "Failed to execute command {}.";
+      if (LOG.isDebugEnabled()) {
+        LOG.debug(message, processForDebug(request), e);
+      } else {

Review Comment:
   Nit: You need to give space before "Exception Class". 



##########
hadoop-hdds/client/src/main/java/org/apache/hadoop/ozone/client/io/ECBlockInputStreamProxy.java:
##########
@@ -165,8 +165,15 @@ public synchronized int read(ByteBuffer buf) throws IOException {
         throw e;
       }
       if (e instanceof BadDataLocationException) {
-        LOG.warn("Failing over to reconstruction read due to an error in " +
-            "ECBlockReader", e);
+        String message = "Failing over to reconstruction read due" +
+                " to an error in ECBlockReader.";
+        if(LOG.isDebugEnabled()) {
+          LOG.debug(message, e);
+        } else {

Review Comment:
   Nit: Give space after {}



-- 
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] umamaheswararao commented on a diff in pull request #3522: HDDS-6899. [EC] Remove warnings and errors from console during online reconstruction of data.

Posted by GitBox <gi...@apache.org>.
umamaheswararao commented on code in PR #3522:
URL: https://github.com/apache/ozone/pull/3522#discussion_r901977620


##########
hadoop-hdds/client/src/main/java/org/apache/hadoop/ozone/client/io/ECBlockInputStreamProxy.java:
##########
@@ -165,8 +165,15 @@ public synchronized int read(ByteBuffer buf) throws IOException {
         throw e;
       }
       if (e instanceof BadDataLocationException) {
-        LOG.warn("Failing over to reconstruction read due to an error in " +
-            "ECBlockReader", e);
+        String message = "Failing over to reconstruction read due" +
+                " to an error in ECBlockReader.";
+        if(LOG.isDebugEnabled()) {
+          LOG.debug(message, e);
+        } else {

Review Comment:
   Ok, I see, please ignore this one. comma is out side of string. Thanks



-- 
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] umamaheswararao commented on pull request #3522: HDDS-6899. [EC] Remove warnings and errors from console during online reconstruction of data.

Posted by GitBox <gi...@apache.org>.
umamaheswararao commented on PR #3522:
URL: https://github.com/apache/ozone/pull/3522#issuecomment-1159301309

   @swamirishi you may want to run the checkstyle locally first to know if any checkstyle warnings.
   cd hadoop-ozone/dev-support/checks
   ./checkstyle.sh
   


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