[GitHub] [hadoop-ozone] nandakumar131 commented on a change in pull request #596: HDDS-3066. SCM crash during loading containers to DB.

[GitBox <gi...@apache.org>]

nandakumar131 commented on a change in pull request #596: HDDS-3066. SCM crash during loading containers to DB.
URL: https://github.com/apache/hadoop-ozone/pull/596#discussion_r385315139
 
 

 ##########
 File path: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/SCMContainerManager.java
 ##########
 @@ -117,9 +119,28 @@ private void loadExistingContainers() throws IOException {
           ContainerInfoProto.PARSER.parseFrom(entry.getValue()));
       Preconditions.checkNotNull(container);
       containerStateManager.loadContainer(container);
-      if (container.getState() == LifeCycleState.OPEN) {
-        pipelineManager.addContainerToPipeline(container.getPipelineID(),
-            ContainerID.valueof(container.getContainerID()));
+      try {
+        if (container.getState() == LifeCycleState.OPEN) {
+          pipelineManager.addContainerToPipeline(container.getPipelineID(),
+              ContainerID.valueof(container.getContainerID()));
+        }
+      } catch (PipelineNotFoundException ex) {
+        LOG.warn("Found a Container {} which is in {} state with pipeline {} " +
+                "that does not exist. Closing Container.", container,
+            container.getState(), container.getPipelineID());
+
+        // Not firing CLOSE_CONTAINER event because CloseContainer event
+        // handler is not registered by the time when we come
+        // here. So, we are calling update Container state to set
+        // container state to CLOSING, and later replication manager takes care
+        // of send close commands to datanode to close containers on the
+        // datanode.
+
+        // Skipping pipeline to container removal because, we got a
+        // pipelineNotFoundException when adding container to
+        // pipeline. So, we can only update container state.
 
 Review comment:
   We don't need this comment. As `SCMContainerManager` is the one which is taking this decision, it should not file an event to close the container. It is logical to directly update the container state.
   
   It is very explicit that we are performing this in case of `PipelineNotFoundException`, so there is no need to repeat it again.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org