diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerException.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerException.java index 2d45b155adb1..91c129fbb459 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerException.java +++ b/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. * diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerNotFoundException.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerNotFoundException.java index 3da1025572e5..ab15f4f3f859 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerNotFoundException.java +++ b/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"); + } } diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerReplicaNotFoundException.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerReplicaNotFoundException.java index 1fa36ca9eb24..5c1eac42a796 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerReplicaNotFoundException.java +++ b/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); + } } diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/ContainerController.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/ContainerController.java index 451b369733b2..e315e1bf4aff 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/ContainerController.java +++ b/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); diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerManagerImpl.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerManagerImpl.java index 2e6bfa6f3dd3..81de8f8b5cab 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerManagerImpl.java +++ b/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 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 getContainerReplicas(final ContainerID id) throws ContainerNotFoundException { - return Optional.ofNullable(containerStateManager - .getContainerReplicas(id)) - .orElseThrow(() -> new ContainerNotFoundException("Container with id " + - id + " not found.")); + final Set 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(); diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/MoveManager.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/MoveManager.java index 4283f1813182..e04688ab2cc5 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/MoveManager.java +++ b/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(); } diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/TestUnknownContainerReport.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/TestUnknownContainerReport.java index 369d1142ea02..705d2fca8ff1 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/TestUnknownContainerReport.java +++ b/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 diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/balancer/TestContainerBalancerDatanodeNodeLimit.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/balancer/TestContainerBalancerDatanodeNodeLimit.java index 872a7469d6a9..d5ecbb34e62c 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/balancer/TestContainerBalancerDatanodeNodeLimit.java +++ b/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); diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/balancer/TestMoveManager.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/balancer/TestMoveManager.java index b141fd82753e..dc645ab0e8d1 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/balancer/TestMoveManager.java +++ b/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 res = setupSuccessfulMove(); - doThrow(new ContainerNotFoundException("test")) + doThrow(ContainerNotFoundException.newInstanceForTesting()) .when(containerManager).getContainer(any(ContainerID.class)); ContainerReplicaOp op = new ContainerReplicaOp( diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/pipeline/TestWritableECContainerProvider.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/pipeline/TestWritableECContainerProvider.java index 5e9ff0a01008..090fe5f1df08 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/pipeline/TestWritableECContainerProvider.java +++ b/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 = diff --git a/hadoop-ozone/integration-test/pom.xml b/hadoop-ozone/integration-test/pom.xml index 198f03f20789..22906c611add 100644 --- a/hadoop-ozone/integration-test/pom.xml +++ b/hadoop-ozone/integration-test/pom.xml @@ -521,6 +521,11 @@ hamcrest test + + org.reflections + reflections + test + org.rocksdb rocksdbjni diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/TestRemoteEx.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/TestRemoteEx.java new file mode 100644 index 000000000000..95a17b8720ce --- /dev/null +++ b/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 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()); + } +}