HDFS-13013. Fix closeContainer API with the right container state change. Contributed by Xiaoyu Yao.

4 files changed, 42 insertions, 74 deletions

diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/ozone/protocolPB/StorageContainerLocationProtocolServerSideTranslatorPB.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/ozone/protocolPB/StorageContainerLocationProtocolServerSideTranslatorPB.java
index ae99a373c1..c8258b9dcb 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/ozone/protocolPB/StorageContainerLocationProtocolServerSideTranslatorPB.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/ozone/protocolPB/StorageContainerLocationProtocolServerSideTranslatorPB.java
@@ -33,8 +33,6 @@ import org.apache.hadoop.scm.ScmInfo;
 import org.apache.hadoop.scm.container.common.helpers.ContainerInfo;
 import org.apache.hadoop.scm.protocol.StorageContainerLocationProtocol;
 
-import org.apache.hadoop.ozone.protocol.proto.StorageContainerLocationProtocolProtos.CloseContainerRequestProto;
-import org.apache.hadoop.ozone.protocol.proto.StorageContainerLocationProtocolProtos.CloseContainerResponseProto;
 import org.apache.hadoop.ozone.protocol.proto.StorageContainerLocationProtocolProtos.ContainerRequestProto;
 import org.apache.hadoop.ozone.protocol.proto.StorageContainerLocationProtocolProtos.ContainerResponseProto;
 import org.apache.hadoop.ozone.protocol.proto.StorageContainerLocationProtocolProtos.GetContainerRequestProto;
@@ -43,8 +41,8 @@ import org.apache.hadoop.ozone.protocol.proto.StorageContainerLocationProtocolPr
 import org.apache.hadoop.ozone.protocol.proto.StorageContainerLocationProtocolProtos.DeleteContainerResponseProto;
 import org.apache.hadoop.ozone.protocol.proto.StorageContainerLocationProtocolProtos.ListContainerResponseProto;
 import org.apache.hadoop.ozone.protocol.proto.StorageContainerLocationProtocolProtos.ListContainerRequestProto;
-import org.apache.hadoop.ozone.protocol.proto.StorageContainerLocationProtocolProtos.NotifyObjectCreationStageRequestProto;
-import org.apache.hadoop.ozone.protocol.proto.StorageContainerLocationProtocolProtos.NotifyObjectCreationStageResponseProto;
+import org.apache.hadoop.ozone.protocol.proto.StorageContainerLocationProtocolProtos.ObjectStageChangeRequestProto;
+import org.apache.hadoop.ozone.protocol.proto.StorageContainerLocationProtocolProtos.ObjectStageChangeResponseProto;
 import org.apache.hadoop.ozone.protocol.proto.StorageContainerLocationProtocolProtos.PipelineResponseProto;
 import org.apache.hadoop.ozone.protocol.proto.StorageContainerLocationProtocolProtos.PipelineRequestProto;
 import org.apache.hadoop.scm.container.common.helpers.Pipeline;
@@ -166,31 +164,19 @@ public final class StorageContainerLocationProtocolServerSideTranslatorPB
   }
 
   @Override
-  public NotifyObjectCreationStageResponseProto notifyObjectCreationStage(
-      RpcController controller, NotifyObjectCreationStageRequestProto request)
+  public ObjectStageChangeResponseProto notifyObjectStageChange(
+      RpcController controller, ObjectStageChangeRequestProto request)
       throws ServiceException {
     try {
-      impl.notifyObjectCreationStage(request.getType(), request.getName(),
-          request.getStage());
-      return NotifyObjectCreationStageResponseProto.newBuilder().build();
+      impl.notifyObjectStageChange(request.getType(), request.getName(),
+          request.getOp(), request.getStage());
+      return ObjectStageChangeResponseProto.newBuilder().build();
     } catch (IOException e) {
       throw new ServiceException(e);
     }
   }
 
   @Override
-  public CloseContainerResponseProto closeContainer(
-      RpcController controller, CloseContainerRequestProto request)
-      throws ServiceException {
-    try {
-      impl.closeContainer(request.getContainerName());
-      return CloseContainerResponseProto.newBuilder().build();
-    } catch (IOException ioe) {
-      throw new ServiceException(ioe);
-    }
-  }
-
-  @Override
   public PipelineResponseProto allocatePipeline(
       RpcController controller, PipelineRequestProto request)
       throws ServiceException {
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/ozone/scm/StorageContainerManager.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/ozone/scm/StorageContainerManager.java
index ca1fa4865f..3276db8d4a 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/ozone/scm/StorageContainerManager.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/ozone/scm/StorageContainerManager.java
@@ -68,7 +68,7 @@ import org.apache.hadoop.ozone.protocol.proto.StorageContainerDatanodeProtocolPr
 import org.apache.hadoop.ozone.protocol.proto.StorageContainerDatanodeProtocolProtos.ContainerBlocksDeletionACKProto;
 import org.apache.hadoop.ozone.protocol.proto.StorageContainerDatanodeProtocolProtos.ContainerBlocksDeletionACKResponseProto;
 import org.apache.hadoop.ozone.protocol.proto.StorageContainerLocationProtocolProtos;
-import org.apache.hadoop.ozone.protocol.proto.StorageContainerLocationProtocolProtos.NotifyObjectCreationStageRequestProto;
+import org.apache.hadoop.ozone.protocol.proto.StorageContainerLocationProtocolProtos.ObjectStageChangeRequestProto;
 import org.apache.hadoop.ozone.protocolPB.ScmBlockLocationProtocolServerSideTranslatorPB;
 import org.apache.hadoop.ozone.protocolPB.StorageContainerDatanodeProtocolPB;
 import org.apache.hadoop.ozone.protocolPB.StorageContainerDatanodeProtocolServerSideTranslatorPB;
@@ -671,31 +671,42 @@ public class StorageContainerManager extends ServiceRuntimeInfoImpl
   }
 
   /**
-   * Notify from client when begin/finish creating container/pipeline objects
-   * on datanodes.
+   * Notify from client when begin/finish operation for container/pipeline
+   * objects on datanodes.
    * @param type
    * @param name
+   * @param op
    * @param stage
    */
   @Override
-  public void notifyObjectCreationStage(
-      NotifyObjectCreationStageRequestProto.Type type, String name,
-      NotifyObjectCreationStageRequestProto.Stage stage) throws IOException {
-
-    if (type == NotifyObjectCreationStageRequestProto.Type.container) {
-      ContainerInfo info = scmContainerManager.getContainer(name);
-      LOG.info("Container {} current state {} new stage {}", name,
-          info.getState(), stage);
-      if (stage == NotifyObjectCreationStageRequestProto.Stage.begin) {
-        scmContainerManager.updateContainerState(name,
-            OzoneProtos.LifeCycleEvent.CREATE);
-      } else {
-        scmContainerManager.updateContainerState(name,
-            OzoneProtos.LifeCycleEvent.CREATED);
+  public void notifyObjectStageChange(
+      ObjectStageChangeRequestProto.Type type, String name,
+      ObjectStageChangeRequestProto.Op op,
+      ObjectStageChangeRequestProto.Stage stage) throws IOException {
+
+    LOG.info("Object type {} name {} op {} new stage {}",
+        type, name, op, stage);
+    if (type == ObjectStageChangeRequestProto.Type.container) {
+      if (op == ObjectStageChangeRequestProto.Op.create) {
+        if (stage == ObjectStageChangeRequestProto.Stage.begin) {
+          scmContainerManager.updateContainerState(name,
+              OzoneProtos.LifeCycleEvent.CREATE);
+        } else {
+          scmContainerManager.updateContainerState(name,
+              OzoneProtos.LifeCycleEvent.CREATED);
+        }
+      } else if (op == ObjectStageChangeRequestProto.Op.close) {
+        if (stage == ObjectStageChangeRequestProto.Stage.begin) {
+          scmContainerManager.updateContainerState(name,
+              OzoneProtos.LifeCycleEvent.FINALIZE);
+        } else {
+          scmContainerManager.updateContainerState(name,
+              OzoneProtos.LifeCycleEvent.CLOSE);
+        }
       }
-    } else if (type == NotifyObjectCreationStageRequestProto.Type.pipeline) {
-      // TODO: pipeline state update will be addressed in future patch.
-    }
+    } //else if (type == ObjectStageChangeRequestProto.Type.pipeline) {
+    // TODO: pipeline state update will be addressed in future patch.
+    //}
   }
 
   /**
@@ -711,12 +722,6 @@ public class StorageContainerManager extends ServiceRuntimeInfoImpl
     return null;
   }
 
-  @Override
-  public void closeContainer(String containerName) throws IOException {
-    checkAdminAccess();
-    scmContainerManager.closeContainer(containerName);
-  }
-
   /**
    * Queries a list of Node that match a set of statuses.
    * <p>
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/ozone/scm/container/ContainerMapping.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/ozone/scm/container/ContainerMapping.java
index 79974d9ca7..c8875ba06f 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/ozone/scm/container/ContainerMapping.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/ozone/scm/container/ContainerMapping.java
@@ -255,25 +255,6 @@ public class ContainerMapping implements Mapping {
     }
   }
 
-  @Override
-  public void closeContainer(String containerName) throws IOException {
-    lock.lock();
-    try {
-      OzoneProtos.LifeCycleState newState =
-          updateContainerState(containerName, OzoneProtos.LifeCycleEvent.CLOSE);
-      if (newState != OzoneProtos.LifeCycleState.CLOSED) {
-        throw new SCMException(
-            "Failed to close container "
-                + containerName
-                + ", reason : container in state "
-                + newState,
-            SCMException.ResultCodes.UNEXPECTED_CONTAINER_STATE);
-      }
-    } finally {
-      lock.unlock();
-    }
-  }
-
   /** {@inheritDoc} Used by client to update container state on SCM. */
   @Override
   public OzoneProtos.LifeCycleState updateContainerState(
@@ -313,6 +294,10 @@ public class ContainerMapping implements Mapping {
         containerLeaseManager.release(containerInfo);
         break;
       case FINALIZE:
+        // TODO: we don't need a lease manager here for closing as the
+        // container report will include the container state after HDFS-13008
+        // If a client failed to update the container close state, DN container
+        // report from 3 DNs will be used to close the container eventually.
         break;
       case CLOSE:
         break;
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/ozone/scm/container/Mapping.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/ozone/scm/container/Mapping.java
index 5d2518ee13..577571f0fe 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/ozone/scm/container/Mapping.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/ozone/scm/container/Mapping.java
@@ -84,14 +84,6 @@ public interface Mapping extends Closeable {
   void deleteContainer(String containerName) throws IOException;
 
   /**
-   * Close a container.
-   *
-   * @param containerName - name of the container to close.
-   * @throws IOException
-   */
-  void closeContainer(String containerName) throws IOException;
-
-  /**
    * Update container state.
    * @param containerName - Container Name
    * @param event - container life cycle event