diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/TestContainerReportHandling.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/TestContainerReportHandling.java index ad0925913412..00f912d8ddd6 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/TestContainerReportHandling.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/TestContainerReportHandling.java @@ -38,8 +38,8 @@ import org.apache.hadoop.fs.FileUtil; import org.apache.hadoop.hdds.client.ReplicationConfig; import org.apache.hadoop.hdds.conf.OzoneConfiguration; -import org.apache.hadoop.hdds.protocol.DatanodeDetails; import org.apache.hadoop.hdds.protocol.proto.HddsProtos; +import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.ContainerReplicaProto; import org.apache.hadoop.hdds.scm.container.ContainerID; import org.apache.hadoop.hdds.scm.container.ContainerManager; import org.apache.hadoop.hdds.scm.container.ContainerNotFoundException; @@ -79,8 +79,8 @@ private static Stream delStatesAndReplication() { * Tests that a DELETING (or DELETED) container replica gets deleted when replica bcsid <= container bcsid * applicable to RATIS; EC ignores bcsid. * To do this, the test first creates a key and closes its corresponding container. Then it moves that container to - * DELETING (or DELETED) state using ContainerManager. Then it restarts a Datanode hosting that container, - * making it send a full container report. + * DELETING (or DELETED) state using ContainerManager. SCM then deletes the replicas when it processes a periodic + * container report for the CLOSED replicas. * Tests wait for a DELETING (or DELETED) container replica gets deleted based on the bcsid comparison. */ @ParameterizedTest @@ -111,8 +111,15 @@ void testDeletingOrDeletedContainerWhenNonEmptyReplicaIsReported( // also wait till the container is closed in SCM waitForContainerStateInSCM(cluster.getStorageContainerManager(), containerID, HddsProtos.LifeCycleState.CLOSED); - // move the container to DELETING ContainerManager containerManager = cluster.getStorageContainerManager().getContainerManager(); + // Wait until SCM sees all replicas CLOSED before moving the container to DELETING. The container state above + // flips to CLOSED as soon as the first replica is reported CLOSED, so a lagging replica may still be CLOSING in + // SCM. Deleting then races with that lagging CLOSING report, which would resurrect the container out of + // DELETING/DELETED and the replicas would never be deleted. + TestHelper.waitForReplicaState(containerManager, containerID, replicationInput.getNumDatanodes(), + ContainerReplicaProto.State.CLOSED); + + // move the container to DELETING assertFalse(containerManager.getContainerReplicas(containerID).isEmpty()); containerManager.updateContainerState(containerID, HddsProtos.LifeCycleEvent.DELETE); assertEquals(HddsProtos.LifeCycleState.DELETING, containerManager.getContainer(containerID).getState()); @@ -123,14 +130,9 @@ void testDeletingOrDeletedContainerWhenNonEmptyReplicaIsReported( assertEquals(HddsProtos.LifeCycleState.DELETED, containerManager.getContainer(containerID).getState()); } - // restart all the DNs - List dnlist = keyLocation.getPipeline().getNodes(); - for (DatanodeDetails dn: dnlist) { - cluster.restartHddsDatanode(dn, false); - } - - // Since replica state is CLOSED and container is DELETED/DELETING in SCM - // bcsid of replica and container is same, SCM will trigger delete replica for RATIS, while EC ignores bcsid + // Since replica state is CLOSED and container is DELETED/DELETING in SCM, and the bcsid of replica and + // container is same, SCM will trigger delete replica for RATIS (EC ignores bcsid) when it processes a + // periodic container report for the CLOSED replicas. // wait for all replica to be deleted GenericTestUtils.waitFor(() -> { try { diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/TestContainerReportHandlingWithHA.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/TestContainerReportHandlingWithHA.java index ff4455321f56..d68880ddd361 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/TestContainerReportHandlingWithHA.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/TestContainerReportHandlingWithHA.java @@ -39,8 +39,8 @@ import org.apache.hadoop.fs.FileUtil; import org.apache.hadoop.hdds.client.ReplicationConfig; import org.apache.hadoop.hdds.conf.OzoneConfiguration; -import org.apache.hadoop.hdds.protocol.DatanodeDetails; import org.apache.hadoop.hdds.protocol.proto.HddsProtos; +import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.ContainerReplicaProto; import org.apache.hadoop.hdds.scm.container.ContainerID; import org.apache.hadoop.hdds.scm.container.ContainerManager; import org.apache.hadoop.hdds.scm.container.ContainerNotFoundException; @@ -82,8 +82,8 @@ private static Stream delStatesAndReplication() { * Tests that a DELETING (or DELETED) container replica gets deleted when replica bcsid <= container bcsid * applicable to RATIS; EC ignores bcsid. * To do this, the test first creates a key and closes its corresponding container. Then it moves that container to - * DELETING (or DELETED) state using ContainerManager. Then it restarts Datanodes hosting that container, - * making it send a full container report. + * DELETING (or DELETED) state using ContainerManager. SCM then deletes the replicas when it processes a periodic + * container report for the CLOSED replicas. * Tests wait for a DELETING (or DELETED) container replica gets deleted based on the bcsid comparison. */ @ParameterizedTest @@ -114,8 +114,15 @@ void testDeletingOrDeletedContainerWhenNonEmptyReplicaIsReportedWithScmHA( waitForContainerStateInAllSCMs(cluster, containerID, HddsProtos.LifeCycleState.CLOSED); - // move the container to DELETING ContainerManager containerManager = cluster.getScmLeader().getContainerManager(); + // Wait until SCM sees all replicas CLOSED before moving the container to DELETING. The container state above + // flips to CLOSED as soon as the first replica is reported CLOSED, so a lagging replica may still be CLOSING in + // SCM. Deleting then races with that lagging CLOSING report, which would resurrect the container out of + // DELETING/DELETED and the replicas would never be deleted. + TestHelper.waitForReplicaState(containerManager, containerID, replicationInput.getNumDatanodes(), + ContainerReplicaProto.State.CLOSED); + + // move the container to DELETING assertFalse(containerManager.getContainerReplicas(containerID).isEmpty()); containerManager.updateContainerState(containerID, HddsProtos.LifeCycleEvent.DELETE); assertEquals(HddsProtos.LifeCycleState.DELETING, containerManager.getContainer(containerID).getState()); @@ -126,14 +133,9 @@ void testDeletingOrDeletedContainerWhenNonEmptyReplicaIsReportedWithScmHA( assertEquals(HddsProtos.LifeCycleState.DELETED, containerManager.getContainer(containerID).getState()); } - // restart all the DNs - List dnlist = keyLocation.getPipeline().getNodes(); - for (DatanodeDetails dn: dnlist) { - cluster.restartHddsDatanode(dn, false); - } - - // Since replica state is CLOSED and container is DELETED/DELETING in SCM - // bcsid of replica and container is same, SCM will trigger delete replica for RATIS, while EC ignores bcsid + // Since replica state is CLOSED and container is DELETED/DELETING in SCM, and the bcsid of replica and + // container is same, SCM will trigger delete replica for RATIS (EC ignores bcsid) when it processes a + // periodic container report for the CLOSED replicas. // wait for all replica to be deleted GenericTestUtils.waitFor(() -> { try { diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/TestHelper.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/TestHelper.java index 246a88076007..bf6dd3306d3f 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/TestHelper.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/TestHelper.java @@ -44,6 +44,7 @@ import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.hdds.protocol.DatanodeDetails; import org.apache.hadoop.hdds.protocol.proto.HddsProtos; +import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.ContainerReplicaProto; import org.apache.hadoop.hdds.ratis.RatisHelper; import org.apache.hadoop.hdds.scm.container.ContainerID; import org.apache.hadoop.hdds.scm.container.ContainerInfo; @@ -464,6 +465,25 @@ public static void waitForReplicaCount(long containerID, int count, 200, 30000); } + /** + * Wait until SCM reports exactly {@code count} replicas for the container and every replica is in {@code state}. + * Unlike {@link #waitForContainerStateInSCM}, which checks the container's aggregate state (it flips as soon as the + * first replica reaches the state), this requires all replicas to have settled, so a lagging replica cannot trip + * later report handling. + */ + public static void waitForReplicaState(ContainerManager containerManager, ContainerID containerID, + int count, ContainerReplicaProto.State state) throws TimeoutException, InterruptedException { + GenericTestUtils.waitFor(() -> { + try { + Set replicas = containerManager.getContainerReplicas(containerID); + return replicas.size() == count + && replicas.stream().allMatch(replica -> replica.getState() == state); + } catch (ContainerNotFoundException e) { + return false; + } + }, 100, 60000); + } + /** Helper to set config even if {@code value} is null, which * {@link OzoneConfiguration#set(String, String) does not allow. */ public static void setConfig(OzoneConfiguration conf, String key, String value) {