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 2021/09/08 07:53:40 UTC

[GitHub] [ozone] guihecheng opened a new pull request #2624: HDDS-5726. Skip remove for already removed pipeline.

guihecheng opened a new pull request #2624:
URL: https://github.com/apache/ozone/pull/2624


   ## What changes were proposed in this pull request?
   
   Skip remove for already removed pipeline.
   Logs in the JIRA below.
   
   ## What is the link to the Apache JIRA
   
   https://issues.apache.org/jira/browse/HDDS-5726
   
   ## How was this patch tested?
   
   manual test.
   


-- 
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 change in pull request #2624: HDDS-5726. Skip remove for already removed pipeline.

Posted by GitBox <gi...@apache.org>.
sodonnel commented on a change in pull request #2624:
URL: https://github.com/apache/ozone/pull/2624#discussion_r704222763



##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/commandhandler/ClosePipelineCommandHandler.java
##########
@@ -68,12 +69,15 @@ public void handle(SCMCommand command, OzoneContainer ozoneContainer,
     final DatanodeDetails dn = context.getParent().getDatanodeDetails();
     ClosePipelineCommand closePipelineCommand = (ClosePipelineCommand) command;
     final PipelineID pipelineID = closePipelineCommand.getPipelineID();
+    final HddsProtos.PipelineID pipelineIdProto = pipelineID.getProtobuf();
 
     try {
       XceiverServerSpi server = ozoneContainer.getWriteChannel();
-      server.removeGroup(pipelineID.getProtobuf());
-      LOG.info("Close Pipeline {} command on datanode {}.", pipelineID,
-          dn.getUuidString());
+      if (server.isExist(pipelineIdProto)) {
+        server.removeGroup(pipelineIdProto);
+        LOG.info("Close Pipeline {} command on datanode {}.", pipelineID,
+            dn.getUuidString());
+      }

Review comment:
       I wonder if we should add an else branch for debugging (just in case we need it):
   
   ```
   } else {
     LOG.debug("Ignoring close pipeline command for pipeline {} as it does not exist", pipelineID);
   }
   ```




-- 
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] guihecheng commented on a change in pull request #2624: HDDS-5726. Skip remove for already removed pipeline.

Posted by GitBox <gi...@apache.org>.
guihecheng commented on a change in pull request #2624:
URL: https://github.com/apache/ozone/pull/2624#discussion_r704242484



##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/commandhandler/ClosePipelineCommandHandler.java
##########
@@ -68,12 +69,15 @@ public void handle(SCMCommand command, OzoneContainer ozoneContainer,
     final DatanodeDetails dn = context.getParent().getDatanodeDetails();
     ClosePipelineCommand closePipelineCommand = (ClosePipelineCommand) command;
     final PipelineID pipelineID = closePipelineCommand.getPipelineID();
+    final HddsProtos.PipelineID pipelineIdProto = pipelineID.getProtobuf();
 
     try {
       XceiverServerSpi server = ozoneContainer.getWriteChannel();
-      server.removeGroup(pipelineID.getProtobuf());
-      LOG.info("Close Pipeline {} command on datanode {}.", pipelineID,
-          dn.getUuidString());
+      if (server.isExist(pipelineIdProto)) {
+        server.removeGroup(pipelineIdProto);
+        LOG.info("Close Pipeline {} command on datanode {}.", pipelineID,
+            dn.getUuidString());
+      }

Review comment:
       @sodonnel this makes sense since this is a special case, will update, 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] sodonnel merged pull request #2624: HDDS-5726. Skip remove for already removed pipeline.

Posted by GitBox <gi...@apache.org>.
sodonnel merged pull request #2624:
URL: https://github.com/apache/ozone/pull/2624


   


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