Copilot commented on code in PR #10703: URL: https://github.com/apache/ozone/pull/10703#discussion_r3562272862
########## hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/replication/VolumeReplicationThreadPools.java: ########## @@ -0,0 +1,160 @@ +/* + * 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.ozone.container.replication; + +import com.google.common.annotations.VisibleForTesting; +import com.google.common.util.concurrent.ThreadFactoryBuilder; +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.PriorityBlockingQueue; +import java.util.concurrent.ThreadFactory; +import java.util.concurrent.ThreadPoolExecutor; +import java.util.concurrent.TimeUnit; +import org.apache.hadoop.hdds.utils.HddsServerUtil; +import org.apache.hadoop.ozone.container.common.volume.StorageVolume; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Per-volume replication handler thread pools for push-based replication. + */ +final class VolumeReplicationThreadPools { + + private static final Logger LOG = + LoggerFactory.getLogger(VolumeReplicationThreadPools.class); + + private final ConcurrentHashMap<String, ThreadPoolExecutor> pools = + new ConcurrentHashMap<>(); + private int currentPoolSize; + + void init(Collection<? extends StorageVolume> volumes, int poolSize, + String threadNamePrefix) { + currentPoolSize = poolSize; + List<String> volumeRoots = new ArrayList<>(); + for (StorageVolume volume : volumes) { + String volumeRoot = volume.getStorageDir().getPath(); + volumeRoots.add(volumeRoot); + pools.put(volumeRoot, createPool(poolSize, threadNamePrefix, volumeRoot)); + } + LOG.info("Initialized {} per-volume replication thread pools " + + "(threads per volume = {}): {}", + volumeRoots.size(), poolSize, volumeRoots); + } + + private static ThreadPoolExecutor createPool(int poolSize, + String threadNamePrefix, String volumeRoot) { + String volumeLabel = volumeRoot.substring( + Math.max(0, volumeRoot.lastIndexOf('/') + 1)); + ThreadFactory threadFactory = new ThreadFactoryBuilder() + .setDaemon(true) + .setNameFormat(threadNamePrefix + "ContainerReplicationThread-" + + volumeLabel + "-%d") + .build(); Review Comment: The per-volume thread name currently derives the volume label using `lastIndexOf('/')`, which is not portable across platforms and can embed path separators/colon characters into the thread name. Use `File#getName()` and sanitize the label so thread names remain valid and stable regardless of the underlying path format. ########## hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/replication/TestPerVolumePushReplication.java: ########## @@ -0,0 +1,524 @@ +/* + * 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.ozone.container.replication; + +import static java.nio.charset.StandardCharsets.UTF_8; +import static java.util.Collections.singleton; +import static java.util.Collections.singletonList; +import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_CONTAINER_REPORT_INTERVAL; +import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_HEARTBEAT_INTERVAL; +import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_NODE_REPORT_INTERVAL; +import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_PIPELINE_REPORT_INTERVAL; +import static org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.ContainerDataProto.State.CLOSED; +import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.NodeOperationalState.DECOMMISSIONED; +import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.NodeOperationalState.IN_SERVICE; +import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.NodeState.DEAD; +import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.ReplicationFactor.THREE; +import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_DATANODE_ADMIN_MONITOR_INTERVAL; +import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_DEADNODE_INTERVAL; +import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_HEARTBEAT_PROCESS_INTERVAL; +import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_STALENODE_INTERVAL; +import static org.apache.hadoop.hdds.scm.node.TestNodeUtil.getDNHostAndPort; +import static org.apache.hadoop.hdds.scm.node.TestNodeUtil.waitForDnToReachHealthState; +import static org.apache.hadoop.hdds.scm.node.TestNodeUtil.waitForDnToReachOpState; +import static org.apache.hadoop.hdds.scm.pipeline.MockPipeline.createPipeline; +import static org.apache.hadoop.hdds.scm.storage.ContainerProtocolCalls.createContainer; +import static org.apache.hadoop.ozone.container.TestHelper.waitForContainerClose; +import static org.apache.hadoop.ozone.container.TestHelper.waitForReplicaCount; +import static org.apache.ozone.test.GenericTestUtils.waitFor; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.time.Duration; +import java.util.List; +import java.util.Set; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; +import java.util.concurrent.atomic.AtomicLong; +import java.util.function.ToLongFunction; +import java.util.stream.Collectors; +import org.apache.hadoop.hdds.HddsConfigKeys; +import org.apache.hadoop.hdds.client.BlockID; +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.conf.StorageUnit; +import org.apache.hadoop.hdds.protocol.DatanodeDetails; +import org.apache.hadoop.hdds.protocol.DatanodeID; +import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos; +import org.apache.hadoop.hdds.scm.ScmConfigKeys; +import org.apache.hadoop.hdds.scm.XceiverClientFactory; +import org.apache.hadoop.hdds.scm.XceiverClientManager; +import org.apache.hadoop.hdds.scm.XceiverClientSpi; +import org.apache.hadoop.hdds.scm.cli.ContainerOperationClient; +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.replication.ReplicationManager.ReplicationManagerConfiguration; +import org.apache.hadoop.hdds.scm.node.NodeManager; +import org.apache.hadoop.hdds.scm.pipeline.Pipeline; +import org.apache.hadoop.hdds.scm.pipeline.PipelineManager; +import org.apache.hadoop.hdds.scm.server.StorageContainerManager; +import org.apache.hadoop.hdds.utils.IOUtils; +import org.apache.hadoop.ozone.HddsDatanodeService; +import org.apache.hadoop.ozone.MiniOzoneCluster; +import org.apache.hadoop.ozone.OzoneConfigKeys; +import org.apache.hadoop.ozone.TestDataUtil; +import org.apache.hadoop.ozone.UniformDatanodesFactory; +import org.apache.hadoop.ozone.client.OzoneBucket; +import org.apache.hadoop.ozone.client.OzoneClient; +import org.apache.hadoop.ozone.client.OzoneKeyDetails; +import org.apache.hadoop.ozone.container.ContainerTestHelper; +import org.apache.hadoop.ozone.container.common.interfaces.Container; +import org.apache.hadoop.ozone.container.common.statemachine.DatanodeConfiguration; +import org.apache.hadoop.ozone.container.common.statemachine.DatanodeStateMachine; +import org.apache.hadoop.ozone.container.common.statemachine.StateContext; +import org.apache.hadoop.ozone.container.common.volume.HddsVolume; +import org.apache.hadoop.ozone.container.common.volume.MutableVolumeSet; +import org.apache.hadoop.ozone.container.common.volume.StorageVolume; +import org.apache.hadoop.ozone.dn.DatanodeTestUtils; +import org.apache.hadoop.ozone.protocol.commands.ReplicateContainerCommand; +import org.apache.ozone.test.GenericTestUtils; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.MethodOrderer; +import org.junit.jupiter.api.Order; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInstance; +import org.junit.jupiter.api.TestMethodOrder; +import org.junit.jupiter.api.parallel.Execution; +import org.junit.jupiter.api.parallel.ExecutionMode; +import org.junit.jupiter.api.parallel.ResourceLock; + +/** + * Integration tests for per-volume push replication thread pools (HDDS-15412). + */ +@TestInstance(TestInstance.Lifecycle.PER_CLASS) +@TestMethodOrder(MethodOrderer.OrderAnnotation.class) +@Execution(ExecutionMode.SAME_THREAD) +@ResourceLock("MiniOzoneCluster") +class TestPerVolumePushReplication { + + private static final AtomicLong CONTAINER_ID = new AtomicLong(1_000_000L); + private static final int DATA_VOLUMES = 2; + private static final int DATANODE_COUNT = 7; + private static final String VOLUME = "vol1"; + private static final String BUCKET = "bucket1"; + private static final RatisReplicationConfig RATIS_THREE = + RatisReplicationConfig.getInstance(THREE); + private static final ECReplicationConfig EC_REP = new ECReplicationConfig(3, 2); + + private MiniOzoneCluster cluster; + private XceiverClientFactory clientFactory; + + @BeforeAll + void setUp() throws Exception { + OzoneConfiguration conf = createSharedConfig(); + cluster = newCluster(conf, DATANODE_COUNT); + cluster.waitForClusterToBeReady(); + clientFactory = new XceiverClientManager(conf); + } + + @AfterAll + void tearDown() { + IOUtils.closeQuietly(clientFactory, cluster); + } + + @Order(1) + @Test + void testPushAndScmReplicationWithPerVolumeEnabled() throws Exception { + try (OzoneClient client = cluster.newClient()) { + HddsDatanodeService sourceDn = selectHealthyDatanode(0); + DatanodeDetails source = sourceDn.getDatanodeDetails(); + DatanodeDetails target = selectOtherHealthyNode(source); + long containerId = createClosedContainer(clientFactory, source, 0L); + + ReplicateContainerCommand cmd = + ReplicateContainerCommand.toTarget(containerId, target); + queuePushAndWait(cluster, cmd, source, ReplicationSupervisor::getReplicationSuccessCount); + assertNotNull(getContainer(cluster, target, containerId)); + assertVolumePools(sourceDn, DATA_VOLUMES, 1); + + OzoneBucket bucket = TestDataUtil.createVolumeAndBucket(client, VOLUME, BUCKET); + TestDataUtil.createKey(bucket, "pushKey1", RATIS_THREE, "data".getBytes(UTF_8)); + OzoneKeyDetails keyDetails = bucket.getKey("pushKey1"); + long scmContainerId = keyDetails.getOzoneKeyLocations().get(0).getContainerID(); + waitForContainerClose(cluster, scmContainerId); + + ContainerManager containerManager = cluster.getStorageContainerManager().getContainerManager(); + Set<ContainerReplica> replicas = + containerManager.getContainerReplicas(ContainerID.valueOf(scmContainerId)); + DatanodeDetails replicaDn = replicas.iterator().next().getDatanodeDetails(); + cluster.shutdownHddsDatanode(replicaDn); + waitForReplicaCount(scmContainerId, 3, cluster); + } + } + + @Order(2) + @Test + void testHealthyVolumeReplicationAfterVolumeFailure() throws Exception { + HddsDatanodeService sourceDn = selectHealthyDatanode(1); + DatanodeDetails source = sourceDn.getDatanodeDetails(); + DatanodeDetails target = selectOtherHealthyNode(source); + MutableVolumeSet volSet = sourceDn.getDatanodeStateMachine().getContainer().getVolumeSet(); + HddsVolume vol0 = (HddsVolume) volSet.getVolumesList().get(0); + HddsVolume vol1 = (HddsVolume) volSet.getVolumesList().get(1); + + long containerOnVol0 = findOrCreateContainerOnVolume( + cluster, clientFactory, source, vol0, 0L); + long containerOnVol1 = findOrCreateContainerOnVolume( + cluster, clientFactory, source, vol1, 0L); + + assertVolumePools(sourceDn, DATA_VOLUMES, 1); + + triggerAndWaitForVolumeFailure(volSet, vol0); + waitForVolumePoolState(sourceDn, vol0, vol1); + + ReplicateContainerCommand cmd = + ReplicateContainerCommand.toTarget(containerOnVol1, target); + queuePushAndWait(cluster, cmd, source, ReplicationSupervisor::getReplicationSuccessCount); + assertNotNull(getContainer(cluster, target, containerOnVol1)); + assertEquals(1, volSet.getFailedVolumesList().size()); + + // Container on the failed volume must not replicate successfully. + ReplicateContainerCommand failedVolCmd = + ReplicateContainerCommand.toTarget(containerOnVol0, target); + long previousFailures = sourceDn.getDatanodeStateMachine().getSupervisor() + .getReplicationFailureCount(); + queuePushAndWait(cluster, failedVolCmd, source, + ReplicationSupervisor::getReplicationFailureCount); + assertEquals(previousFailures + 1, + sourceDn.getDatanodeStateMachine().getSupervisor().getReplicationFailureCount()); + + DatanodeTestUtils.restoreBadVolume(vol0); + } + + @Order(3) + @Test + void testDecommissionWithPerVolumePools() throws Exception { + try (OzoneClient client = cluster.newClient(); + ContainerOperationClient scmClient = new ContainerOperationClient(cluster.getConf())) { + StorageContainerManager scm = cluster.getStorageContainerManager(); + NodeManager nm = scm.getScmNodeManager(); + ContainerManager cm = scm.getContainerManager(); + PipelineManager pm = scm.getPipelineManager(); + + OzoneBucket bucket = client.getObjectStore().getVolume(VOLUME).getBucket(BUCKET); + generateData(bucket, 20, "decomKey", RATIS_THREE); + generateData(bucket, 20, "decomEcKey", EC_REP); + + ContainerInfo ratisContainer = waitForKeyContainer(bucket, cm, "decomKey0", 3); + ContainerInfo ecContainer = waitForKeyContainer(bucket, cm, "decomEcKey0", 5); + Pipeline ratisPipeline = pm.getPipeline(ratisContainer.getPipelineID()); + Pipeline ecPipeline = pm.getPipeline(ecContainer.getPipelineID()); + + DatanodeID dnId = ratisPipeline.getNodes().stream() + .filter(node -> ecPipeline.getNodes().contains(node)) + .findFirst() + .orElseThrow(() -> new AssertionError("no intersecting datanode found")) + .getID(); + DatanodeDetails toDecommission = nm.getNode(dnId); + + scmClient.decommissionNodes(singletonList(getDNHostAndPort(toDecommission)), false); + waitForDnToReachOpState(nm, toDecommission, DECOMMISSIONED); + + waitForContainerReplicas(cm, ratisContainer, 4); + waitForContainerReplicas(cm, ecContainer, 6); + + cluster.shutdownHddsDatanode(toDecommission); + waitForDnToReachHealthState(nm, toDecommission, DEAD); + + waitForContainerReplicas(cm, ratisContainer, 3); + waitForContainerReplicas(cm, ecContainer, 5); + + TestDataUtil.createKey(bucket, "sanityKey", RATIS_THREE, + "still healthy".getBytes(StandardCharsets.UTF_8)); + } + } + + private HddsDatanodeService selectHealthyDatanode(int indexAmongHealthy) { + List<HddsDatanodeService> healthy = cluster.getHddsDatanodes().stream() + .filter(this::isHealthyDatanode) + .collect(Collectors.toList()); + if (indexAmongHealthy >= healthy.size()) { + throw new AssertionError("not enough healthy datanodes: requested index " + + indexAmongHealthy + ", found " + healthy.size()); + } + return healthy.get(indexAmongHealthy); + } + + private DatanodeDetails selectOtherHealthyNode(DatanodeDetails source) { + return cluster.getHddsDatanodes().stream() + .filter(this::isHealthyDatanode) + .map(HddsDatanodeService::getDatanodeDetails) + .filter(dn -> !dn.equals(source)) + .findAny() + .orElseThrow(() -> new AssertionError("no target datanode found")); + } + + private boolean isHealthyDatanode(HddsDatanodeService datanode) { + if (datanode.getDatanodeDetails().getPersistedOpState() != IN_SERVICE) { + return false; + } + MutableVolumeSet volumeSet = + datanode.getDatanodeStateMachine().getContainer().getVolumeSet(); + return volumeSet.getFailedVolumesList().isEmpty() + && volumeSet.getVolumesList().size() == DATA_VOLUMES; + } + + private static void assertVolumePools(HddsDatanodeService dn, + int expectedVolumeCount, int expectedPoolSize) { + VolumeReplicationThreadPools pools = + dn.getDatanodeStateMachine().getSupervisor().getVolumeReplicationThreadPools(); + assertNotNull(pools); + List<? extends StorageVolume> volumes = + dn.getDatanodeStateMachine().getContainer().getVolumeSet().getVolumesList(); + assertEquals(expectedVolumeCount, volumes.size()); + for (StorageVolume volume : volumes) { + String volumeRoot = volume.getStorageDir().getPath(); + assertTrue(pools.hasPool(volumeRoot), "missing pool for " + volumeRoot); + assertEquals(expectedPoolSize, pools.getPoolSize(volumeRoot)); + } + } + + private static void waitForVolumePoolState(HddsDatanodeService sourceDn, + HddsVolume failedVolume, HddsVolume healthyVolume) + throws TimeoutException, InterruptedException { + String failedPath = failedVolume.getStorageDir().getPath(); + String healthyPath = healthyVolume.getStorageDir().getPath(); + GenericTestUtils.waitFor(() -> { + VolumeReplicationThreadPools pools = + sourceDn.getDatanodeStateMachine().getSupervisor().getVolumeReplicationThreadPools(); + return pools != null + && !pools.hasPool(failedPath) + && pools.hasPool(healthyPath); + }, 100, 60000); + VolumeReplicationThreadPools pools = + sourceDn.getDatanodeStateMachine().getSupervisor().getVolumeReplicationThreadPools(); + assertNotNull(pools); + assertFalse(pools.hasPool(failedPath)); + assertTrue(pools.hasPool(healthyPath)); + } + + private static void queuePushAndWait(MiniOzoneCluster cluster, + ReplicateContainerCommand cmd, DatanodeDetails source, + ToLongFunction<ReplicationSupervisor> counter) + throws IOException, InterruptedException, TimeoutException { + DatanodeStateMachine stateMachine = cluster.getHddsDatanode(source).getDatanodeStateMachine(); + ReplicationSupervisor supervisor = stateMachine.getSupervisor(); + long previousCount = counter.applyAsLong(supervisor); + StateContext context = stateMachine.getContext(); + context.getTermOfLeaderSCM().ifPresent(cmd::setTerm); + context.addCommand(cmd); + waitFor(() -> counter.applyAsLong(supervisor) == previousCount + 1, 100, 30000); Review Comment: `queuePushAndWait` waits for the counter to become exactly `previousCount + 1`. If any other replication task completes in the background (or if the same command results in more than one task), the counter can jump by more than 1 and this wait condition will never be satisfied, causing a test timeout. Use `>= previousCount + 1` to make the wait robust. ########## hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/replication/TestPerVolumePushReplication.java: ########## @@ -0,0 +1,524 @@ +/* + * 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.ozone.container.replication; + +import static java.nio.charset.StandardCharsets.UTF_8; +import static java.util.Collections.singleton; +import static java.util.Collections.singletonList; +import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_CONTAINER_REPORT_INTERVAL; +import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_HEARTBEAT_INTERVAL; +import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_NODE_REPORT_INTERVAL; +import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_PIPELINE_REPORT_INTERVAL; +import static org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.ContainerDataProto.State.CLOSED; +import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.NodeOperationalState.DECOMMISSIONED; +import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.NodeOperationalState.IN_SERVICE; +import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.NodeState.DEAD; +import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.ReplicationFactor.THREE; +import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_DATANODE_ADMIN_MONITOR_INTERVAL; +import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_DEADNODE_INTERVAL; +import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_HEARTBEAT_PROCESS_INTERVAL; +import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_STALENODE_INTERVAL; +import static org.apache.hadoop.hdds.scm.node.TestNodeUtil.getDNHostAndPort; +import static org.apache.hadoop.hdds.scm.node.TestNodeUtil.waitForDnToReachHealthState; +import static org.apache.hadoop.hdds.scm.node.TestNodeUtil.waitForDnToReachOpState; +import static org.apache.hadoop.hdds.scm.pipeline.MockPipeline.createPipeline; +import static org.apache.hadoop.hdds.scm.storage.ContainerProtocolCalls.createContainer; +import static org.apache.hadoop.ozone.container.TestHelper.waitForContainerClose; +import static org.apache.hadoop.ozone.container.TestHelper.waitForReplicaCount; +import static org.apache.ozone.test.GenericTestUtils.waitFor; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.time.Duration; +import java.util.List; +import java.util.Set; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; +import java.util.concurrent.atomic.AtomicLong; +import java.util.function.ToLongFunction; +import java.util.stream.Collectors; +import org.apache.hadoop.hdds.HddsConfigKeys; +import org.apache.hadoop.hdds.client.BlockID; +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.conf.StorageUnit; +import org.apache.hadoop.hdds.protocol.DatanodeDetails; +import org.apache.hadoop.hdds.protocol.DatanodeID; +import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos; +import org.apache.hadoop.hdds.scm.ScmConfigKeys; +import org.apache.hadoop.hdds.scm.XceiverClientFactory; +import org.apache.hadoop.hdds.scm.XceiverClientManager; +import org.apache.hadoop.hdds.scm.XceiverClientSpi; +import org.apache.hadoop.hdds.scm.cli.ContainerOperationClient; +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.replication.ReplicationManager.ReplicationManagerConfiguration; +import org.apache.hadoop.hdds.scm.node.NodeManager; +import org.apache.hadoop.hdds.scm.pipeline.Pipeline; +import org.apache.hadoop.hdds.scm.pipeline.PipelineManager; +import org.apache.hadoop.hdds.scm.server.StorageContainerManager; +import org.apache.hadoop.hdds.utils.IOUtils; +import org.apache.hadoop.ozone.HddsDatanodeService; +import org.apache.hadoop.ozone.MiniOzoneCluster; +import org.apache.hadoop.ozone.OzoneConfigKeys; +import org.apache.hadoop.ozone.TestDataUtil; +import org.apache.hadoop.ozone.UniformDatanodesFactory; +import org.apache.hadoop.ozone.client.OzoneBucket; +import org.apache.hadoop.ozone.client.OzoneClient; +import org.apache.hadoop.ozone.client.OzoneKeyDetails; +import org.apache.hadoop.ozone.container.ContainerTestHelper; +import org.apache.hadoop.ozone.container.common.interfaces.Container; +import org.apache.hadoop.ozone.container.common.statemachine.DatanodeConfiguration; +import org.apache.hadoop.ozone.container.common.statemachine.DatanodeStateMachine; +import org.apache.hadoop.ozone.container.common.statemachine.StateContext; +import org.apache.hadoop.ozone.container.common.volume.HddsVolume; +import org.apache.hadoop.ozone.container.common.volume.MutableVolumeSet; +import org.apache.hadoop.ozone.container.common.volume.StorageVolume; +import org.apache.hadoop.ozone.dn.DatanodeTestUtils; +import org.apache.hadoop.ozone.protocol.commands.ReplicateContainerCommand; +import org.apache.ozone.test.GenericTestUtils; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.MethodOrderer; +import org.junit.jupiter.api.Order; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInstance; +import org.junit.jupiter.api.TestMethodOrder; +import org.junit.jupiter.api.parallel.Execution; +import org.junit.jupiter.api.parallel.ExecutionMode; +import org.junit.jupiter.api.parallel.ResourceLock; + +/** + * Integration tests for per-volume push replication thread pools (HDDS-15412). + */ +@TestInstance(TestInstance.Lifecycle.PER_CLASS) +@TestMethodOrder(MethodOrderer.OrderAnnotation.class) +@Execution(ExecutionMode.SAME_THREAD) +@ResourceLock("MiniOzoneCluster") +class TestPerVolumePushReplication { + + private static final AtomicLong CONTAINER_ID = new AtomicLong(1_000_000L); + private static final int DATA_VOLUMES = 2; + private static final int DATANODE_COUNT = 7; + private static final String VOLUME = "vol1"; + private static final String BUCKET = "bucket1"; + private static final RatisReplicationConfig RATIS_THREE = + RatisReplicationConfig.getInstance(THREE); + private static final ECReplicationConfig EC_REP = new ECReplicationConfig(3, 2); + + private MiniOzoneCluster cluster; + private XceiverClientFactory clientFactory; + + @BeforeAll + void setUp() throws Exception { + OzoneConfiguration conf = createSharedConfig(); + cluster = newCluster(conf, DATANODE_COUNT); + cluster.waitForClusterToBeReady(); + clientFactory = new XceiverClientManager(conf); + } + + @AfterAll + void tearDown() { + IOUtils.closeQuietly(clientFactory, cluster); + } + + @Order(1) + @Test + void testPushAndScmReplicationWithPerVolumeEnabled() throws Exception { + try (OzoneClient client = cluster.newClient()) { + HddsDatanodeService sourceDn = selectHealthyDatanode(0); + DatanodeDetails source = sourceDn.getDatanodeDetails(); + DatanodeDetails target = selectOtherHealthyNode(source); + long containerId = createClosedContainer(clientFactory, source, 0L); + + ReplicateContainerCommand cmd = + ReplicateContainerCommand.toTarget(containerId, target); + queuePushAndWait(cluster, cmd, source, ReplicationSupervisor::getReplicationSuccessCount); + assertNotNull(getContainer(cluster, target, containerId)); + assertVolumePools(sourceDn, DATA_VOLUMES, 1); + + OzoneBucket bucket = TestDataUtil.createVolumeAndBucket(client, VOLUME, BUCKET); + TestDataUtil.createKey(bucket, "pushKey1", RATIS_THREE, "data".getBytes(UTF_8)); + OzoneKeyDetails keyDetails = bucket.getKey("pushKey1"); + long scmContainerId = keyDetails.getOzoneKeyLocations().get(0).getContainerID(); + waitForContainerClose(cluster, scmContainerId); + + ContainerManager containerManager = cluster.getStorageContainerManager().getContainerManager(); + Set<ContainerReplica> replicas = + containerManager.getContainerReplicas(ContainerID.valueOf(scmContainerId)); + DatanodeDetails replicaDn = replicas.iterator().next().getDatanodeDetails(); + cluster.shutdownHddsDatanode(replicaDn); + waitForReplicaCount(scmContainerId, 3, cluster); + } + } + + @Order(2) + @Test + void testHealthyVolumeReplicationAfterVolumeFailure() throws Exception { + HddsDatanodeService sourceDn = selectHealthyDatanode(1); + DatanodeDetails source = sourceDn.getDatanodeDetails(); + DatanodeDetails target = selectOtherHealthyNode(source); + MutableVolumeSet volSet = sourceDn.getDatanodeStateMachine().getContainer().getVolumeSet(); + HddsVolume vol0 = (HddsVolume) volSet.getVolumesList().get(0); + HddsVolume vol1 = (HddsVolume) volSet.getVolumesList().get(1); + + long containerOnVol0 = findOrCreateContainerOnVolume( + cluster, clientFactory, source, vol0, 0L); + long containerOnVol1 = findOrCreateContainerOnVolume( + cluster, clientFactory, source, vol1, 0L); + + assertVolumePools(sourceDn, DATA_VOLUMES, 1); + + triggerAndWaitForVolumeFailure(volSet, vol0); + waitForVolumePoolState(sourceDn, vol0, vol1); + + ReplicateContainerCommand cmd = + ReplicateContainerCommand.toTarget(containerOnVol1, target); + queuePushAndWait(cluster, cmd, source, ReplicationSupervisor::getReplicationSuccessCount); + assertNotNull(getContainer(cluster, target, containerOnVol1)); + assertEquals(1, volSet.getFailedVolumesList().size()); + + // Container on the failed volume must not replicate successfully. + ReplicateContainerCommand failedVolCmd = + ReplicateContainerCommand.toTarget(containerOnVol0, target); + long previousFailures = sourceDn.getDatanodeStateMachine().getSupervisor() + .getReplicationFailureCount(); + queuePushAndWait(cluster, failedVolCmd, source, + ReplicationSupervisor::getReplicationFailureCount); + assertEquals(previousFailures + 1, + sourceDn.getDatanodeStateMachine().getSupervisor().getReplicationFailureCount()); Review Comment: After making the wait condition tolerant of counter jumps, the strict `assertEquals(previousFailures + 1, ...)` can still be flaky if other failures are recorded in parallel. Assert that the failure count increased by at least 1 instead of exactly 1. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
