You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "ashishkumar50 (via GitHub)" <gi...@apache.org> on 2023/08/28 07:17:17 UTC

[GitHub] [ozone] ashishkumar50 opened a new pull request, #5226: HDDS-9216. Update log in exception while processing ICR for container.

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

   ## What changes were proposed in this pull request?
   
   Not leader log was changed in [HDDS-6241](https://issues.apache.org/jira/browse/HDDS-6241) when there is exception during ICR. But in [HDDS-8934](https://issues.apache.org/jira/browse/HDDS-8934) not leader exception is wrapped inside SCMException. Now it is not getting caught in `NotLeaderException`. In this PR updating the log for SCM_NOT_LEADER exception.
   
   ## What is the link to the Apache JIRA
   
   https://issues.apache.org/jira/browse/HDDS-9216
   
   ## How was this patch tested?
   
   Just logging changed, no test impact.
   


-- 
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 #5226: HDDS-9216. Update log in exception while processing ICR for container.

Posted by "sodonnel (via GitHub)" <gi...@apache.org>.
sodonnel commented on code in PR #5226:
URL: https://github.com/apache/ozone/pull/5226#discussion_r1308659542


##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/IncrementalContainerReportHandler.java:
##########
@@ -108,9 +108,14 @@ public void onMessage(final IncrementalContainerReportFromDatanode report,
         } catch (ContainerReplicaNotFoundException e) {
           LOG.warn("Container {} replica not found!",
               replicaProto.getContainerID());
-        } catch (NotLeaderException nle) {
-          LOG.warn("Failed to process " + replicaProto.getState()
-              + " Container " + id + " due to " + nle);
+        } catch (SCMException ex) {
+          if (ex.getResult() == SCMException.ResultCodes.SCM_NOT_LEADER) {
+            LOG.warn("Failed to process " + replicaProto.getState()

Review Comment:
   Can we change this to the log placeholder syntax?
   
   ```
   LOG.warn("Failed to process {} container {}", replicaProto.getState(), id, ex);
   ```
   
   I know it was like this before you made this change, but we may as well tidy it up as part of this change.



-- 
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] ashishkumar50 commented on a diff in pull request #5226: HDDS-9216. Update log in exception while processing ICR for container.

Posted by "ashishkumar50 (via GitHub)" <gi...@apache.org>.
ashishkumar50 commented on code in PR #5226:
URL: https://github.com/apache/ozone/pull/5226#discussion_r1308691544


##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/IncrementalContainerReportHandler.java:
##########
@@ -108,9 +108,14 @@ public void onMessage(final IncrementalContainerReportFromDatanode report,
         } catch (ContainerReplicaNotFoundException e) {
           LOG.warn("Container {} replica not found!",
               replicaProto.getContainerID());
-        } catch (NotLeaderException nle) {
-          LOG.warn("Failed to process " + replicaProto.getState()
-              + " Container " + id + " due to " + nle);
+        } catch (SCMException ex) {
+          if (ex.getResult() == SCMException.ResultCodes.SCM_NOT_LEADER) {
+            LOG.warn("Failed to process " + replicaProto.getState()

Review Comment:
   Done



-- 
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] sumitagrawl merged pull request #5226: HDDS-9216. Update log in exception while processing ICR for container.

Posted by "sumitagrawl (via GitHub)" <gi...@apache.org>.
sumitagrawl merged PR #5226:
URL: https://github.com/apache/ozone/pull/5226


-- 
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] ashishkumar50 commented on a diff in pull request #5226: HDDS-9216. Update log in exception while processing ICR for container.

Posted by "ashishkumar50 (via GitHub)" <gi...@apache.org>.
ashishkumar50 commented on code in PR #5226:
URL: https://github.com/apache/ozone/pull/5226#discussion_r1309694714


##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/IncrementalContainerReportHandler.java:
##########
@@ -108,9 +108,14 @@ public void onMessage(final IncrementalContainerReportFromDatanode report,
         } catch (ContainerReplicaNotFoundException e) {
           LOG.warn("Container {} replica not found!",
               replicaProto.getContainerID());
-        } catch (NotLeaderException nle) {
-          LOG.warn("Failed to process " + replicaProto.getState()
-              + " Container " + id + " due to " + nle);
+        } catch (SCMException ex) {
+          if (ex.getResult() == SCMException.ResultCodes.SCM_NOT_LEADER) {
+            LOG.warn("Failed to process {} container {}",
+                replicaProto.getState(), id, ex);

Review Comment:
   Updated 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] ashishkumar50 commented on pull request #5226: HDDS-9216. Update log in exception while processing ICR for container.

Posted by "ashishkumar50 (via GitHub)" <gi...@apache.org>.
ashishkumar50 commented on PR #5226:
URL: https://github.com/apache/ozone/pull/5226#issuecomment-1695161044

   @szetszwo, Please help to review.


-- 
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] szetszwo commented on a diff in pull request #5226: HDDS-9216. Update log in exception while processing ICR for container.

Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on code in PR #5226:
URL: https://github.com/apache/ozone/pull/5226#discussion_r1309271116


##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/IncrementalContainerReportHandler.java:
##########
@@ -108,9 +108,14 @@ public void onMessage(final IncrementalContainerReportFromDatanode report,
         } catch (ContainerReplicaNotFoundException e) {
           LOG.warn("Container {} replica not found!",
               replicaProto.getContainerID());
-        } catch (NotLeaderException nle) {
-          LOG.warn("Failed to process " + replicaProto.getState()
-              + " Container " + id + " due to " + nle);
+        } catch (SCMException ex) {
+          if (ex.getResult() == SCMException.ResultCodes.SCM_NOT_LEADER) {
+            LOG.warn("Failed to process {} container {}",
+                replicaProto.getState(), id, ex);

Review Comment:
   @ashishkumar50 , thanks for working on this!
   
   Please print only the exception message but not the stack trace. #4302 was to avoid printing the stack trace.



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