From 21ea0746cb5e8d21d9c24deec6976f42892be4fb Mon Sep 17 00:00:00 2001 From: Jackson Yao Date: Tue, 25 Oct 2022 16:11:22 +0800 Subject: [PATCH 1/9] HDDS-7384. EC: ReplicationManager - implement deleting container handler --- .../replication/ReplicationManager.java | 2 + .../health/DeletingContainerHandler.java | 103 ++++++ .../health/TestDeletingContainerHandler.java | 324 ++++++++++++++++++ 3 files changed, 429 insertions(+) create mode 100644 hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/health/DeletingContainerHandler.java create mode 100644 hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/health/TestDeletingContainerHandler.java diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ReplicationManager.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ReplicationManager.java index 77142848b692..2404b8380993 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ReplicationManager.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ReplicationManager.java @@ -38,6 +38,7 @@ import org.apache.hadoop.hdds.scm.container.ReplicationManagerReport; import org.apache.hadoop.hdds.scm.container.replication.health.ClosedWithMismatchedReplicasHandler; import org.apache.hadoop.hdds.scm.container.replication.health.ClosingContainerHandler; +import org.apache.hadoop.hdds.scm.container.replication.health.DeletingContainerHandler; import org.apache.hadoop.hdds.scm.container.replication.health.ECReplicationCheckHandler; import org.apache.hadoop.hdds.scm.container.replication.health.HealthCheck; import org.apache.hadoop.hdds.scm.container.replication.health.OpenContainerHandler; @@ -220,6 +221,7 @@ public ReplicationManager(final ConfigurationSource conf, .addNext(new ClosingContainerHandler(this)) .addNext(new QuasiClosedContainerHandler(this)) .addNext(new ClosedWithMismatchedReplicasHandler(this)) + .addNext(new DeletingContainerHandler(this, containerManager)) .addNext(ecReplicationCheckHandler) .addNext(ratisReplicationCheckHandler); start(); diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/health/DeletingContainerHandler.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/health/DeletingContainerHandler.java new file mode 100644 index 000000000000..f5c70f0ad253 --- /dev/null +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/health/DeletingContainerHandler.java @@ -0,0 +1,103 @@ +/* + * 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.scm.container.replication.health; + +import org.apache.hadoop.hdds.protocol.DatanodeDetails; +import org.apache.hadoop.hdds.protocol.proto.HddsProtos; +import org.apache.hadoop.hdds.scm.container.ContainerID; +import org.apache.hadoop.hdds.scm.container.ContainerInfo; +import org.apache.hadoop.hdds.scm.container.ContainerManager; +import org.apache.hadoop.hdds.scm.container.replication.ContainerCheckRequest; +import org.apache.hadoop.hdds.scm.container.replication.ContainerReplicaOp; +import org.apache.hadoop.hdds.scm.container.replication.ReplicationManager; +import org.apache.hadoop.ozone.common.statemachine.InvalidStateTransitionException; +import org.apache.ratis.protocol.exceptions.NotLeaderException; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.io.IOException; +import java.util.Set; +import java.util.concurrent.TimeoutException; +import java.util.stream.Collectors; + +/** + * Class used in Replication Manager to handle the + * replicas of containers in DELETING State. + */ +public class DeletingContainerHandler extends AbstractCheck { + private final ReplicationManager replicationManager; + private final ContainerManager containerManager; + + public static final Logger LOG = + LoggerFactory.getLogger(DeletingContainerHandler.class); + + public DeletingContainerHandler(ReplicationManager replicationManager, + ContainerManager containerManager) { + this.replicationManager = replicationManager; + this.containerManager = containerManager; + } + + /** + * If the replica size of the container is 0, change the state + * of the container to Deleted, otherwise resend delete command if needed. + * @param request ContainerCheckRequest object representing the container + * @return false if the specified container is not in DELETING state, + * otherwise true. + */ + @Override + public boolean handle(ContainerCheckRequest request) { + ContainerInfo containerInfo = request.getContainerInfo(); + ContainerID cID = containerInfo.containerID(); + + if (containerInfo.getState() != HddsProtos.LifeCycleState.DELETING) { + return false; + } + + if (request.getContainerReplicas().size() == 0) { + try { + containerManager.updateContainerState( + cID, HddsProtos.LifeCycleEvent.CLEANUP); + LOG.debug("Container {} state changes to DELETED", cID); + } catch (IOException | InvalidStateTransitionException | + TimeoutException e) { + LOG.error("Failed to cleanup empty container {}", + request.getContainerInfo(), e); + } + return true; + } + + Set pendingDelete = request.getPendingOps().stream() + .filter(o -> o.getOpType() == ContainerReplicaOp.PendingOpType.DELETE) + .map(o -> o.getTarget()).collect(Collectors.toSet()); + //resend deleteCommand if needed + request.getContainerReplicas().stream() + .filter(r -> !pendingDelete.contains(r.getDatanodeDetails())) + .forEach(rp -> { + try { + replicationManager.sendDeleteCommand( + containerInfo, rp.getReplicaIndex(), rp.getDatanodeDetails()); + } catch (NotLeaderException e) { + LOG.warn("Failed to delete empty replica with index {} for " + + "container {} on datanode {}", rp.getReplicaIndex(), + cID, rp.getDatanodeDetails().getUuidString(), e); + } + }); + return true; + } +} \ No newline at end of file diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/health/TestDeletingContainerHandler.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/health/TestDeletingContainerHandler.java new file mode 100644 index 000000000000..fa734354755f --- /dev/null +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/health/TestDeletingContainerHandler.java @@ -0,0 +1,324 @@ +/* + * 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.scm.container.replication.health; + +import org.apache.hadoop.fs.FileUtil; +import org.apache.hadoop.hdds.HddsConfigKeys; +import org.apache.hadoop.hdds.client.ECReplicationConfig; +import org.apache.hadoop.hdds.client.RatisReplicationConfig; +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.ContainerInfo; +import org.apache.hadoop.hdds.scm.container.ContainerManager; +import org.apache.hadoop.hdds.scm.container.ContainerManagerImpl; +import org.apache.hadoop.hdds.scm.container.ContainerReplica; +import org.apache.hadoop.hdds.scm.container.MockNodeManager; +import org.apache.hadoop.hdds.scm.container.ReplicationManagerReport; +import org.apache.hadoop.hdds.scm.container.replication.ContainerCheckRequest; +import org.apache.hadoop.hdds.scm.container.replication.ContainerReplicaOp; +import org.apache.hadoop.hdds.scm.container.replication.ContainerReplicaPendingOps; +import org.apache.hadoop.hdds.scm.container.replication.ReplicationManager; +import org.apache.hadoop.hdds.scm.container.replication.ReplicationTestUtil; +import org.apache.hadoop.hdds.scm.ha.SCMHAManager; +import org.apache.hadoop.hdds.scm.ha.SCMHAManagerStub; +import org.apache.hadoop.hdds.scm.ha.SequenceIdGenerator; +import org.apache.hadoop.hdds.scm.metadata.SCMDBDefinition; +import org.apache.hadoop.hdds.scm.node.NodeManager; +import org.apache.hadoop.hdds.scm.pipeline.MockPipelineManager; +import org.apache.hadoop.hdds.scm.pipeline.PipelineManager; +import org.apache.hadoop.hdds.utils.db.DBStore; +import org.apache.hadoop.hdds.utils.db.DBStoreBuilder; +import org.apache.hadoop.ozone.common.statemachine.InvalidStateTransitionException; +import org.apache.hadoop.ozone.container.common.SCMTestUtils; +import org.apache.ozone.test.GenericTestUtils; +import org.junit.Assert; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.Mockito; + +import java.io.File; +import java.io.IOException; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashSet; +import java.util.Iterator; +import java.util.List; +import java.util.Set; +import java.util.UUID; +import java.util.concurrent.TimeoutException; + +import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.LifeCycleState.CLOSED; +import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.LifeCycleState.DELETED; + +/** + * Tests for {@link DeletingContainerHandler}. + */ +public class TestDeletingContainerHandler { + private ReplicationManager replicationManager; + private File testDir; + private ContainerManager containerManager; + private DeletingContainerHandler deletingContainerHandler; + private ECReplicationConfig ecReplicationConfig; + private RatisReplicationConfig ratisReplicationConfig; + private ContainerReplicaPendingOps pendingOpsMock; + private SequenceIdGenerator sequenceIdGen; + private SCMHAManager scmhaManager; + private NodeManager nodeManager; + private PipelineManager pipelineManager; + private DBStore dbStore; + + @BeforeEach + public void setup() throws IOException { + final OzoneConfiguration conf = SCMTestUtils.getConf(); + testDir = GenericTestUtils.getTestDir( + TestDeletingContainerHandler.class.getSimpleName() + UUID.randomUUID()); + conf.set(HddsConfigKeys.OZONE_METADATA_DIRS, testDir.getAbsolutePath()); + dbStore = DBStoreBuilder.createDBStore( + conf, new SCMDBDefinition()); + ecReplicationConfig = new ECReplicationConfig(3, 2); + ratisReplicationConfig = RatisReplicationConfig.getInstance( + HddsProtos.ReplicationFactor.THREE); + scmhaManager = SCMHAManagerStub.getInstance(true); + replicationManager = Mockito.mock(ReplicationManager.class); + pendingOpsMock = Mockito.mock(ContainerReplicaPendingOps.class); + nodeManager = new MockNodeManager(true, 10); + sequenceIdGen = new SequenceIdGenerator( + conf, scmhaManager, SCMDBDefinition.SEQUENCE_ID.getTable(dbStore)); + pipelineManager = + new MockPipelineManager(dbStore, scmhaManager, nodeManager); + containerManager = new ContainerManagerImpl(conf, + scmhaManager, sequenceIdGen, pipelineManager, + SCMDBDefinition.CONTAINERS.getTable(dbStore), pendingOpsMock); + deletingContainerHandler = + new DeletingContainerHandler(replicationManager, containerManager); + } + + @AfterEach + public void cleanup() throws Exception { + if (containerManager != null) { + containerManager.close(); + } + + if (dbStore != null) { + dbStore.close(); + } + + FileUtil.fullyDelete(testDir); + } + + /** + * If a container is not in Deleting state, it should not be handled by + * DeletingContainerHandler. It should return false so the request can be + * passed to the next handler in the chain. + */ + @Test + public void testNonDeletingContainerReturnsFalse() { + ContainerInfo containerInfo = ReplicationTestUtil.createContainerInfo( + ecReplicationConfig, 1, CLOSED); + Set containerReplicas = ReplicationTestUtil + .createReplicas(containerInfo.containerID(), + ContainerReplicaProto.State.CLOSING, 1, 2, 3, 4, 5); + + ContainerCheckRequest request = new ContainerCheckRequest.Builder() + .setPendingOps(Collections.EMPTY_LIST) + .setReport(new ReplicationManagerReport()) + .setContainerInfo(containerInfo) + .setContainerReplicas(containerReplicas) + .build(); + + Assert.assertFalse(deletingContainerHandler.handle(request)); + } + + @Test + public void testNonDeletingRatisContainerReturnsFalse() { + ContainerInfo containerInfo = ReplicationTestUtil.createContainerInfo( + ratisReplicationConfig, 1, CLOSED); + Set containerReplicas = ReplicationTestUtil + .createReplicas(containerInfo.containerID(), + ContainerReplicaProto.State.CLOSING, 0, 0, 0); + + ContainerCheckRequest request = new ContainerCheckRequest.Builder() + .setPendingOps(Collections.EMPTY_LIST) + .setReport(new ReplicationManagerReport()) + .setContainerInfo(containerInfo) + .setContainerReplicas(containerReplicas) + .build(); + + Assert.assertFalse(deletingContainerHandler.handle(request)); + } + + /** + * If a container is in Deleting state and no replica exists, + * change the state of the container to DELETED. + */ + @Test + public void testCleanupIfNoReplicaExist() + throws IOException, TimeoutException, InvalidStateTransitionException { + //ratis container + cleanupIfNoReplicaExist(RatisReplicationConfig.getInstance( + HddsProtos.ReplicationFactor.THREE)); + + //ec container + cleanupIfNoReplicaExist(ecReplicationConfig); + } + + + private void cleanupIfNoReplicaExist(ReplicationConfig replicationConfig) + throws IOException, TimeoutException, InvalidStateTransitionException { + ContainerInfo containerInfo = containerManager.allocateContainer( + replicationConfig, "admin"); + ContainerID cID = containerInfo.containerID(); + + //change the state of the container to Deleting + containerManager.updateContainerState(cID, + HddsProtos.LifeCycleEvent.FINALIZE); + containerManager.updateContainerState(cID, HddsProtos.LifeCycleEvent.CLOSE); + containerManager.updateContainerState(cID, + HddsProtos.LifeCycleEvent.DELETE); + + Set containerReplicas = new HashSet<>(); + ContainerCheckRequest request = new ContainerCheckRequest.Builder() + .setPendingOps(Collections.EMPTY_LIST) + .setReport(new ReplicationManagerReport()) + .setContainerInfo(containerInfo) + .setContainerReplicas(containerReplicas) + .build(); + + Assert.assertTrue(deletingContainerHandler.handle(request)); + Assert.assertTrue(containerInfo.getState() == DELETED); + } + + /** + * If a container is in Deleting state , some replicas exist and + * for each replica there is a pending delete, then do nothing. + */ + @Test + public void testNoNeedResendDeleteCommand() + throws IOException, TimeoutException, InvalidStateTransitionException { + //ratis container + ContainerInfo containerInfo = containerManager.allocateContainer( + RatisReplicationConfig.getInstance( + HddsProtos.ReplicationFactor.THREE), "admin"); + Set containerReplicas = ReplicationTestUtil + .createReplicas(containerInfo.containerID(), + ContainerReplicaProto.State.CLOSING, 0, 0, 0); + List pendingOps = new ArrayList<>(); + containerReplicas.forEach(r -> pendingOps.add( + ContainerReplicaOp.create(ContainerReplicaOp.PendingOpType.DELETE, + r.getDatanodeDetails(), r.getReplicaIndex()))); + resendDeleteCommand(containerInfo, containerReplicas, pendingOps, 0); + + //EC container + containerInfo = containerManager.allocateContainer( + ecReplicationConfig, "admin"); + containerReplicas = ReplicationTestUtil + .createReplicas(containerInfo.containerID(), + ContainerReplicaProto.State.CLOSING, 1, 2, 3, 4, 5); + pendingOps.clear(); + containerReplicas.forEach(r -> pendingOps.add( + ContainerReplicaOp.create(ContainerReplicaOp.PendingOpType.DELETE, + r.getDatanodeDetails(), r.getReplicaIndex()))); + resendDeleteCommand(containerInfo, containerReplicas, pendingOps, 0); + + } + + /** + * If a container is in Deleting state , some replicas exist and + * for some replica there is no pending delete, then resending delete + * command. + */ + @Test + public void testResendDeleteCommand() + throws IOException, TimeoutException, InvalidStateTransitionException { + //ratis container + ContainerInfo containerInfo = containerManager.allocateContainer( + RatisReplicationConfig.getInstance( + HddsProtos.ReplicationFactor.THREE), "admin"); + Set containerReplicas = ReplicationTestUtil + .createReplicas(containerInfo.containerID(), + ContainerReplicaProto.State.CLOSING, 0, 0, 0); + List pendingOps = new ArrayList<>(); + Set tempContainerReplicas = new HashSet<>(); + tempContainerReplicas.addAll(containerReplicas); + Iterator iter = tempContainerReplicas.iterator(); + iter.next(); + iter.remove(); + Assert.assertEquals(2, tempContainerReplicas.size()); + tempContainerReplicas.forEach(r -> pendingOps.add( + ContainerReplicaOp.create(ContainerReplicaOp.PendingOpType.DELETE, + r.getDatanodeDetails(), r.getReplicaIndex()))); + resendDeleteCommand(containerInfo, containerReplicas, pendingOps, 1); + + //EC container + containerInfo = containerManager.allocateContainer( + ecReplicationConfig, "admin"); + containerReplicas = ReplicationTestUtil + .createReplicas(containerInfo.containerID(), + ContainerReplicaProto.State.CLOSING, 1, 2, 3, 4, 5); + pendingOps.clear(); + tempContainerReplicas.clear(); + tempContainerReplicas.addAll(containerReplicas); + iter = tempContainerReplicas.iterator(); + iter.next(); + iter.remove(); + iter.next(); + iter.remove(); + Assert.assertEquals(3, tempContainerReplicas.size()); + + tempContainerReplicas.forEach(r -> pendingOps.add( + ContainerReplicaOp.create(ContainerReplicaOp.PendingOpType.DELETE, + r.getDatanodeDetails(), r.getReplicaIndex()))); + //since one delete command is end when testing ratis container, so + //here should be 1+2 = 3 times + resendDeleteCommand(containerInfo, containerReplicas, pendingOps, 3); + + } + + private void resendDeleteCommand(ContainerInfo containerInfo, + Set containerReplicas, + List pendingOps, + int times) + throws InvalidStateTransitionException, IOException, TimeoutException { + ContainerID cID = containerInfo.containerID(); + //change the state of the container to Deleting + containerManager.updateContainerState(cID, + HddsProtos.LifeCycleEvent.FINALIZE); + containerManager.updateContainerState(cID, HddsProtos.LifeCycleEvent.CLOSE); + containerManager.updateContainerState(cID, + HddsProtos.LifeCycleEvent.DELETE); + + ContainerCheckRequest request = new ContainerCheckRequest.Builder() + .setPendingOps(pendingOps) + .setReport(new ReplicationManagerReport()) + .setContainerInfo(containerInfo) + .setContainerReplicas(containerReplicas) + .build(); + + Assert.assertTrue(deletingContainerHandler.handle(request)); + + Mockito.verify(replicationManager, Mockito.times(times)) + .sendDeleteCommand(Mockito.any(ContainerInfo.class), Mockito.anyInt(), + Mockito.any(DatanodeDetails.class)); + } +} From 97d6d6ad6c82a80d898b2eec9feb3c8516e799cc Mon Sep 17 00:00:00 2001 From: Jackson Yao Date: Wed, 26 Oct 2022 12:02:29 +0800 Subject: [PATCH 2/9] add check for DELETED state --- .../replication/health/DeletingContainerHandler.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/health/DeletingContainerHandler.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/health/DeletingContainerHandler.java index f5c70f0ad253..9163c4404145 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/health/DeletingContainerHandler.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/health/DeletingContainerHandler.java @@ -64,8 +64,13 @@ public DeletingContainerHandler(ReplicationManager replicationManager, public boolean handle(ContainerCheckRequest request) { ContainerInfo containerInfo = request.getContainerInfo(); ContainerID cID = containerInfo.containerID(); + HddsProtos.LifeCycleState containerState = containerInfo.getState(); - if (containerInfo.getState() != HddsProtos.LifeCycleState.DELETING) { + if (containerState == HddsProtos.LifeCycleState.DELETED) { + return true; + } + + if (containerState != HddsProtos.LifeCycleState.DELETING) { return false; } From e43e6ec08b6f312e9cf92b0a1635403ac2c6a51d Mon Sep 17 00:00:00 2001 From: Jackson Yao Date: Wed, 26 Oct 2022 16:33:28 +0800 Subject: [PATCH 3/9] add updateContainerState in RM --- .../replication/ReplicationManager.java | 20 ++++++++++++++++++- .../health/DeletingContainerHandler.java | 20 +++---------------- .../health/EmptyContainerHandler.java | 19 +++--------------- .../health/TestDeletingContainerHandler.java | 12 ++++++++++- .../health/TestEmptyContainerHandler.java | 11 +++++++++- 5 files changed, 46 insertions(+), 36 deletions(-) diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ReplicationManager.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ReplicationManager.java index 2404b8380993..b7befbc720a2 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ReplicationManager.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ReplicationManager.java @@ -52,6 +52,7 @@ import org.apache.hadoop.hdds.scm.node.states.NodeNotFoundException; import org.apache.hadoop.hdds.scm.server.StorageContainerManager; import org.apache.hadoop.hdds.server.events.EventPublisher; +import org.apache.hadoop.ozone.common.statemachine.InvalidStateTransitionException; import org.apache.hadoop.ozone.protocol.commands.CloseContainerCommand; import org.apache.hadoop.ozone.protocol.commands.CommandForDatanode; import org.apache.hadoop.ozone.protocol.commands.DeleteContainerCommand; @@ -221,7 +222,7 @@ public ReplicationManager(final ConfigurationSource conf, .addNext(new ClosingContainerHandler(this)) .addNext(new QuasiClosedContainerHandler(this)) .addNext(new ClosedWithMismatchedReplicasHandler(this)) - .addNext(new DeletingContainerHandler(this, containerManager)) + .addNext(new DeletingContainerHandler(this)) .addNext(ecReplicationCheckHandler) .addNext(ratisReplicationCheckHandler); start(); @@ -405,6 +406,23 @@ public void sendDeleteCommand(final ContainerInfo container, int replicaIndex, } } + /** + * update container state. + * + * @param containerID Container to be updated + * @param event the event to update the container + */ + public void updateContainerState(ContainerID containerID, + HddsProtos.LifeCycleEvent event) { + try { + containerManager.updateContainerState(containerID, event); + } catch (IOException | InvalidStateTransitionException | + TimeoutException e) { + LOG.error("Failed to update the state of container {}, update Event {}", + containerID, event, e); + } + } + /** * Add an under replicated container back to the queue if it was unable to diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/health/DeletingContainerHandler.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/health/DeletingContainerHandler.java index 9163c4404145..eed8c13938a3 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/health/DeletingContainerHandler.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/health/DeletingContainerHandler.java @@ -22,18 +22,14 @@ import org.apache.hadoop.hdds.protocol.proto.HddsProtos; import org.apache.hadoop.hdds.scm.container.ContainerID; import org.apache.hadoop.hdds.scm.container.ContainerInfo; -import org.apache.hadoop.hdds.scm.container.ContainerManager; import org.apache.hadoop.hdds.scm.container.replication.ContainerCheckRequest; import org.apache.hadoop.hdds.scm.container.replication.ContainerReplicaOp; import org.apache.hadoop.hdds.scm.container.replication.ReplicationManager; -import org.apache.hadoop.ozone.common.statemachine.InvalidStateTransitionException; import org.apache.ratis.protocol.exceptions.NotLeaderException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.io.IOException; import java.util.Set; -import java.util.concurrent.TimeoutException; import java.util.stream.Collectors; /** @@ -42,15 +38,12 @@ */ public class DeletingContainerHandler extends AbstractCheck { private final ReplicationManager replicationManager; - private final ContainerManager containerManager; public static final Logger LOG = LoggerFactory.getLogger(DeletingContainerHandler.class); - public DeletingContainerHandler(ReplicationManager replicationManager, - ContainerManager containerManager) { + public DeletingContainerHandler(ReplicationManager replicationManager) { this.replicationManager = replicationManager; - this.containerManager = containerManager; } /** @@ -75,15 +68,8 @@ public boolean handle(ContainerCheckRequest request) { } if (request.getContainerReplicas().size() == 0) { - try { - containerManager.updateContainerState( - cID, HddsProtos.LifeCycleEvent.CLEANUP); - LOG.debug("Container {} state changes to DELETED", cID); - } catch (IOException | InvalidStateTransitionException | - TimeoutException e) { - LOG.error("Failed to cleanup empty container {}", - request.getContainerInfo(), e); - } + replicationManager.updateContainerState( + cID, HddsProtos.LifeCycleEvent.CLEANUP); return true; } diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/health/EmptyContainerHandler.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/health/EmptyContainerHandler.java index 47ac50ce3a02..19c8d0a93aaa 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/health/EmptyContainerHandler.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/health/EmptyContainerHandler.java @@ -20,20 +20,16 @@ import org.apache.hadoop.hdds.protocol.proto.HddsProtos; import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.ContainerReplicaProto; import org.apache.hadoop.hdds.scm.container.ContainerInfo; -import org.apache.hadoop.hdds.scm.container.ContainerManager; import org.apache.hadoop.hdds.scm.container.ContainerReplica; import org.apache.hadoop.hdds.scm.container.ReplicationManagerReport; import org.apache.hadoop.hdds.scm.container.replication.ContainerCheckRequest; import org.apache.hadoop.hdds.scm.container.replication.ReplicationManager; -import org.apache.hadoop.ozone.common.statemachine.InvalidStateTransitionException; import org.apache.ratis.protocol.exceptions.NotLeaderException; import org.apache.ratis.util.Preconditions; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.io.IOException; import java.util.Set; -import java.util.concurrent.TimeoutException; /** * This handler deletes a container if it's closed and empty (0 key count) @@ -44,12 +40,9 @@ public class EmptyContainerHandler extends AbstractCheck { LoggerFactory.getLogger(EmptyContainerHandler.class); private final ReplicationManager replicationManager; - private final ContainerManager containerManager; - public EmptyContainerHandler(ReplicationManager replicationManager, - ContainerManager containerManager) { + public EmptyContainerHandler(ReplicationManager replicationManager) { this.replicationManager = replicationManager; - this.containerManager = containerManager; } /** @@ -72,14 +65,8 @@ public boolean handle(ContainerCheckRequest request) { deleteContainerReplicas(containerInfo, replicas); // Update the container's state - try { - containerManager.updateContainerState(containerInfo.containerID(), - HddsProtos.LifeCycleEvent.DELETE); - } catch (IOException | InvalidStateTransitionException | - TimeoutException e) { - LOG.error("Failed to delete empty container {}", - request.getContainerInfo(), e); - } + replicationManager.updateContainerState( + containerInfo.containerID(), HddsProtos.LifeCycleEvent.DELETE); return true; } diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/health/TestDeletingContainerHandler.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/health/TestDeletingContainerHandler.java index fa734354755f..a46dad18811d 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/health/TestDeletingContainerHandler.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/health/TestDeletingContainerHandler.java @@ -110,8 +110,18 @@ public void setup() throws IOException { containerManager = new ContainerManagerImpl(conf, scmhaManager, sequenceIdGen, pipelineManager, SCMDBDefinition.CONTAINERS.getTable(dbStore), pendingOpsMock); + + Mockito.doAnswer(invocation -> { + containerManager.updateContainerState( + ((ContainerID)invocation.getArguments()[0]), + (HddsProtos.LifeCycleEvent) invocation.getArguments()[1]); + return null; + }).when(replicationManager).updateContainerState( + Mockito.any(ContainerID.class), + Mockito.any(HddsProtos.LifeCycleEvent.class)); + deletingContainerHandler = - new DeletingContainerHandler(replicationManager, containerManager); + new DeletingContainerHandler(replicationManager); } @AfterEach diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/health/TestEmptyContainerHandler.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/health/TestEmptyContainerHandler.java index ac746900f544..4d546d71ebf6 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/health/TestEmptyContainerHandler.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/health/TestEmptyContainerHandler.java @@ -63,9 +63,18 @@ public void setup() HddsProtos.ReplicationFactor.THREE); replicationManager = Mockito.mock(ReplicationManager.class); containerManager = Mockito.mock(ContainerManager.class); + + Mockito.doAnswer(invocation -> { + containerManager.updateContainerState( + ((ContainerID)invocation.getArguments()[0]), + (HddsProtos.LifeCycleEvent) invocation.getArguments()[1]); + return null; + }).when(replicationManager).updateContainerState( + Mockito.any(ContainerID.class), + Mockito.any(HddsProtos.LifeCycleEvent.class)); emptyContainerHandler = - new EmptyContainerHandler(replicationManager, containerManager); + new EmptyContainerHandler(replicationManager); } /** From f963d3201bb8ed7f94f39325542898fb4c9f083a Mon Sep 17 00:00:00 2001 From: Jackson Yao Date: Wed, 26 Oct 2022 19:02:11 +0800 Subject: [PATCH 4/9] update test --- .../health/TestDeletingContainerHandler.java | 148 ++++-------------- 1 file changed, 33 insertions(+), 115 deletions(-) diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/health/TestDeletingContainerHandler.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/health/TestDeletingContainerHandler.java index a46dad18811d..07b60c1db4e1 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/health/TestDeletingContainerHandler.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/health/TestDeletingContainerHandler.java @@ -18,46 +18,26 @@ package org.apache.hadoop.hdds.scm.container.replication.health; -import org.apache.hadoop.fs.FileUtil; -import org.apache.hadoop.hdds.HddsConfigKeys; import org.apache.hadoop.hdds.client.ECReplicationConfig; import org.apache.hadoop.hdds.client.RatisReplicationConfig; 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.ContainerInfo; -import org.apache.hadoop.hdds.scm.container.ContainerManager; -import org.apache.hadoop.hdds.scm.container.ContainerManagerImpl; import org.apache.hadoop.hdds.scm.container.ContainerReplica; -import org.apache.hadoop.hdds.scm.container.MockNodeManager; import org.apache.hadoop.hdds.scm.container.ReplicationManagerReport; import org.apache.hadoop.hdds.scm.container.replication.ContainerCheckRequest; import org.apache.hadoop.hdds.scm.container.replication.ContainerReplicaOp; -import org.apache.hadoop.hdds.scm.container.replication.ContainerReplicaPendingOps; import org.apache.hadoop.hdds.scm.container.replication.ReplicationManager; import org.apache.hadoop.hdds.scm.container.replication.ReplicationTestUtil; -import org.apache.hadoop.hdds.scm.ha.SCMHAManager; -import org.apache.hadoop.hdds.scm.ha.SCMHAManagerStub; -import org.apache.hadoop.hdds.scm.ha.SequenceIdGenerator; -import org.apache.hadoop.hdds.scm.metadata.SCMDBDefinition; -import org.apache.hadoop.hdds.scm.node.NodeManager; -import org.apache.hadoop.hdds.scm.pipeline.MockPipelineManager; -import org.apache.hadoop.hdds.scm.pipeline.PipelineManager; -import org.apache.hadoop.hdds.utils.db.DBStore; -import org.apache.hadoop.hdds.utils.db.DBStoreBuilder; -import org.apache.hadoop.ozone.common.statemachine.InvalidStateTransitionException; -import org.apache.hadoop.ozone.container.common.SCMTestUtils; -import org.apache.ozone.test.GenericTestUtils; +import org.apache.ratis.protocol.exceptions.NotLeaderException; import org.junit.Assert; -import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mockito.Mockito; -import java.io.File; import java.io.IOException; import java.util.ArrayList; import java.util.Collections; @@ -65,78 +45,36 @@ import java.util.Iterator; import java.util.List; import java.util.Set; -import java.util.UUID; -import java.util.concurrent.TimeoutException; import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.LifeCycleState.CLOSED; -import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.LifeCycleState.DELETED; +import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.LifeCycleState.DELETING; /** * Tests for {@link DeletingContainerHandler}. */ public class TestDeletingContainerHandler { private ReplicationManager replicationManager; - private File testDir; - private ContainerManager containerManager; private DeletingContainerHandler deletingContainerHandler; private ECReplicationConfig ecReplicationConfig; private RatisReplicationConfig ratisReplicationConfig; - private ContainerReplicaPendingOps pendingOpsMock; - private SequenceIdGenerator sequenceIdGen; - private SCMHAManager scmhaManager; - private NodeManager nodeManager; - private PipelineManager pipelineManager; - private DBStore dbStore; + @BeforeEach public void setup() throws IOException { - final OzoneConfiguration conf = SCMTestUtils.getConf(); - testDir = GenericTestUtils.getTestDir( - TestDeletingContainerHandler.class.getSimpleName() + UUID.randomUUID()); - conf.set(HddsConfigKeys.OZONE_METADATA_DIRS, testDir.getAbsolutePath()); - dbStore = DBStoreBuilder.createDBStore( - conf, new SCMDBDefinition()); + ecReplicationConfig = new ECReplicationConfig(3, 2); ratisReplicationConfig = RatisReplicationConfig.getInstance( HddsProtos.ReplicationFactor.THREE); - scmhaManager = SCMHAManagerStub.getInstance(true); replicationManager = Mockito.mock(ReplicationManager.class); - pendingOpsMock = Mockito.mock(ContainerReplicaPendingOps.class); - nodeManager = new MockNodeManager(true, 10); - sequenceIdGen = new SequenceIdGenerator( - conf, scmhaManager, SCMDBDefinition.SEQUENCE_ID.getTable(dbStore)); - pipelineManager = - new MockPipelineManager(dbStore, scmhaManager, nodeManager); - containerManager = new ContainerManagerImpl(conf, - scmhaManager, sequenceIdGen, pipelineManager, - SCMDBDefinition.CONTAINERS.getTable(dbStore), pendingOpsMock); - - Mockito.doAnswer(invocation -> { - containerManager.updateContainerState( - ((ContainerID)invocation.getArguments()[0]), - (HddsProtos.LifeCycleEvent) invocation.getArguments()[1]); - return null; - }).when(replicationManager).updateContainerState( - Mockito.any(ContainerID.class), - Mockito.any(HddsProtos.LifeCycleEvent.class)); + + Mockito.doNothing().when(replicationManager) + .updateContainerState(Mockito.any(ContainerID.class), + Mockito.any(HddsProtos.LifeCycleEvent.class)); deletingContainerHandler = new DeletingContainerHandler(replicationManager); } - @AfterEach - public void cleanup() throws Exception { - if (containerManager != null) { - containerManager.close(); - } - - if (dbStore != null) { - dbStore.close(); - } - - FileUtil.fullyDelete(testDir); - } - /** * If a container is not in Deleting state, it should not be handled by * DeletingContainerHandler. It should return false so the request can be @@ -183,29 +121,20 @@ public void testNonDeletingRatisContainerReturnsFalse() { * change the state of the container to DELETED. */ @Test - public void testCleanupIfNoReplicaExist() - throws IOException, TimeoutException, InvalidStateTransitionException { + public void testCleanupIfNoReplicaExist() { //ratis container cleanupIfNoReplicaExist(RatisReplicationConfig.getInstance( - HddsProtos.ReplicationFactor.THREE)); + HddsProtos.ReplicationFactor.THREE), 1); //ec container - cleanupIfNoReplicaExist(ecReplicationConfig); + cleanupIfNoReplicaExist(ecReplicationConfig, 2); } - private void cleanupIfNoReplicaExist(ReplicationConfig replicationConfig) - throws IOException, TimeoutException, InvalidStateTransitionException { - ContainerInfo containerInfo = containerManager.allocateContainer( - replicationConfig, "admin"); - ContainerID cID = containerInfo.containerID(); - - //change the state of the container to Deleting - containerManager.updateContainerState(cID, - HddsProtos.LifeCycleEvent.FINALIZE); - containerManager.updateContainerState(cID, HddsProtos.LifeCycleEvent.CLOSE); - containerManager.updateContainerState(cID, - HddsProtos.LifeCycleEvent.DELETE); + private void cleanupIfNoReplicaExist( + ReplicationConfig replicationConfig, int times) { + ContainerInfo containerInfo = ReplicationTestUtil.createContainerInfo( + replicationConfig, 1, DELETING); Set containerReplicas = new HashSet<>(); ContainerCheckRequest request = new ContainerCheckRequest.Builder() @@ -216,7 +145,9 @@ private void cleanupIfNoReplicaExist(ReplicationConfig replicationConfig) .build(); Assert.assertTrue(deletingContainerHandler.handle(request)); - Assert.assertTrue(containerInfo.getState() == DELETED); + Mockito.verify(replicationManager, Mockito.times(times)) + .updateContainerState(Mockito.any(ContainerID.class), + Mockito.any(HddsProtos.LifeCycleEvent.class)); } /** @@ -224,15 +155,13 @@ private void cleanupIfNoReplicaExist(ReplicationConfig replicationConfig) * for each replica there is a pending delete, then do nothing. */ @Test - public void testNoNeedResendDeleteCommand() - throws IOException, TimeoutException, InvalidStateTransitionException { + public void testNoNeedResendDeleteCommand() throws NotLeaderException { //ratis container - ContainerInfo containerInfo = containerManager.allocateContainer( - RatisReplicationConfig.getInstance( - HddsProtos.ReplicationFactor.THREE), "admin"); + ContainerInfo containerInfo = ReplicationTestUtil.createContainerInfo( + ratisReplicationConfig, 1, DELETING); Set containerReplicas = ReplicationTestUtil .createReplicas(containerInfo.containerID(), - ContainerReplicaProto.State.CLOSING, 0, 0, 0); + ContainerReplicaProto.State.CLOSED, 0, 0, 0); List pendingOps = new ArrayList<>(); containerReplicas.forEach(r -> pendingOps.add( ContainerReplicaOp.create(ContainerReplicaOp.PendingOpType.DELETE, @@ -240,11 +169,11 @@ public void testNoNeedResendDeleteCommand() resendDeleteCommand(containerInfo, containerReplicas, pendingOps, 0); //EC container - containerInfo = containerManager.allocateContainer( - ecReplicationConfig, "admin"); + containerInfo = ReplicationTestUtil.createContainerInfo( + ecReplicationConfig, 1, DELETING); containerReplicas = ReplicationTestUtil .createReplicas(containerInfo.containerID(), - ContainerReplicaProto.State.CLOSING, 1, 2, 3, 4, 5); + ContainerReplicaProto.State.CLOSED, 1, 2, 3, 4, 5); pendingOps.clear(); containerReplicas.forEach(r -> pendingOps.add( ContainerReplicaOp.create(ContainerReplicaOp.PendingOpType.DELETE, @@ -259,15 +188,13 @@ public void testNoNeedResendDeleteCommand() * command. */ @Test - public void testResendDeleteCommand() - throws IOException, TimeoutException, InvalidStateTransitionException { + public void testResendDeleteCommand() throws NotLeaderException { //ratis container - ContainerInfo containerInfo = containerManager.allocateContainer( - RatisReplicationConfig.getInstance( - HddsProtos.ReplicationFactor.THREE), "admin"); + ContainerInfo containerInfo = ReplicationTestUtil.createContainerInfo( + ratisReplicationConfig, 1, DELETING); Set containerReplicas = ReplicationTestUtil .createReplicas(containerInfo.containerID(), - ContainerReplicaProto.State.CLOSING, 0, 0, 0); + ContainerReplicaProto.State.CLOSED, 0, 0, 0); List pendingOps = new ArrayList<>(); Set tempContainerReplicas = new HashSet<>(); tempContainerReplicas.addAll(containerReplicas); @@ -281,11 +208,11 @@ public void testResendDeleteCommand() resendDeleteCommand(containerInfo, containerReplicas, pendingOps, 1); //EC container - containerInfo = containerManager.allocateContainer( - ecReplicationConfig, "admin"); + containerInfo = ReplicationTestUtil.createContainerInfo( + ecReplicationConfig, 1, DELETING); containerReplicas = ReplicationTestUtil .createReplicas(containerInfo.containerID(), - ContainerReplicaProto.State.CLOSING, 1, 2, 3, 4, 5); + ContainerReplicaProto.State.CLOSED, 1, 2, 3, 4, 5); pendingOps.clear(); tempContainerReplicas.clear(); tempContainerReplicas.addAll(containerReplicas); @@ -308,16 +235,7 @@ public void testResendDeleteCommand() private void resendDeleteCommand(ContainerInfo containerInfo, Set containerReplicas, List pendingOps, - int times) - throws InvalidStateTransitionException, IOException, TimeoutException { - ContainerID cID = containerInfo.containerID(); - //change the state of the container to Deleting - containerManager.updateContainerState(cID, - HddsProtos.LifeCycleEvent.FINALIZE); - containerManager.updateContainerState(cID, HddsProtos.LifeCycleEvent.CLOSE); - containerManager.updateContainerState(cID, - HddsProtos.LifeCycleEvent.DELETE); - + int times) throws NotLeaderException { ContainerCheckRequest request = new ContainerCheckRequest.Builder() .setPendingOps(pendingOps) .setReport(new ReplicationManagerReport()) From 7c4e96a7cd947d87bc03d7e4dbf2cc20753bd08f Mon Sep 17 00:00:00 2001 From: Jackson Yao Date: Thu, 27 Oct 2022 09:39:37 +0800 Subject: [PATCH 5/9] remove containerManager from TestEmptyContainerHandler --- .../health/TestEmptyContainerHandler.java | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/health/TestEmptyContainerHandler.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/health/TestEmptyContainerHandler.java index 4d546d71ebf6..d522f511378e 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/health/TestEmptyContainerHandler.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/health/TestEmptyContainerHandler.java @@ -25,7 +25,6 @@ import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.ContainerReplicaProto; import org.apache.hadoop.hdds.scm.container.ContainerID; import org.apache.hadoop.hdds.scm.container.ContainerInfo; -import org.apache.hadoop.hdds.scm.container.ContainerManager; import org.apache.hadoop.hdds.scm.container.ContainerReplica; import org.apache.hadoop.hdds.scm.container.ReplicationManagerReport; import org.apache.hadoop.hdds.scm.container.replication.ContainerCheckRequest; @@ -50,7 +49,6 @@ */ public class TestEmptyContainerHandler { private ReplicationManager replicationManager; - private ContainerManager containerManager; private EmptyContainerHandler emptyContainerHandler; private ECReplicationConfig ecReplicationConfig; private RatisReplicationConfig ratisReplicationConfig; @@ -62,17 +60,6 @@ public void setup() ratisReplicationConfig = RatisReplicationConfig.getInstance( HddsProtos.ReplicationFactor.THREE); replicationManager = Mockito.mock(ReplicationManager.class); - containerManager = Mockito.mock(ContainerManager.class); - - Mockito.doAnswer(invocation -> { - containerManager.updateContainerState( - ((ContainerID)invocation.getArguments()[0]), - (HddsProtos.LifeCycleEvent) invocation.getArguments()[1]); - return null; - }).when(replicationManager).updateContainerState( - Mockito.any(ContainerID.class), - Mockito.any(HddsProtos.LifeCycleEvent.class)); - emptyContainerHandler = new EmptyContainerHandler(replicationManager); } @@ -233,7 +220,7 @@ private void assertAndVerify(ContainerCheckRequest request, ReplicationManagerReport.HealthState.EMPTY)); if (times > 0) { - Mockito.verify(containerManager, Mockito.times(1)) + Mockito.verify(replicationManager, Mockito.times(1)) .updateContainerState(Mockito.any(ContainerID.class), Mockito.any(HddsProtos.LifeCycleEvent.class)); } From a49183a3a70eebfb56887edf264d5b8786937541 Mon Sep 17 00:00:00 2001 From: Jackson Yao Date: Thu, 27 Oct 2022 09:45:48 +0800 Subject: [PATCH 6/9] remove unused exception --- .../health/TestEmptyContainerHandler.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/health/TestEmptyContainerHandler.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/health/TestEmptyContainerHandler.java index d522f511378e..bac90b4c390e 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/health/TestEmptyContainerHandler.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/health/TestEmptyContainerHandler.java @@ -70,7 +70,7 @@ public void setup() */ @Test public void testEmptyAndClosedECContainerReturnsTrue() - throws InvalidStateTransitionException, IOException, TimeoutException { + throws IOException { long keyCount = 0L; long bytesUsed = 123L; ContainerInfo containerInfo = ReplicationTestUtil.createContainerInfo( @@ -92,7 +92,7 @@ public void testEmptyAndClosedECContainerReturnsTrue() @Test public void testEmptyAndClosedRatisContainerReturnsTrue() - throws InvalidStateTransitionException, IOException, TimeoutException { + throws IOException { long keyCount = 0L; long bytesUsed = 123L; ContainerInfo containerInfo = ReplicationTestUtil.createContainerInfo( @@ -118,7 +118,7 @@ public void testEmptyAndClosedRatisContainerReturnsTrue() */ @Test public void testEmptyAndNonClosedECContainerReturnsFalse() - throws InvalidStateTransitionException, IOException, TimeoutException { + throws IOException { long keyCount = 0L; long bytesUsed = 123L; ContainerInfo containerInfo = ReplicationTestUtil.createContainerInfo( @@ -146,7 +146,7 @@ public void testEmptyAndNonClosedECContainerReturnsFalse() */ @Test public void testNonEmptyRatisContainerReturnsFalse() - throws InvalidStateTransitionException, IOException, TimeoutException { + throws IOException { long keyCount = 5L; long bytesUsed = 123L; ContainerInfo containerInfo = ReplicationTestUtil.createContainerInfo( @@ -173,7 +173,7 @@ public void testNonEmptyRatisContainerReturnsFalse() */ @Test public void testEmptyECContainerWithNonEmptyReplicaReturnsFalse() - throws InvalidStateTransitionException, IOException, TimeoutException { + throws IOException { ContainerInfo containerInfo = ReplicationTestUtil.createContainerInfo( ecReplicationConfig, 1, CLOSED, 0L, 0L); Set containerReplicas = ReplicationTestUtil @@ -211,7 +211,7 @@ public void testEmptyECContainerWithNonEmptyReplicaReturnsFalse() */ private void assertAndVerify(ContainerCheckRequest request, boolean assertion, int times, long numEmptyExpected) - throws IOException, InvalidStateTransitionException, TimeoutException { + throws IOException { Assertions.assertEquals(assertion, emptyContainerHandler.handle(request)); Mockito.verify(replicationManager, Mockito.times(times)) .sendDeleteCommand(Mockito.any(ContainerInfo.class), Mockito.anyInt(), From 48708187ad409ad1be796b74e543ae0ef40d1507 Mon Sep 17 00:00:00 2001 From: Jackson Yao Date: Fri, 28 Oct 2022 11:13:25 +0800 Subject: [PATCH 7/9] fix comments --- .../health/DeletingContainerHandler.java | 2 +- .../health/TestDeletingContainerHandler.java | 36 ++++++------------- 2 files changed, 12 insertions(+), 26 deletions(-) diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/health/DeletingContainerHandler.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/health/DeletingContainerHandler.java index eed8c13938a3..a31e5ecb161f 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/health/DeletingContainerHandler.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/health/DeletingContainerHandler.java @@ -47,7 +47,7 @@ public DeletingContainerHandler(ReplicationManager replicationManager) { } /** - * If the replica size of the container is 0, change the state + * If the number of replicas of the container is 0, change the state * of the container to Deleted, otherwise resend delete command if needed. * @param request ContainerCheckRequest object representing the container * @return false if the specified container is not in DELETING state, diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/health/TestDeletingContainerHandler.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/health/TestDeletingContainerHandler.java index 07b60c1db4e1..e2a147ffd2f6 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/health/TestDeletingContainerHandler.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/health/TestDeletingContainerHandler.java @@ -42,7 +42,6 @@ import java.util.ArrayList; import java.util.Collections; import java.util.HashSet; -import java.util.Iterator; import java.util.List; import java.util.Set; @@ -127,6 +126,8 @@ public void testCleanupIfNoReplicaExist() { HddsProtos.ReplicationFactor.THREE), 1); //ec container + //since updateContainerState is called once when testing + //ratis container, so here should be 1+1 = 2 times cleanupIfNoReplicaExist(ecReplicationConfig, 2); } @@ -166,7 +167,7 @@ public void testNoNeedResendDeleteCommand() throws NotLeaderException { containerReplicas.forEach(r -> pendingOps.add( ContainerReplicaOp.create(ContainerReplicaOp.PendingOpType.DELETE, r.getDatanodeDetails(), r.getReplicaIndex()))); - resendDeleteCommand(containerInfo, containerReplicas, pendingOps, 0); + verifyDeleteCommandCount(containerInfo, containerReplicas, pendingOps, 0); //EC container containerInfo = ReplicationTestUtil.createContainerInfo( @@ -178,7 +179,7 @@ public void testNoNeedResendDeleteCommand() throws NotLeaderException { containerReplicas.forEach(r -> pendingOps.add( ContainerReplicaOp.create(ContainerReplicaOp.PendingOpType.DELETE, r.getDatanodeDetails(), r.getReplicaIndex()))); - resendDeleteCommand(containerInfo, containerReplicas, pendingOps, 0); + verifyDeleteCommandCount(containerInfo, containerReplicas, pendingOps, 0); } @@ -196,16 +197,10 @@ public void testResendDeleteCommand() throws NotLeaderException { .createReplicas(containerInfo.containerID(), ContainerReplicaProto.State.CLOSED, 0, 0, 0); List pendingOps = new ArrayList<>(); - Set tempContainerReplicas = new HashSet<>(); - tempContainerReplicas.addAll(containerReplicas); - Iterator iter = tempContainerReplicas.iterator(); - iter.next(); - iter.remove(); - Assert.assertEquals(2, tempContainerReplicas.size()); - tempContainerReplicas.forEach(r -> pendingOps.add( + containerReplicas.stream().limit(2).forEach(replica -> pendingOps.add( ContainerReplicaOp.create(ContainerReplicaOp.PendingOpType.DELETE, - r.getDatanodeDetails(), r.getReplicaIndex()))); - resendDeleteCommand(containerInfo, containerReplicas, pendingOps, 1); + replica.getDatanodeDetails(), replica.getReplicaIndex()))); + verifyDeleteCommandCount(containerInfo, containerReplicas, pendingOps, 1); //EC container containerInfo = ReplicationTestUtil.createContainerInfo( @@ -214,25 +209,16 @@ public void testResendDeleteCommand() throws NotLeaderException { .createReplicas(containerInfo.containerID(), ContainerReplicaProto.State.CLOSED, 1, 2, 3, 4, 5); pendingOps.clear(); - tempContainerReplicas.clear(); - tempContainerReplicas.addAll(containerReplicas); - iter = tempContainerReplicas.iterator(); - iter.next(); - iter.remove(); - iter.next(); - iter.remove(); - Assert.assertEquals(3, tempContainerReplicas.size()); - - tempContainerReplicas.forEach(r -> pendingOps.add( + containerReplicas.stream().limit(3).forEach(replica -> pendingOps.add( ContainerReplicaOp.create(ContainerReplicaOp.PendingOpType.DELETE, - r.getDatanodeDetails(), r.getReplicaIndex()))); + replica.getDatanodeDetails(), replica.getReplicaIndex()))); //since one delete command is end when testing ratis container, so //here should be 1+2 = 3 times - resendDeleteCommand(containerInfo, containerReplicas, pendingOps, 3); + verifyDeleteCommandCount(containerInfo, containerReplicas, pendingOps, 3); } - private void resendDeleteCommand(ContainerInfo containerInfo, + private void verifyDeleteCommandCount(ContainerInfo containerInfo, Set containerReplicas, List pendingOps, int times) throws NotLeaderException { From 7c189b39a071ececc317bcd2029cbd425831fc9b Mon Sep 17 00:00:00 2001 From: Jackson Yao Date: Fri, 28 Oct 2022 14:13:23 +0800 Subject: [PATCH 8/9] triger CI From 66b5eda4b328f1a46b068bc3de2efcddb4900b78 Mon Sep 17 00:00:00 2001 From: Jackson Yao Date: Fri, 28 Oct 2022 15:43:05 +0800 Subject: [PATCH 9/9] triger CI