From 78b7eb17b2da8ab11d20771c9ee37fbf10320355 Mon Sep 17 00:00:00 2001 From: Duong Nguyen Date: Fri, 6 Jan 2023 10:09:03 -0800 Subject: [PATCH 1/3] HDDS-7738: SCM terminates when adding container to a closed pipeline --- .../hadoop/hdds/scm/pipeline/PipelineStateMap.java | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelineStateMap.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelineStateMap.java index 6b40f28fc0b1..aefc611c8be0 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelineStateMap.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelineStateMap.java @@ -106,9 +106,13 @@ void addContainerToPipeline(PipelineID pipelineID, ContainerID containerID) Pipeline pipeline = getPipeline(pipelineID); if (pipeline.isClosed()) { - throw new IOException(String - .format("Cannot add container to pipeline=%s in closed state", - pipelineID)); + /* + * It should be fine to add a container to a CLOSED pipeline as pipeline + * state can be changed while the container creating transaction is + * waiting to be processed by SCM. + */ + LOG.info("Container {} in open state for pipeline={} in closed state", + containerID, pipelineID); } pipeline2container.get(pipelineID).add(containerID); } From 58313084be18300bd0925cc0bbf197ec5c929401 Mon Sep 17 00:00:00 2001 From: Duong Nguyen Date: Fri, 6 Jan 2023 22:51:09 -0800 Subject: [PATCH 2/3] Refine log statement. --- .../hdds/scm/pipeline/PipelineStateMap.java | 10 +++---- .../scm/pipeline/TestPipelineManagerImpl.java | 26 ++++++++++++++++++- 2 files changed, 28 insertions(+), 8 deletions(-) diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelineStateMap.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelineStateMap.java index aefc611c8be0..a505c5f766c3 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelineStateMap.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelineStateMap.java @@ -106,13 +106,9 @@ void addContainerToPipeline(PipelineID pipelineID, ContainerID containerID) Pipeline pipeline = getPipeline(pipelineID); if (pipeline.isClosed()) { - /* - * It should be fine to add a container to a CLOSED pipeline as pipeline - * state can be changed while the container creating transaction is - * waiting to be processed by SCM. - */ - LOG.info("Container {} in open state for pipeline={} in closed state", - containerID, pipelineID); + LOG.warn("Adding container {} to pipeline={} in CLOSED state." + + " This happens only for some exceptional cases." + + " Check for the previous exceptions.", containerID, pipelineID); } pipeline2container.get(pipelineID).add(containerID); } diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/pipeline/TestPipelineManagerImpl.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/pipeline/TestPipelineManagerImpl.java index 1ed9b845ac25..924cb048b338 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/pipeline/TestPipelineManagerImpl.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/pipeline/TestPipelineManagerImpl.java @@ -762,7 +762,7 @@ public void testSafeModeUpdatedOnSafemodeExit() throws Exception { } @Test - public void testAddContainerWithClosedPipeline() throws Exception { + public void testAddContainerWithClosedPipelineScmStart() throws Exception { GenericTestUtils.LogCapturer logCapturer = GenericTestUtils.LogCapturer. captureLogs(LoggerFactory.getLogger(PipelineStateMap.class)); SCMHADBTransactionBuffer buffer = new SCMHADBTransactionBufferStub(dbStore); @@ -786,6 +786,30 @@ public void testAddContainerWithClosedPipeline() throws Exception { pipelineID + " in closed state")); } + @Test + public void testAddContainerWithClosedPipeline() throws Exception { + GenericTestUtils.LogCapturer logCapturer = GenericTestUtils.LogCapturer. + captureLogs(LoggerFactory.getLogger(PipelineStateMap.class)); + SCMHADBTransactionBuffer buffer = new SCMHADBTransactionBufferStub(dbStore); + PipelineManagerImpl pipelineManager = + createPipelineManager(true, buffer); + Table pipelineStore = + SCMDBDefinition.PIPELINES.getTable(dbStore); + Pipeline pipeline = pipelineManager.createPipeline( + RatisReplicationConfig + .getInstance(HddsProtos.ReplicationFactor.THREE)); + PipelineID pipelineID = pipeline.getId(); + pipelineManager.addContainerToPipeline(pipelineID, ContainerID.valueOf(1)); + pipelineManager.getStateManager().updatePipelineState( + pipelineID.getProtobuf(), HddsProtos.PipelineState.PIPELINE_CLOSED); + buffer.flush(); + Assertions.assertTrue(pipelineStore.get(pipelineID).isClosed()); + pipelineManager.addContainerToPipeline(pipelineID, + ContainerID.valueOf(2)); + assertTrue(logCapturer.getOutput().contains( + "Adding container #2 to pipeline=" + pipelineID +" in CLOSED state.")); + } + @Test public void testPipelineCloseFlow() throws IOException, TimeoutException { GenericTestUtils.LogCapturer logCapturer = GenericTestUtils.LogCapturer From 6635ef7f2c62e7360f0ac5364b61249c8b018639 Mon Sep 17 00:00:00 2001 From: Duong Nguyen Date: Fri, 6 Jan 2023 22:54:48 -0800 Subject: [PATCH 3/3] Fix checkstyle. --- .../org/apache/hadoop/hdds/scm/pipeline/PipelineStateMap.java | 4 ++-- .../hadoop/hdds/scm/pipeline/TestPipelineManagerImpl.java | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelineStateMap.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelineStateMap.java index a505c5f766c3..5db887b6742d 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelineStateMap.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelineStateMap.java @@ -107,8 +107,8 @@ void addContainerToPipeline(PipelineID pipelineID, ContainerID containerID) Pipeline pipeline = getPipeline(pipelineID); if (pipeline.isClosed()) { LOG.warn("Adding container {} to pipeline={} in CLOSED state." + - " This happens only for some exceptional cases." + - " Check for the previous exceptions.", containerID, pipelineID); + " This happens only for some exceptional cases." + + " Check for the previous exceptions.", containerID, pipelineID); } pipeline2container.get(pipelineID).add(containerID); } diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/pipeline/TestPipelineManagerImpl.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/pipeline/TestPipelineManagerImpl.java index 924cb048b338..c75cb937e5b6 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/pipeline/TestPipelineManagerImpl.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/pipeline/TestPipelineManagerImpl.java @@ -807,7 +807,7 @@ public void testAddContainerWithClosedPipeline() throws Exception { pipelineManager.addContainerToPipeline(pipelineID, ContainerID.valueOf(2)); assertTrue(logCapturer.getOutput().contains( - "Adding container #2 to pipeline=" + pipelineID +" in CLOSED state.")); + "Adding container #2 to pipeline=" + pipelineID + " in CLOSED state.")); } @Test