HDDS-13216. Standardize Container[Replica]NotFoundException messages.

hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerException.java

@@ -35,15 +35,6 @@ public ContainerException(String message) {
     super(message);
   }
 
-  /**
-   * Constructs an {@code ContainerException} with {@code null}
-   * as its error detail message.
-   * @param resultCode ResultCode for the exception
-   */
-  public ContainerException(ResultCodes resultCode) {
-    super(resultCode);
-  }
-
   /**
    * Constructs an {@code ContainerException} with the specified detail message.
    *

hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerNotFoundException.java

@@ -22,23 +22,20 @@
  */
 public class ContainerNotFoundException extends ContainerException {
 
-  /**
-   * Constructs an {@code ContainerNotFoundException} with {@code null}
-   * as its error detail message.
-   */
   public ContainerNotFoundException() {
-    super(ResultCodes.CONTAINER_NOT_FOUND);
+    this("Container not found for unknown id");
   }
 
-  /**
-   * Constructs an {@code ContainerNotFoundException} with the specified
-   * detail message.
-   *
-   * @param message
-   *        The detail message (which is saved for later retrieval
-   *        by the {@link #getMessage()} method)
-   */
+  /** Required by {@link org.apache.hadoop.ipc.RemoteException#unwrapRemoteException()}. */
   public ContainerNotFoundException(String message) {
     super(message, ResultCodes.CONTAINER_NOT_FOUND);
   }
+
+  public ContainerNotFoundException(ContainerID containerID) {
+    this("Container " +  containerID + " not found");
+  }
+
+  public static ContainerNotFoundException newInstanceForTesting() {
+    return new ContainerNotFoundException("For testing");
+  }
 }

hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerReplicaNotFoundException.java

@@ -17,6 +17,8 @@
 
 package org.apache.hadoop.hdds.scm.container;
 
+import org.apache.hadoop.hdds.protocol.DatanodeDetails;
+
 /**
  * Signals that a ContainerReplica is missing from the Container in
  * ContainerManager.
@@ -28,18 +30,16 @@ public class ContainerReplicaNotFoundException extends ContainerException {
    * as its error detail message.
    */
   public ContainerReplicaNotFoundException() {
-    super(ResultCodes.CONTAINER_REPLICA_NOT_FOUND);
+    this(null, null);
   }
 
-  /**
-   * Constructs an {@code ContainerReplicaNotFoundException} with the
-   * specified detail message.
-   *
-   * @param message
-   *        The detail message (which is saved for later retrieval
-   *        by the {@link #getMessage()} method)
-   */
+  /** Required by {@link org.apache.hadoop.ipc.RemoteException#unwrapRemoteException()}. */
   public ContainerReplicaNotFoundException(String message) {
     super(message, ResultCodes.CONTAINER_REPLICA_NOT_FOUND);
   }
+
+  public ContainerReplicaNotFoundException(ContainerID container, DatanodeDetails datanode) {
+    super("Replica not found for container " + container + " and datanode " + datanode,
+        ResultCodes.CONTAINER_REPLICA_NOT_FOUND);
+  }
 }

hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/ContainerController.java

@@ -29,6 +29,7 @@
 import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.ContainerDataProto.State;
 import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.ContainerType;
 import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.ContainerReportsProto;
+import org.apache.hadoop.hdds.scm.container.ContainerID;
 import org.apache.hadoop.hdds.scm.container.ContainerNotFoundException;
 import org.apache.hadoop.ozone.container.common.impl.ContainerData;
 import org.apache.hadoop.ozone.container.common.impl.ContainerSet;
@@ -93,7 +94,7 @@ public void markContainerForClose(final long containerId)
         warning = "The Container is not found. ContainerID: " + containerId;
       }
       LOG.warn(warning);
-      throw new ContainerNotFoundException(warning);
+      throw new ContainerNotFoundException(ContainerID.valueOf(containerId));
     } else {
       if (container.getContainerState() == State.OPEN) {
         getHandler(container).markContainerForClose(container);

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

@@ -26,7 +26,6 @@
 import java.util.List;
 import java.util.Map;
 import java.util.NavigableSet;
-import java.util.Optional;
 import java.util.Random;
 import java.util.Set;
 import java.util.concurrent.locks.Lock;
@@ -130,10 +129,11 @@ public void reinitialize(Table<ContainerID, ContainerInfo> containerStore)
   @Override
   public ContainerInfo getContainer(final ContainerID id)
       throws ContainerNotFoundException {
-    return Optional.ofNullable(containerStateManager
-        .getContainer(id))
-        .orElseThrow(() -> new ContainerNotFoundException("Container with id " +
-            id + " not found."));
+    final ContainerInfo info = containerStateManager.getContainer(id);
+    if (info == null) {
+      throw new ContainerNotFoundException(id);
+    }
+    return info;
   }
 
   @Override
@@ -274,7 +274,7 @@ public void updateContainerState(final ContainerID cid,
       if (containerExist(cid)) {
         containerStateManager.updateContainerState(protoId, event);
       } else {
-        throwContainerNotFoundException(cid);
+        throw new ContainerNotFoundException(cid);
       }
     } finally {
       lock.unlock();
@@ -289,7 +289,7 @@ public void transitionDeletingOrDeletedToClosedState(ContainerID containerID) th
       if (containerExist(containerID)) {
         containerStateManager.transitionDeletingOrDeletedToClosedState(proto);
       } else {
-        throwContainerNotFoundException(containerID);
+        throw new ContainerNotFoundException(containerID);
       }
     } finally {
       lock.unlock();
@@ -299,10 +299,11 @@ public void transitionDeletingOrDeletedToClosedState(ContainerID containerID) th
   @Override
   public Set<ContainerReplica> getContainerReplicas(final ContainerID id)
       throws ContainerNotFoundException {
-    return Optional.ofNullable(containerStateManager
-        .getContainerReplicas(id))
-        .orElseThrow(() -> new ContainerNotFoundException("Container with id " +
-            id + " not found."));
+    final Set<ContainerReplica> replicas = containerStateManager.getContainerReplicas(id);
+    if (replicas == null) {
+      throw new ContainerNotFoundException(id);
+    }
+    return replicas;
   }
 
   @Override
@@ -312,7 +313,7 @@ public void updateContainerReplica(final ContainerID cid,
     if (containerExist(cid)) {
       containerStateManager.updateContainerReplica(replica);
     } else {
-      throwContainerNotFoundException(cid);
+      throw new ContainerNotFoundException(cid);
     }
   }
 
@@ -323,7 +324,7 @@ public void removeContainerReplica(final ContainerID cid,
     if (containerExist(cid)) {
       containerStateManager.removeContainerReplica(replica);
     } else {
-      throwContainerNotFoundException(cid);
+      throw new ContainerNotFoundException(cid);
     }
   }
 
@@ -430,7 +431,7 @@ public void deleteContainer(final ContainerID cid)
       scmContainerManagerMetrics.incNumSuccessfulDeleteContainers();
     } else {
       scmContainerManagerMetrics.incNumFailureDeleteContainers();
-      throwContainerNotFoundException(cid);
+      throw new ContainerNotFoundException(cid);
     }
   }
 
@@ -439,12 +440,6 @@ public boolean containerExist(final ContainerID id) {
     return containerStateManager.contains(id);
   }
 
-  private void throwContainerNotFoundException(final ContainerID id)
-      throws ContainerNotFoundException {
-    throw new ContainerNotFoundException("Container with id " +
-        id + " not found.");
-  }
-
   @Override
   public void close() throws IOException {
     containerStateManager.close();

hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/MoveManager.java

@@ -425,7 +425,7 @@ private int getContainerReplicaIndex(
         //there should not be more than one replica of a container on the same
         //datanode. handle this if found in the future.
         .findFirst().orElseThrow(() ->
-            new ContainerReplicaNotFoundException("ID " + id + ", DN " + dn))
+            new ContainerReplicaNotFoundException(id, dn))
         .getReplicaIndex();
   }
 

hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/TestUnknownContainerReport.java

@@ -89,7 +89,7 @@ public void setup() throws IOException {
     this.publisher = mock(EventPublisher.class);
 
     when(containerManager.getContainer(any(ContainerID.class)))
-        .thenThrow(new ContainerNotFoundException());
+        .thenThrow(ContainerNotFoundException.newInstanceForTesting());
   }
 
   @AfterEach

hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/balancer/TestContainerBalancerDatanodeNodeLimit.java

@@ -553,7 +553,7 @@ public void checkIterationResultException(@Nonnull MockedSCM mockedSCM)
     when(mockedSCM.getMoveManager()
         .move(any(ContainerID.class), any(DatanodeDetails.class), any(DatanodeDetails.class)))
         .thenReturn(genCompletableFutureWithException(1))
-        .thenThrow(new ContainerNotFoundException("Test Container not found"))
+        .thenThrow(ContainerNotFoundException.newInstanceForTesting())
         .thenReturn(future);
 
     ContainerBalancerTask task = mockedSCM.startBalancerTask(config);

hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/balancer/TestMoveManager.java

@@ -323,7 +323,7 @@ public void testReplicationCommandFails() throws Exception {
   public void testDeleteCommandFails() throws Exception {
     CompletableFuture<MoveManager.MoveResult> res = setupSuccessfulMove();
 
-    doThrow(new ContainerNotFoundException("test"))
+    doThrow(ContainerNotFoundException.newInstanceForTesting())
         .when(containerManager).getContainer(any(ContainerID.class));
 
     ContainerReplicaOp op = new ContainerReplicaOp(

hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/pipeline/TestWritableECContainerProvider.java

@@ -447,7 +447,7 @@ public void testContainerNotFoundWhenAttemptingToUseExisting(
     // Ensure ContainerManager always throws when a container is requested so
     // existing pipelines cannot be used
     doAnswer(call -> {
-      throw new ContainerNotFoundException();
+      throw ContainerNotFoundException.newInstanceForTesting();
     }).when(containerManager).getContainer(any(ContainerID.class));
 
     ContainerInfo newContainer =

hadoop-ozone/integration-test/pom.xml

@@ -521,6 +521,11 @@
       <artifactId>hamcrest</artifactId>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>org.reflections</groupId>
+      <artifactId>reflections</artifactId>
+      <scope>test</scope>
+    </dependency>
     <dependency>
       <groupId>org.rocksdb</groupId>
       <artifactId>rocksdbjni</artifactId>

hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/TestRemoteEx.java

@@ -0,0 +1,55 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hdds;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import java.io.IOException;
+import org.apache.hadoop.hdds.scm.exceptions.SCMException;
+import org.apache.hadoop.ipc.RemoteException;
+import org.junit.jupiter.api.Test;
+import org.reflections.Reflections;
+
+/** Testing {@link RemoteException#unwrapRemoteException()}. */
+public class TestRemoteEx {
+  private static final Reflections REFLECTIONS = new Reflections(TestRemoteEx.class.getPackage().getName());
+
+  /** An exception without a single {@link String} parameter constructor. */
+  static class SomeException extends SCMException {
+    SomeException(ResultCodes result) {
+      super(result);
+    }
+  }
+
+  @Test
+  public void testSCMException() {
+    REFLECTIONS.getSubTypesOf(SCMException.class)
+        .forEach(TestRemoteEx::runUnwrappingRemoteException);
+  }
+
+  static void runUnwrappingRemoteException(Class<? extends Exception> clazz) {
+    final String message = clazz.getSimpleName() + "-message";
+    final RemoteException remoteException = new RemoteException(clazz.getName(), message);
+    System.out.println("Test      " + remoteException);
+    final IOException unwrapped = remoteException.unwrapRemoteException();
+    System.out.println("unwrapped " + unwrapped);
+    final Class<?> expected = clazz == SomeException.class ? RemoteException.class : clazz;
+    assertEquals(expected, unwrapped.getClass());
+    assertEquals(message, unwrapped.getMessage());
+  }
+}