HDDS-9635. Trying to close a closed container from CLI results in ind…

…efinite retries

(cherry picked from commit bfafdfc98191054873f11ff94239bf1a2852e1a2)

hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/exceptions/SCMException.java

@@ -138,5 +138,6 @@ public enum ResultCodes {
     TIMEOUT,
     CA_ROTATION_IN_PROGRESS,
     CA_ROTATION_IN_POST_PROGRESS,
+    CONTAINER_ALREADY_CLOSED
   }
 }

hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/protocolPB/StorageContainerLocationProtocolClientSideTranslatorPB.java

@@ -576,8 +576,17 @@ public void closeContainer(long containerID) throws IOException {
         .setTraceID(TracingUtil.exportCurrentSpan())
         .setContainerID(containerID)
         .build();
-    submitRequest(Type.CloseContainer,
-        builder -> builder.setScmCloseContainerRequest(request));
+    StorageContainerLocationProtocolProtos.SCMCloseContainerResponseProto
+        response = submitRequest(Type.CloseContainer,
+          builder -> builder.setScmCloseContainerRequest(
+            request)).getScmCloseContainerResponse();
+    if (response.getErrorCode().equals(
+        StorageContainerLocationProtocolProtos.SCMCloseContainerResponseProto.
+            Error.CONTAINER_ALREADY_CLOSED)) {
+      String errorMessage =
+          String.format("Container %s already closed", containerID);
+      throw new IOException(errorMessage);
+    }
   }
 
   /**

hadoop-hdds/interface-admin/src/main/proto/ScmAdminProtocol.proto

@@ -302,7 +302,11 @@ message SCMCloseContainerRequestProto {
 }
 
 message SCMCloseContainerResponseProto {
-  // Empty response
+  enum Error {
+    OK = 1;
+    CONTAINER_ALREADY_CLOSED = 2;
+  }
+  required Error errorCode = 1;
 }
 
 /*

hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/protocol/StorageContainerLocationProtocolServerSideTranslatorPB.java

@@ -867,7 +867,17 @@ public NodeQueryResponseProto queryNode(
   public SCMCloseContainerResponseProto closeContainer(
       SCMCloseContainerRequestProto request)
       throws IOException {
-    impl.closeContainer(request.getContainerID());
+    try {
+      impl.closeContainer(request.getContainerID());
+    } catch (SCMException exception) {
+      if (exception.getResult()
+          .equals(SCMException.ResultCodes.CONTAINER_ALREADY_CLOSED)) {
+        return SCMCloseContainerResponseProto.newBuilder().setErrorCode(
+                SCMCloseContainerResponseProto.Error.CONTAINER_ALREADY_CLOSED)
+            .build();
+      }
+      throw exception;
+    }
     return SCMCloseContainerResponseProto.newBuilder().build();
   }
 

hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMClientProtocolServer.java

@@ -662,8 +662,12 @@ public void closeContainer(long containerID) throws IOException {
       final HddsProtos.LifeCycleState state = scm.getContainerManager()
           .getContainer(cid).getState();
       if (!state.equals(HddsProtos.LifeCycleState.OPEN)) {
+        ResultCodes resultCode = ResultCodes.UNEXPECTED_CONTAINER_STATE;
+        if (state.equals(HddsProtos.LifeCycleState.CLOSED)) {
+          resultCode = ResultCodes.CONTAINER_ALREADY_CLOSED;
+        }
         throw new SCMException("Cannot close a " + state + " container.",
-            ResultCodes.UNEXPECTED_CONTAINER_STATE);
+            resultCode);
       }
       scm.getEventQueue().fireEvent(SCMEvents.CLOSE_CONTAINER,
           ContainerID.valueOf(containerID));

hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/scm/TestCloseContainer.java

@@ -35,6 +35,7 @@
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
+import java.io.IOException;
 import java.time.Duration;
 import java.util.Set;
 import java.util.concurrent.TimeUnit;
@@ -50,6 +51,7 @@
 import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_STALENODE_INTERVAL;
 import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 
 /**
  * Integration test to ensure a container can be closed and its replicas
@@ -146,4 +148,22 @@ private Set<ContainerReplica> getContainerReplicas(ContainerInfo c) {
         "Unexpected exception while retrieving container replicas");
   }
 
+  @Test
+  public void testCloseClosedContainer()
+      throws Exception {
+    // Create some keys to write data into the open containers
+    for (int i = 0; i < 10; i++) {
+      TestDataUtil.createKey(bucket, "key" + i, ReplicationFactor.THREE,
+          ReplicationType.RATIS, "this is the content");
+    }
+    StorageContainerManager scm = cluster.getStorageContainerManager();
+    // Pick any container on the cluster and close it via client
+    ContainerInfo container = scm.getContainerManager().getContainers().get(0);
+    OzoneTestUtils.closeContainer(scm, container);
+    assertThrows(IOException.class,
+        () -> cluster.getStorageContainerLocationClient()
+            .closeContainer(container.getContainerID()),
+        "Container " + container.getContainerID() + " already closed");
+  }
+
 }