From 1df0bf666a3fa2ecb06c134503c33599aa8fc003 Mon Sep 17 00:00:00 2001 From: jianghuazhu <740087514@qq.com> Date: Sat, 31 Aug 2024 17:22:26 +0800 Subject: [PATCH 1/4] HDDS-11396. NPE in Handler#clusterId --- .../container/common/impl/HddsDispatcher.java | 10 +++---- .../container/common/interfaces/Handler.java | 23 ++++++++++++++-- .../container/keyvalue/KeyValueHandler.java | 4 +-- .../keyvalue/TestKeyValueHandler.java | 27 +++++++++++++++++++ 4 files changed, 54 insertions(+), 10 deletions(-) diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java index c5855b38b74e..e3174026b5cf 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java @@ -647,12 +647,10 @@ public Handler getHandler(ContainerProtos.ContainerType containerType) { @Override public void setClusterId(String clusterId) { - Preconditions.checkNotNull(clusterId, "clusterId Cannot be null"); - if (this.clusterId == null) { - this.clusterId = clusterId; - for (Map.Entry handlerMap : handlers.entrySet()) { - handlerMap.getValue().setClusterID(clusterId); - } + Preconditions.checkNotNull(clusterId, "clusterId cannot be null"); + this.clusterId = clusterId; + for (Map.Entry handlerMap : handlers.entrySet()) { + handlerMap.getValue().setClusterID(clusterId); } } diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/interfaces/Handler.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/interfaces/Handler.java index bfdff69be46f..95fe63efa732 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/interfaces/Handler.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/interfaces/Handler.java @@ -21,7 +21,9 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.util.concurrent.locks.ReentrantReadWriteLock; +import com.google.common.base.Preconditions; import org.apache.hadoop.hdds.conf.ConfigurationSource; import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos; import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.ContainerCommandRequestProto; @@ -51,11 +53,13 @@ public abstract class Handler { protected final ConfigurationSource conf; protected final ContainerSet containerSet; protected final VolumeSet volumeSet; - protected String clusterId; + private String clusterId; protected final ContainerMetrics metrics; protected String datanodeId; private IncrementalReportSender icrSender; + private ReentrantReadWriteLock lock = new ReentrantReadWriteLock(); + protected Handler(ConfigurationSource config, String datanodeId, ContainerSet contSet, VolumeSet volumeSet, ContainerMetrics containerMetrics, @@ -217,8 +221,23 @@ public abstract void deleteUnreferenced(Container container, long localID) public abstract boolean isFinalizedBlockExist(Container container, long localID); + public String getClusterId() { + lock.readLock().lock(); + try { + return clusterId; + } finally { + lock.readLock().unlock(); + } + } + public void setClusterID(String clusterID) { - this.clusterId = clusterID; + Preconditions.checkNotNull(clusterID, "clusterId cannot be null"); + lock.writeLock().lock(); + try { + this.clusterId = clusterID; + } finally { + lock.writeLock().unlock(); + } } } diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java index 402e1be4cd0f..c71e6456fa0f 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java @@ -377,7 +377,7 @@ ContainerCommandResponseProto handleCreateContainer( containerIdLock.lock(); try { if (containerSet.getContainer(containerID) == null) { - newContainer.create(volumeSet, volumeChoosingPolicy, clusterId); + newContainer.create(volumeSet, volumeChoosingPolicy, getClusterId()); created = containerSet.addContainer(newContainer); } else { // The create container request for an already existing container can @@ -408,7 +408,7 @@ private void populateContainerPathFields(KeyValueContainer container, volumeSet.readLock(); try { String idDir = VersionedDatanodeFeatures.ScmHA.chooseContainerPathID( - hddsVolume, clusterId); + hddsVolume, getClusterId()); container.populatePathFields(idDir, hddsVolume); } finally { volumeSet.readUnlock(); diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueHandler.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueHandler.java index 2637f1922c68..8fc939b2bd65 100644 --- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueHandler.java +++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueHandler.java @@ -62,6 +62,7 @@ import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.any; import org.junit.jupiter.api.BeforeEach; @@ -346,6 +347,32 @@ public void testCloseInvalidContainer(ContainerLayoutVersion layoutVersion) "Close container should return Invalid container error"); } + @Test + public void testClusterId() throws IOException { + OzoneConfiguration conf = new OzoneConfiguration(); + MutableVolumeSet volumeSet = new MutableVolumeSet( + UUID.randomUUID().toString(), conf, null, + StorageVolume.VolumeType.DATA_VOLUME, null); + ContainerSet cset = new ContainerSet(1000); + int[] interval = new int[]{2}; + ContainerMetrics metrics = new ContainerMetrics(interval); + DatanodeDetails datanodeDetails = mock(DatanodeDetails.class); + StateContext context = ContainerTestUtils.getMockContext( + datanodeDetails, conf); + KeyValueHandler keyValueHandler = new KeyValueHandler(conf, + context.getParent().getDatanodeDetails().getUuidString(), cset, + volumeSet, metrics, c -> { + }); + String tmpClusterId = "test_cluster1"; + keyValueHandler.setClusterID(tmpClusterId); + assertEquals(keyValueHandler.getClusterId(), tmpClusterId); + try { + keyValueHandler.setClusterID(null); + } catch (Exception e) { + assertTrue(true); + } + } + @Test public void testDeleteContainer() throws IOException { final String testDir = tempDir.toString(); From 42b2cc3bcf2377338487d9cb3b396e819bd169d5 Mon Sep 17 00:00:00 2001 From: jianghuazhu <740087514@qq.com> Date: Mon, 2 Sep 2024 23:38:59 +0800 Subject: [PATCH 2/4] Update some code. --- .../hadoop/ozone/container/ozoneimpl/OzoneContainer.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/OzoneContainer.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/OzoneContainer.java index 5cdeaaa57870..e40fa635c121 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/OzoneContainer.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/OzoneContainer.java @@ -478,10 +478,10 @@ public void start(String clusterId) throws IOException { replicationServer.start(); datanodeDetails.setPort(Name.REPLICATION, replicationServer.getPort()); - writeChannel.start(); - readChannel.start(); hddsDispatcher.init(); hddsDispatcher.setClusterId(clusterId); + writeChannel.start(); + readChannel.start(); blockDeletingService.start(); recoveringContainerScrubbingService.start(); From 209da3724a21f92159d1d689d422c3342e35a26d Mon Sep 17 00:00:00 2001 From: jianghuazhu <740087514@qq.com> Date: Thu, 19 Sep 2024 21:12:27 +0800 Subject: [PATCH 3/4] Update some code. --- .../container/common/interfaces/Handler.java | 23 ++-------------- .../container/keyvalue/KeyValueHandler.java | 4 +-- .../keyvalue/TestKeyValueHandler.java | 27 ------------------- 3 files changed, 4 insertions(+), 50 deletions(-) diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/interfaces/Handler.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/interfaces/Handler.java index 95fe63efa732..bfdff69be46f 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/interfaces/Handler.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/interfaces/Handler.java @@ -21,9 +21,7 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; -import java.util.concurrent.locks.ReentrantReadWriteLock; -import com.google.common.base.Preconditions; import org.apache.hadoop.hdds.conf.ConfigurationSource; import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos; import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.ContainerCommandRequestProto; @@ -53,13 +51,11 @@ public abstract class Handler { protected final ConfigurationSource conf; protected final ContainerSet containerSet; protected final VolumeSet volumeSet; - private String clusterId; + protected String clusterId; protected final ContainerMetrics metrics; protected String datanodeId; private IncrementalReportSender icrSender; - private ReentrantReadWriteLock lock = new ReentrantReadWriteLock(); - protected Handler(ConfigurationSource config, String datanodeId, ContainerSet contSet, VolumeSet volumeSet, ContainerMetrics containerMetrics, @@ -221,23 +217,8 @@ public abstract void deleteUnreferenced(Container container, long localID) public abstract boolean isFinalizedBlockExist(Container container, long localID); - public String getClusterId() { - lock.readLock().lock(); - try { - return clusterId; - } finally { - lock.readLock().unlock(); - } - } - public void setClusterID(String clusterID) { - Preconditions.checkNotNull(clusterID, "clusterId cannot be null"); - lock.writeLock().lock(); - try { - this.clusterId = clusterID; - } finally { - lock.writeLock().unlock(); - } + this.clusterId = clusterID; } } diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java index c71e6456fa0f..402e1be4cd0f 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java @@ -377,7 +377,7 @@ ContainerCommandResponseProto handleCreateContainer( containerIdLock.lock(); try { if (containerSet.getContainer(containerID) == null) { - newContainer.create(volumeSet, volumeChoosingPolicy, getClusterId()); + newContainer.create(volumeSet, volumeChoosingPolicy, clusterId); created = containerSet.addContainer(newContainer); } else { // The create container request for an already existing container can @@ -408,7 +408,7 @@ private void populateContainerPathFields(KeyValueContainer container, volumeSet.readLock(); try { String idDir = VersionedDatanodeFeatures.ScmHA.chooseContainerPathID( - hddsVolume, getClusterId()); + hddsVolume, clusterId); container.populatePathFields(idDir, hddsVolume); } finally { volumeSet.readUnlock(); diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueHandler.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueHandler.java index 8fc939b2bd65..2637f1922c68 100644 --- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueHandler.java +++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueHandler.java @@ -62,7 +62,6 @@ import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.any; import org.junit.jupiter.api.BeforeEach; @@ -347,32 +346,6 @@ public void testCloseInvalidContainer(ContainerLayoutVersion layoutVersion) "Close container should return Invalid container error"); } - @Test - public void testClusterId() throws IOException { - OzoneConfiguration conf = new OzoneConfiguration(); - MutableVolumeSet volumeSet = new MutableVolumeSet( - UUID.randomUUID().toString(), conf, null, - StorageVolume.VolumeType.DATA_VOLUME, null); - ContainerSet cset = new ContainerSet(1000); - int[] interval = new int[]{2}; - ContainerMetrics metrics = new ContainerMetrics(interval); - DatanodeDetails datanodeDetails = mock(DatanodeDetails.class); - StateContext context = ContainerTestUtils.getMockContext( - datanodeDetails, conf); - KeyValueHandler keyValueHandler = new KeyValueHandler(conf, - context.getParent().getDatanodeDetails().getUuidString(), cset, - volumeSet, metrics, c -> { - }); - String tmpClusterId = "test_cluster1"; - keyValueHandler.setClusterID(tmpClusterId); - assertEquals(keyValueHandler.getClusterId(), tmpClusterId); - try { - keyValueHandler.setClusterID(null); - } catch (Exception e) { - assertTrue(true); - } - } - @Test public void testDeleteContainer() throws IOException { final String testDir = tempDir.toString(); From 9a88605a2820c3f9e99a9725b5c8b9f336f0fea1 Mon Sep 17 00:00:00 2001 From: jianghuazhu <740087514@qq.com> Date: Fri, 20 Sep 2024 17:32:44 +0800 Subject: [PATCH 4/4] Update some code. --- .../ozone/container/common/impl/HddsDispatcher.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java index e3174026b5cf..5fc971841554 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java @@ -648,9 +648,11 @@ public Handler getHandler(ContainerProtos.ContainerType containerType) { @Override public void setClusterId(String clusterId) { Preconditions.checkNotNull(clusterId, "clusterId cannot be null"); - this.clusterId = clusterId; - for (Map.Entry handlerMap : handlers.entrySet()) { - handlerMap.getValue().setClusterID(clusterId); + if (this.clusterId == null) { + this.clusterId = clusterId; + for (Map.Entry handlerMap : handlers.entrySet()) { + handlerMap.getValue().setClusterID(clusterId); + } } }