From 6bfc869788f0fa8f01e288365d77c81d7c1b262b Mon Sep 17 00:00:00 2001 From: Swaminathan Balachandran Date: Wed, 12 Feb 2025 04:59:07 -0800 Subject: [PATCH 1/5] HDDS-12236. ContainerStateMachine should not apply future transactions in the event of failure Change-Id: Ifb7ca9c4c1d4de40d56fd02885b6f5ae909816f2 --- .../server/ratis/ContainerStateMachine.java | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/ContainerStateMachine.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/ContainerStateMachine.java index a03253116212..643fc1947ed7 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/ContainerStateMachine.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/ContainerStateMachine.java @@ -31,6 +31,7 @@ import java.util.Map; import java.util.Objects; import java.util.Optional; +import java.util.Set; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionException; import java.util.concurrent.ConcurrentHashMap; @@ -197,6 +198,7 @@ long getStartTime() { private final ExecutorService executor; private final List chunkExecutors; private final Map applyTransactionCompletionMap; + private final Set unhealthyContainers; private final Cache stateMachineDataCache; private final AtomicBoolean stateMachineHealthy; @@ -226,6 +228,7 @@ public ContainerStateMachine(HddsDatanodeService hddsDatanodeService, RaftGroupI metrics = CSMMetrics.create(gid); this.writeChunkFutureMap = new ConcurrentHashMap<>(); applyTransactionCompletionMap = new ConcurrentHashMap<>(); + this.unhealthyContainers = ConcurrentHashMap.newKeySet(); long pendingRequestsBytesLimit = (long)conf.getStorageSize( OzoneConfigKeys.HDDS_CONTAINER_RATIS_LEADER_PENDING_BYTES_LIMIT, OzoneConfigKeys.HDDS_CONTAINER_RATIS_LEADER_PENDING_BYTES_LIMIT_DEFAULT, @@ -360,6 +363,13 @@ public boolean isStateMachineHealthy() { return stateMachineHealthy.get(); } + private void checkContainerHealthy(long containerId) throws StorageContainerException { + if (!isStateMachineHealthy() && unhealthyContainers.contains(containerId)) { + throw new StorageContainerException(String.format("Prev writes to container %d failed, stopping all writes to " + + "container", containerId), ContainerProtos.Result.CONTAINER_UNHEALTHY); + } + } + @Override public long takeSnapshot() throws IOException { TermIndex ti = getLastAppliedTermIndex(); @@ -554,6 +564,11 @@ private CompletableFuture writeStateMachineData( CompletableFuture writeChunkFuture = CompletableFuture.supplyAsync(() -> { try { + try { + checkContainerHealthy(write.getBlockID().getContainerID()); + } catch (StorageContainerException e) { + return ContainerUtils.logAndReturnError(LOG, e, requestProto); + } metrics.recordWriteStateMachineQueueingLatencyNs( Time.monotonicNowNanos() - startTime); return dispatchCommand(requestProto, context); @@ -564,6 +579,7 @@ private CompletableFuture writeStateMachineData( metrics.incNumWriteDataFails(); // write chunks go in parallel. It's possible that one write chunk // see the stateMachine is marked unhealthy by other parallel thread + unhealthyContainers.add(write.getBlockID().getContainerID()); stateMachineHealthy.set(false); raftFuture.completeExceptionally(e); throw e; @@ -596,6 +612,7 @@ private CompletableFuture writeStateMachineData( // This leads to pipeline close. Any change in that behavior requires // handling the entry for the write chunk in cache. stateMachineHealthy.set(false); + unhealthyContainers.add(write.getBlockID().getContainerID()); raftFuture.completeExceptionally(sce); } else { metrics.incNumBytesWrittenCount( @@ -763,6 +780,7 @@ private ByteString readStateMachineData( + "{} Container Result: {}", getGroupId(), response.getCmdType(), index, response.getMessage(), response.getResult()); stateMachineHealthy.set(false); + unhealthyContainers.add(requestProto.getContainerID()); throw sce; } @@ -945,6 +963,7 @@ private CompletableFuture applyTransaction( try { try { this.validatePeers(); + this.checkContainerHealthy(containerId); } catch (StorageContainerException e) { return ContainerUtils.logAndReturnError(LOG, e, request); } @@ -1031,6 +1050,7 @@ public CompletableFuture applyTransaction(TransactionContext trx) { LOG.error(getGroupId() + ": failed to applyTransaction at logIndex " + index + " for " + requestProto.getCmdType(), e); stateMachineHealthy.compareAndSet(true, false); + unhealthyContainers.add(requestProto.getContainerID()); metrics.incNumApplyTransactionsFails(); applyTransactionFuture.completeExceptionally(e); }; @@ -1065,6 +1085,7 @@ public CompletableFuture applyTransaction(TransactionContext trx) { // shutdown. applyTransactionFuture.completeExceptionally(sce); stateMachineHealthy.compareAndSet(true, false); + unhealthyContainers.add(requestProto.getContainerID()); ratisServer.handleApplyTransactionFailure(getGroupId(), trx.getServerRole()); } else { if (LOG.isDebugEnabled()) { From 9f6a89e06065eca3d1a60792e84b3b003c36b3dc Mon Sep 17 00:00:00 2001 From: Swaminathan Balachandran Date: Thu, 27 Feb 2025 21:19:42 -0800 Subject: [PATCH 2/5] HDDS-12236. Fail Write statemachine data in the event of failure Change-Id: Id0933a306379046cd438fb50e04b238a8113ea8b --- .../transport/server/ratis/ContainerStateMachine.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/ContainerStateMachine.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/ContainerStateMachine.java index 643fc1947ed7..e1d793efbdc0 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/ContainerStateMachine.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/ContainerStateMachine.java @@ -363,8 +363,8 @@ public boolean isStateMachineHealthy() { return stateMachineHealthy.get(); } - private void checkContainerHealthy(long containerId) throws StorageContainerException { - if (!isStateMachineHealthy() && unhealthyContainers.contains(containerId)) { + private void checkContainerHealthy(long containerId, boolean skipContainerUnhealthyCheck) throws StorageContainerException { + if (!isStateMachineHealthy() && (skipContainerUnhealthyCheck || unhealthyContainers.contains(containerId))) { throw new StorageContainerException(String.format("Prev writes to container %d failed, stopping all writes to " + "container", containerId), ContainerProtos.Result.CONTAINER_UNHEALTHY); } @@ -565,7 +565,7 @@ private CompletableFuture writeStateMachineData( CompletableFuture.supplyAsync(() -> { try { try { - checkContainerHealthy(write.getBlockID().getContainerID()); + checkContainerHealthy(write.getBlockID().getContainerID(), true); } catch (StorageContainerException e) { return ContainerUtils.logAndReturnError(LOG, e, requestProto); } @@ -963,7 +963,7 @@ private CompletableFuture applyTransaction( try { try { this.validatePeers(); - this.checkContainerHealthy(containerId); + this.checkContainerHealthy(containerId, false); } catch (StorageContainerException e) { return ContainerUtils.logAndReturnError(LOG, e, request); } From 452eaddaf6962b8210fe6729fcbbca8e3b47b99b Mon Sep 17 00:00:00 2001 From: Swaminathan Balachandran Date: Wed, 5 Mar 2025 17:50:52 -0800 Subject: [PATCH 3/5] HDDS-12236. Add test cases Change-Id: If2f7616314465c9f019f9ff568882ef2af396b2d --- .../server/ratis/ContainerStateMachine.java | 8 +- .../ratis/TestContainerStateMachine.java | 251 ++++++++++++++++++ .../TestContainerStateMachineFollower.java | 27 ++ .../TestContainerStateMachineLeader.java | 27 ++ 4 files changed, 311 insertions(+), 2 deletions(-) create mode 100644 hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/TestContainerStateMachine.java create mode 100644 hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/TestContainerStateMachineFollower.java create mode 100644 hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/TestContainerStateMachineLeader.java diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/ContainerStateMachine.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/ContainerStateMachine.java index e1d793efbdc0..15992353241a 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/ContainerStateMachine.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/ContainerStateMachine.java @@ -363,10 +363,14 @@ public boolean isStateMachineHealthy() { return stateMachineHealthy.get(); } - private void checkContainerHealthy(long containerId, boolean skipContainerUnhealthyCheck) throws StorageContainerException { - if (!isStateMachineHealthy() && (skipContainerUnhealthyCheck || unhealthyContainers.contains(containerId))) { + private void checkContainerHealthy(long containerId, boolean skipContainerUnhealthyCheck) + throws StorageContainerException { + if (!isStateMachineHealthy() && unhealthyContainers.contains(containerId)) { throw new StorageContainerException(String.format("Prev writes to container %d failed, stopping all writes to " + "container", containerId), ContainerProtos.Result.CONTAINER_UNHEALTHY); + } else if (!isStateMachineHealthy() && skipContainerUnhealthyCheck) { + throw new StorageContainerException(String.format("Prev writes to containers %s failed, stopping all writes to " + + "container", unhealthyContainers.toString()), ContainerProtos.Result.CONTAINER_UNHEALTHY); } } diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/TestContainerStateMachine.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/TestContainerStateMachine.java new file mode 100644 index 000000000000..2086406dd890 --- /dev/null +++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/TestContainerStateMachine.java @@ -0,0 +1,251 @@ +package org.apache.hadoop.ozone.container.common.transport.server.ratis; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.reset; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import com.google.common.util.concurrent.ThreadFactoryBuilder; +import java.io.IOException; +import java.util.List; +import java.util.UUID; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.LinkedBlockingQueue; +import java.util.concurrent.ThreadPoolExecutor; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Function; +import java.util.stream.Collectors; +import java.util.stream.IntStream; +import org.apache.hadoop.hdds.conf.OzoneConfiguration; +import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos; +import org.apache.hadoop.hdds.scm.container.common.helpers.StorageContainerException; +import org.apache.hadoop.ozone.container.common.interfaces.ContainerDispatcher; +import org.apache.hadoop.ozone.container.ozoneimpl.ContainerController; +import org.apache.ratis.proto.RaftProtos; +import org.apache.ratis.protocol.Message; +import org.apache.ratis.protocol.RaftGroup; +import org.apache.ratis.protocol.RaftGroupId; +import org.apache.ratis.protocol.RaftPeer; +import org.apache.ratis.server.DivisionInfo; +import org.apache.ratis.server.RaftServer; +import org.apache.ratis.statemachine.TransactionContext; +import org.apache.ratis.thirdparty.com.google.protobuf.ByteString; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.TestInstance; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +/* + * 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. + */ + +/** + * Test class to ContainerStateMachine class. + */ +@TestInstance(TestInstance.Lifecycle.PER_CLASS) +abstract class TestContainerStateMachine { + private ContainerDispatcher dispatcher; + private static XceiverServerRatis ratisServer; + private static ContainerController controller; + private OzoneConfiguration conf = new OzoneConfiguration(); + private ContainerStateMachine stateMachine; + private List executor = IntStream.range(0, 2).mapToObj(i -> new ThreadPoolExecutor(1, 1, + 0, TimeUnit.SECONDS, new LinkedBlockingQueue<>(), new ThreadFactoryBuilder() + .setDaemon(true) + .setNameFormat("ChunkWriter-" + i + "-%d") + .build())).collect(Collectors.toList()); + private boolean isLeader; + + TestContainerStateMachine(boolean isLeader) { + this.isLeader = isLeader; + } + + @BeforeEach + public void setup() throws IOException { + dispatcher = mock(ContainerDispatcher.class); + controller = mock(ContainerController.class); + ratisServer = mock(XceiverServerRatis.class); + RaftServer raftServer = mock(RaftServer.class); + RaftServer.Division division = mock(RaftServer.Division.class); + RaftGroup raftGroup = mock(RaftGroup.class); + DivisionInfo info = mock(DivisionInfo.class); + RaftPeer raftPeer = mock(RaftPeer.class); + when(ratisServer.getServer()).thenReturn(raftServer); + when(raftServer.getDivision(any())).thenReturn(division); + when(division.getGroup()).thenReturn(raftGroup); + when(raftGroup.getPeer(any())).thenReturn(raftPeer); + when(division.getInfo()).thenReturn(info); + when(info.isLeader()).thenReturn(isLeader); + when(ratisServer.getServerDivision(any())).thenReturn(division); + stateMachine = new ContainerStateMachine(null, + RaftGroupId.randomId(), dispatcher, controller, executor, ratisServer, conf, "containerOp"); + } + + + @AfterEach + public void teardown() { + stateMachine.close(); + } + + + @AfterAll + public void shutdown() { + executor.forEach(ThreadPoolExecutor::shutdown); + } + + @ParameterizedTest + @ValueSource(booleans = {true, false}) + public void testWriteFailure(boolean failWithException) throws ExecutionException, InterruptedException { + RaftProtos.LogEntryProto entry = mock(RaftProtos.LogEntryProto.class); + when(entry.getTerm()).thenReturn(1L); + when(entry.getIndex()).thenReturn(1L); + TransactionContext trx = mock(TransactionContext.class); + ContainerStateMachine.Context context = mock(ContainerStateMachine.Context.class); + when(trx.getStateMachineContext()).thenReturn(context); + if (failWithException) { + when(dispatcher.dispatch(any(), any())).thenThrow(new RuntimeException()); + } else { + when(dispatcher.dispatch(any(), any())).thenReturn(ContainerProtos.ContainerCommandResponseProto + .newBuilder().setCmdType(ContainerProtos.Type.WriteChunk) + .setResult(ContainerProtos.Result.CONTAINER_INTERNAL_ERROR) + .build()); + } + + when(context.getRequestProto()).thenReturn(ContainerProtos.ContainerCommandRequestProto.newBuilder() + .setCmdType(ContainerProtos.Type.WriteChunk).setWriteChunk( + ContainerProtos.WriteChunkRequestProto.newBuilder().setData(ByteString.copyFromUtf8("Test Data")) + .setBlockID( + ContainerProtos.DatanodeBlockID.newBuilder().setContainerID(1).setLocalID(1).build()).build()) + .setContainerID(1) + .setDatanodeUuid(UUID.randomUUID().toString()).build()); + AtomicReference throwable = new AtomicReference<>(null); + Function throwableSetter = t -> { + throwable.set(t); + return null; + }; + stateMachine.write(entry, trx).exceptionally(throwableSetter).get(); + verify(dispatcher, times(1)).dispatch(any(ContainerProtos.ContainerCommandRequestProto.class), + any(DispatcherContext.class)); + reset(dispatcher); + assertNotNull(throwable.get()); + if (failWithException) { + assertInstanceOf(RuntimeException.class, throwable.get()); + } else { + assertInstanceOf(StorageContainerException.class, throwable.get()); + StorageContainerException sce = (StorageContainerException) throwable.get(); + assertEquals(ContainerProtos.Result.CONTAINER_INTERNAL_ERROR, sce.getResult()); + } + // Writing data to another container(containerId 2) should also fail. + when(context.getRequestProto()).thenReturn(ContainerProtos.ContainerCommandRequestProto.newBuilder() + .setCmdType(ContainerProtos.Type.WriteChunk).setWriteChunk( + ContainerProtos.WriteChunkRequestProto.newBuilder().setData(ByteString.copyFromUtf8("Test Data")) + .setBlockID( + ContainerProtos.DatanodeBlockID.newBuilder().setContainerID(2).setLocalID(1).build()).build()) + .setContainerID(2) + .setDatanodeUuid(UUID.randomUUID().toString()).build()); + stateMachine.write(entry, trx).exceptionally(throwableSetter).get(); + verify(dispatcher, times(0)).dispatch(any(ContainerProtos.ContainerCommandRequestProto.class), + any(DispatcherContext.class)); + assertInstanceOf(StorageContainerException.class, throwable.get()); + StorageContainerException sce = (StorageContainerException) throwable.get(); + assertEquals(ContainerProtos.Result.CONTAINER_UNHEALTHY, sce.getResult()); + } + + @ParameterizedTest + @ValueSource(booleans = {true, false}) + public void testApplyTransactionFailure(boolean failWithException) throws ExecutionException, + InterruptedException, IOException { + RaftProtos.LogEntryProto entry = mock(RaftProtos.LogEntryProto.class); + when(entry.getTerm()).thenReturn(1L); + when(entry.getIndex()).thenReturn(1L); + TransactionContext trx = mock(TransactionContext.class); + ContainerStateMachine.Context context = mock(ContainerStateMachine.Context.class); + when(trx.getLogEntry()).thenReturn(entry); + when(trx.getStateMachineContext()).thenReturn(context); + if (failWithException) { + when(dispatcher.dispatch(any(), any())).thenThrow(new RuntimeException()); + } else { + when(dispatcher.dispatch(any(), any())).thenReturn(ContainerProtos.ContainerCommandResponseProto + .newBuilder().setCmdType(ContainerProtos.Type.WriteChunk) + .setResult(ContainerProtos.Result.CONTAINER_INTERNAL_ERROR) + .build()); + } + // Failing apply transaction on congtainer 1. + when(context.getLogProto()).thenReturn(ContainerProtos.ContainerCommandRequestProto.newBuilder() + .setCmdType(ContainerProtos.Type.WriteChunk).setWriteChunk( + ContainerProtos.WriteChunkRequestProto.newBuilder().setBlockID( + ContainerProtos.DatanodeBlockID.newBuilder().setContainerID(1).setLocalID(1).build()).build()) + .setContainerID(1) + .setDatanodeUuid(UUID.randomUUID().toString()).build()); + AtomicReference throwable = new AtomicReference<>(null); + Function throwableSetter = t -> { + throwable.set(t); + return null; + }; + //apply transaction will fail because of runtime exception thrown by dispatcher, which marks the first + // failure on container 1. + stateMachine.applyTransaction(trx).exceptionally(throwableSetter).get(); + verify(dispatcher, times(1)).dispatch(any(ContainerProtos.ContainerCommandRequestProto.class), + any(DispatcherContext.class)); + reset(dispatcher); + assertNotNull(throwable.get()); + if (failWithException) { + assertInstanceOf(RuntimeException.class, throwable.get()); + } else { + assertInstanceOf(StorageContainerException.class, throwable.get()); + StorageContainerException sce = (StorageContainerException) throwable.get(); + assertEquals(ContainerProtos.Result.CONTAINER_INTERNAL_ERROR, sce.getResult()); + } + // Another apply transaction on same container 1 should fail because the previous apply transaction failed. + stateMachine.applyTransaction(trx).exceptionally(throwableSetter).get(); + verify(dispatcher, times(0)).dispatch(any(ContainerProtos.ContainerCommandRequestProto.class), + any(DispatcherContext.class)); + assertInstanceOf(StorageContainerException.class, throwable.get()); + StorageContainerException sce = (StorageContainerException) throwable.get(); + assertEquals(ContainerProtos.Result.CONTAINER_UNHEALTHY, sce.getResult()); + + // Another apply transaction on a different container 2 shouldn't fail because the previous apply transaction + // failure was only on container 1. + when(context.getLogProto()).thenReturn(ContainerProtos.ContainerCommandRequestProto.newBuilder() + .setCmdType(ContainerProtos.Type.WriteChunk).setWriteChunk( + ContainerProtos.WriteChunkRequestProto.newBuilder().setBlockID( + ContainerProtos.DatanodeBlockID.newBuilder().setContainerID(2).setLocalID(1).build()).build()) + .setContainerID(2) + .setDatanodeUuid(UUID.randomUUID().toString()).build()); + + reset(dispatcher); + throwable.set(null); + when(dispatcher.dispatch(any(), any())).thenReturn(ContainerProtos.ContainerCommandResponseProto + .newBuilder().setCmdType(ContainerProtos.Type.WriteChunk).setResult(ContainerProtos.Result.SUCCESS) + .build()); + Message succcesfulTransaction = stateMachine.applyTransaction(trx).exceptionally(throwableSetter).get(); + verify(dispatcher, times(1)).dispatch(any(ContainerProtos.ContainerCommandRequestProto.class), + any(DispatcherContext.class)); + assertNull(throwable.get()); + ContainerProtos.ContainerCommandResponseProto resp = + ContainerProtos.ContainerCommandResponseProto.parseFrom(succcesfulTransaction.getContent()); + assertEquals(ContainerProtos.Result.SUCCESS, resp.getResult()); + } +} diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/TestContainerStateMachineFollower.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/TestContainerStateMachineFollower.java new file mode 100644 index 000000000000..9a98a517330e --- /dev/null +++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/TestContainerStateMachineFollower.java @@ -0,0 +1,27 @@ +package org.apache.hadoop.ozone.container.common.transport.server.ratis; + +/* + * 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. + */ + +/** + * Test class to ContainerStateMachine class for follower. + */ +public class TestContainerStateMachineFollower extends TestContainerStateMachine { + public TestContainerStateMachineFollower() { + super(false); + } +} diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/TestContainerStateMachineLeader.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/TestContainerStateMachineLeader.java new file mode 100644 index 000000000000..ca7772d2f8e0 --- /dev/null +++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/TestContainerStateMachineLeader.java @@ -0,0 +1,27 @@ +package org.apache.hadoop.ozone.container.common.transport.server.ratis; + +/* + * 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. + */ + +/** + * Test class to ContainerStateMachine class for leader. + */ +public class TestContainerStateMachineLeader extends TestContainerStateMachine { + public TestContainerStateMachineLeader() { + super(true); + } +} From fb210973cb70c3af194840b9b72c47c912bfb148 Mon Sep 17 00:00:00 2001 From: Swaminathan Balachandran Date: Wed, 5 Mar 2025 18:33:42 -0800 Subject: [PATCH 4/5] HDDS-12236. Fix license headers and findbugs in tests Change-Id: Icf79997606e7093c10d07de0a6b35dcea4a863fc --- .../ratis/TestContainerStateMachine.java | 29 ++++++++++++++----- .../TestContainerStateMachineFollower.java | 4 +-- .../TestContainerStateMachineLeader.java | 4 +-- .../client/rpc/TestContainerStateMachine.java | 3 +- 4 files changed, 27 insertions(+), 13 deletions(-) diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/TestContainerStateMachine.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/TestContainerStateMachine.java index 2086406dd890..8a5cf4b91ad7 100644 --- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/TestContainerStateMachine.java +++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/TestContainerStateMachine.java @@ -1,3 +1,20 @@ +/* + * 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.common.transport.server.ratis; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -67,16 +84,14 @@ @TestInstance(TestInstance.Lifecycle.PER_CLASS) abstract class TestContainerStateMachine { private ContainerDispatcher dispatcher; - private static XceiverServerRatis ratisServer; - private static ContainerController controller; - private OzoneConfiguration conf = new OzoneConfiguration(); + private final OzoneConfiguration conf = new OzoneConfiguration(); private ContainerStateMachine stateMachine; - private List executor = IntStream.range(0, 2).mapToObj(i -> new ThreadPoolExecutor(1, 1, + private final List executor = IntStream.range(0, 2).mapToObj(i -> new ThreadPoolExecutor(1, 1, 0, TimeUnit.SECONDS, new LinkedBlockingQueue<>(), new ThreadFactoryBuilder() .setDaemon(true) .setNameFormat("ChunkWriter-" + i + "-%d") .build())).collect(Collectors.toList()); - private boolean isLeader; + private final boolean isLeader; TestContainerStateMachine(boolean isLeader) { this.isLeader = isLeader; @@ -85,8 +100,8 @@ abstract class TestContainerStateMachine { @BeforeEach public void setup() throws IOException { dispatcher = mock(ContainerDispatcher.class); - controller = mock(ContainerController.class); - ratisServer = mock(XceiverServerRatis.class); + ContainerController controller = mock(ContainerController.class); + XceiverServerRatis ratisServer = mock(XceiverServerRatis.class); RaftServer raftServer = mock(RaftServer.class); RaftServer.Division division = mock(RaftServer.Division.class); RaftGroup raftGroup = mock(RaftGroup.class); diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/TestContainerStateMachineFollower.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/TestContainerStateMachineFollower.java index 9a98a517330e..e63dfac3ffed 100644 --- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/TestContainerStateMachineFollower.java +++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/TestContainerStateMachineFollower.java @@ -1,5 +1,3 @@ -package org.apache.hadoop.ozone.container.common.transport.server.ratis; - /* * Licensed to the Apache Software Foundation (ASF) under one or more * contributor license agreements. See the NOTICE file distributed with @@ -17,6 +15,8 @@ * limitations under the License. */ +package org.apache.hadoop.ozone.container.common.transport.server.ratis; + /** * Test class to ContainerStateMachine class for follower. */ diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/TestContainerStateMachineLeader.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/TestContainerStateMachineLeader.java index ca7772d2f8e0..29ded1465b14 100644 --- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/TestContainerStateMachineLeader.java +++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/TestContainerStateMachineLeader.java @@ -1,5 +1,3 @@ -package org.apache.hadoop.ozone.container.common.transport.server.ratis; - /* * Licensed to the Apache Software Foundation (ASF) under one or more * contributor license agreements. See the NOTICE file distributed with @@ -17,6 +15,8 @@ * limitations under the License. */ +package org.apache.hadoop.ozone.container.common.transport.server.ratis; + /** * Test class to ContainerStateMachine class for leader. */ diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestContainerStateMachine.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestContainerStateMachine.java index 4ffc1d4f6529..54e130642b9e 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestContainerStateMachine.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestContainerStateMachine.java @@ -71,7 +71,7 @@ public class TestContainerStateMachine { private MiniOzoneCluster cluster; - private OzoneConfiguration conf = new OzoneConfiguration(); + private final OzoneConfiguration conf = new OzoneConfiguration(); private OzoneClient client; private ObjectStore objectStore; private String volumeName; @@ -84,7 +84,6 @@ public class TestContainerStateMachine { */ @BeforeEach public void setup() throws Exception { - conf.setInt(ScmConfigKeys.OZONE_DATANODE_PIPELINE_LIMIT, 1); conf.setTimeDuration(HDDS_HEARTBEAT_INTERVAL, 200, TimeUnit.MILLISECONDS); conf.setBoolean(HDDS_BLOCK_TOKEN_ENABLED, true); From 9a336fb4c5e9c61afe1c8856105453b66287a902 Mon Sep 17 00:00:00 2001 From: Swaminathan Balachandran Date: Wed, 5 Mar 2025 18:37:45 -0800 Subject: [PATCH 5/5] HDDS-12236. Revert unrelated test file change Change-Id: I9714b3ee6526968e2ffb06f211087f4d9151d671 --- .../hadoop/ozone/client/rpc/TestContainerStateMachine.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestContainerStateMachine.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestContainerStateMachine.java index 54e130642b9e..4ffc1d4f6529 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestContainerStateMachine.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestContainerStateMachine.java @@ -71,7 +71,7 @@ public class TestContainerStateMachine { private MiniOzoneCluster cluster; - private final OzoneConfiguration conf = new OzoneConfiguration(); + private OzoneConfiguration conf = new OzoneConfiguration(); private OzoneClient client; private ObjectStore objectStore; private String volumeName; @@ -84,6 +84,7 @@ public class TestContainerStateMachine { */ @BeforeEach public void setup() throws Exception { + conf.setInt(ScmConfigKeys.OZONE_DATANODE_PIPELINE_LIMIT, 1); conf.setTimeDuration(HDDS_HEARTBEAT_INTERVAL, 200, TimeUnit.MILLISECONDS); conf.setBoolean(HDDS_BLOCK_TOKEN_ENABLED, true);