From 0dbfd5e2ca46b2a026f8ea9da5ef3dd6592248c6 Mon Sep 17 00:00:00 2001 From: markgui Date: Mon, 17 Jan 2022 15:42:45 +0800 Subject: [PATCH 1/4] HDDS-6190. Cleanup unnecessary datanode id path checks. --- .../hadoop/ozone/HddsDatanodeService.java | 21 ------------------- ...SetNodeOperationalStateCommandHandler.java | 11 ---------- .../states/datanode/InitDatanodeState.java | 7 ------- .../common/TestDatanodeStateMachine.java | 5 ----- .../hadoop/hdds/utils/HddsServerUtil.java | 5 +++-- 5 files changed, 3 insertions(+), 46 deletions(-) diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/HddsDatanodeService.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/HddsDatanodeService.java index 58c6e728460d..1ba4eb9d2518 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/HddsDatanodeService.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/HddsDatanodeService.java @@ -45,7 +45,6 @@ import org.apache.hadoop.hdds.protocol.DatanodeDetails; import org.apache.hadoop.hdds.protocol.proto.SCMSecurityProtocolProtos.SCMGetCertResponseProto; import org.apache.hadoop.hdds.protocolPB.SCMSecurityProtocolClientSideTranslatorPB; -import org.apache.hadoop.hdds.scm.ScmConfigKeys; import org.apache.hadoop.hdds.security.x509.SecurityConfig; import org.apache.hadoop.hdds.security.x509.certificate.client.CertificateClient; import org.apache.hadoop.hdds.security.x509.certificate.client.DNCertificateClient; @@ -453,16 +452,6 @@ public PKCS10CertificationRequest getCSR(ConfigurationSource config) private DatanodeDetails initializeDatanodeDetails() throws IOException { String idFilePath = HddsServerUtil.getDatanodeIdFilePath(conf); - if (idFilePath == null || idFilePath.isEmpty()) { - LOG.error("A valid path is needed for config setting {}", - ScmConfigKeys.OZONE_SCM_DATANODE_ID_DIR); - throw new IllegalArgumentException( - ScmConfigKeys.OZONE_SCM_DATANODE_ID_DIR + - " must be defined. See" + - " https://wiki.apache.org/hadoop/Ozone#Configuration" + - " for details on configuring Ozone."); - } - Preconditions.checkNotNull(idFilePath); File idFile = new File(idFilePath); if (idFile.exists()) { @@ -487,16 +476,6 @@ private DatanodeDetails initializeDatanodeDetails() private void persistDatanodeDetails(DatanodeDetails dnDetails) throws IOException { String idFilePath = HddsServerUtil.getDatanodeIdFilePath(conf); - if (idFilePath == null || idFilePath.isEmpty()) { - LOG.error("A valid path is needed for config setting {}", - ScmConfigKeys.OZONE_SCM_DATANODE_ID_DIR); - throw new IllegalArgumentException( - ScmConfigKeys.OZONE_SCM_DATANODE_ID_DIR + - " must be defined. See" + - " https://wiki.apache.org/hadoop/Ozone#Configuration" + - " for details on configuring Ozone."); - } - Preconditions.checkNotNull(idFilePath); File idFile = new File(idFilePath); ContainerUtils.writeDatanodeDetailsTo(dnDetails, idFile); diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/commandhandler/SetNodeOperationalStateCommandHandler.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/commandhandler/SetNodeOperationalStateCommandHandler.java index 4a46d5fb8965..a7172ffd9914 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/commandhandler/SetNodeOperationalStateCommandHandler.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/commandhandler/SetNodeOperationalStateCommandHandler.java @@ -21,7 +21,6 @@ import org.apache.hadoop.hdds.conf.ConfigurationSource; import org.apache.hadoop.hdds.protocol.DatanodeDetails; import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos; -import org.apache.hadoop.hdds.scm.ScmConfigKeys; import org.apache.hadoop.hdds.utils.HddsServerUtil; import org.apache.hadoop.ozone.container.common.helpers.ContainerUtils; import org.apache.hadoop.ozone.container.common.statemachine.SCMConnectionManager; @@ -107,16 +106,6 @@ public void handle(SCMCommand command, OzoneContainer container, private void persistDatanodeDetails(DatanodeDetails dnDetails) throws IOException { String idFilePath = HddsServerUtil.getDatanodeIdFilePath(conf); - if (idFilePath == null || idFilePath.isEmpty()) { - LOG.error("A valid path is needed for config setting {}", - ScmConfigKeys.OZONE_SCM_DATANODE_ID_DIR); - throw new IllegalArgumentException( - ScmConfigKeys.OZONE_SCM_DATANODE_ID_DIR + - " must be defined. See" + - " https://wiki.apache.org/hadoop/Ozone#Configuration" + - " for details on configuring Ozone."); - } - Preconditions.checkNotNull(idFilePath); File idFile = new File(idFilePath); ContainerUtils.writeDatanodeDetailsTo(dnDetails, idFile); diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/states/datanode/InitDatanodeState.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/states/datanode/InitDatanodeState.java index 217592ddccd2..858c5fd0da16 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/states/datanode/InitDatanodeState.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/states/datanode/InitDatanodeState.java @@ -29,7 +29,6 @@ import org.apache.hadoop.hdds.conf.ConfigurationSource; import org.apache.hadoop.hdds.protocol.DatanodeDetails; -import org.apache.hadoop.hdds.scm.ScmConfigKeys; import org.apache.hadoop.hdds.utils.HddsServerUtil; import org.apache.hadoop.ozone.container.common.helpers.ContainerUtils; import org.apache.hadoop.ozone.container.common.statemachine.DatanodeStateMachine; @@ -123,12 +122,6 @@ public DatanodeStateMachine.DatanodeStates call() throws Exception { */ private void persistContainerDatanodeDetails() { String dataNodeIDPath = HddsServerUtil.getDatanodeIdFilePath(conf); - if (Strings.isNullOrEmpty(dataNodeIDPath)) { - LOG.error("A valid path is needed for config setting {}", - ScmConfigKeys.OZONE_SCM_DATANODE_ID_DIR); - this.context.setState(DatanodeStateMachine.DatanodeStates.SHUTDOWN); - return; - } File idPath = new File(dataNodeIDPath); DatanodeDetails datanodeDetails = this.context.getParent() .getDatanodeDetails(); diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/TestDatanodeStateMachine.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/TestDatanodeStateMachine.java index 82c9e6e1f7e7..1337f28ad945 100644 --- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/TestDatanodeStateMachine.java +++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/TestDatanodeStateMachine.java @@ -379,7 +379,6 @@ public void testDatanodeStateMachineWithInvalidConfiguration() throws Exception { List> confList = new ArrayList<>(); - confList.add(Maps.immutableEntry(ScmConfigKeys.OZONE_SCM_NAMES, "")); // Invalid ozone.scm.names /** Empty **/ @@ -394,10 +393,6 @@ public void testDatanodeStateMachineWithInvalidConfiguration() /** Port out of range **/ confList.add(Maps.immutableEntry( ScmConfigKeys.OZONE_SCM_NAMES, "scm:123456")); - // Invalid ozone.scm.datanode.id.dir - /** Empty **/ - confList.add(Maps.immutableEntry( - ScmConfigKeys.OZONE_SCM_DATANODE_ID_DIR, "")); confList.forEach((entry) -> { OzoneConfiguration perTestConf = new OzoneConfiguration(conf); diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/HddsServerUtil.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/HddsServerUtil.java index 30c2395514dd..98821d96156e 100644 --- a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/HddsServerUtil.java +++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/HddsServerUtil.java @@ -32,6 +32,7 @@ import java.util.stream.Collectors; import java.util.stream.Stream; +import com.google.common.base.Strings; import com.google.protobuf.BlockingService; import org.apache.commons.compress.archivers.ArchiveEntry; import org.apache.commons.compress.archivers.ArchiveOutputStream; @@ -397,8 +398,8 @@ public static Collection getDatanodeStorageDirs( */ public static String getDatanodeIdFilePath(ConfigurationSource conf) { String dataNodeIDDirPath = - conf.get(ScmConfigKeys.OZONE_SCM_DATANODE_ID_DIR); - if (dataNodeIDDirPath == null) { + conf.getTrimmed(ScmConfigKeys.OZONE_SCM_DATANODE_ID_DIR); + if (Strings.isNullOrEmpty(dataNodeIDDirPath)) { File metaDirPath = ServerUtils.getOzoneMetaDirPath(conf); if (metaDirPath == null) { // this means meta data is not found, in theory should not happen at From 875e2bc140c847eec8bcff3974b988c1db482176 Mon Sep 17 00:00:00 2001 From: markgui Date: Thu, 10 Feb 2022 10:47:12 +0800 Subject: [PATCH 2/4] Add unit test for HddsServerUtil.getDatanodeIdFilePath. --- .../hadoop/hdds/scm/TestHddsServerUtils.java | 38 ++++++++++++++++--- 1 file changed, 33 insertions(+), 5 deletions(-) diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/TestHddsServerUtils.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/TestHddsServerUtils.java index f7386500fd16..6a289bd64e86 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/TestHddsServerUtils.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/TestHddsServerUtils.java @@ -31,11 +31,9 @@ import org.apache.hadoop.test.PathUtils; import org.apache.commons.io.FileUtils; -import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_CLIENT_ADDRESS_KEY; -import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_DATANODE_ADDRESS_KEY; -import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_DATANODE_PORT_DEFAULT; -import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_NAMES; -import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_STALENODE_INTERVAL; + +import static org.apache.hadoop.hdds.scm.ScmConfigKeys.*; +import static org.apache.hadoop.ozone.OzoneConsts.OZONE_SCM_DATANODE_ID_FILE_DEFAULT; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; import org.junit.Rule; @@ -219,4 +217,34 @@ public void testGetStaleNodeInterval() { // the min limit value will be returned assertEquals(90000, HddsServerUtil.getStaleNodeInterval(conf)); } + + @Test + public void testGetDatanodeIdFilePath() { + final File testDir = PathUtils.getTestDir(TestHddsServerUtils.class); + final File metaDir = new File(testDir, "metaDir"); + OzoneConfiguration conf = new OzoneConfiguration(); + conf.set(HddsConfigKeys.OZONE_METADATA_DIRS, metaDir.getPath()); + + try { + // test fallback if not set + assertEquals(new File(metaDir, + OZONE_SCM_DATANODE_ID_FILE_DEFAULT).toString(), + HddsServerUtil.getDatanodeIdFilePath(conf)); + + // test fallback if set empty + conf.set(OZONE_SCM_DATANODE_ID_DIR, ""); + assertEquals(new File(metaDir, + OZONE_SCM_DATANODE_ID_FILE_DEFAULT).toString(), + HddsServerUtil.getDatanodeIdFilePath(conf)); + + // test use specific value if set + final File dnIdDir = new File(testDir, "datanodeIDDir"); + conf.set(OZONE_SCM_DATANODE_ID_DIR, dnIdDir.getPath()); + assertEquals(new File(dnIdDir, + OZONE_SCM_DATANODE_ID_FILE_DEFAULT).toString(), + HddsServerUtil.getDatanodeIdFilePath(conf)); + } finally { + FileUtils.deleteQuietly(metaDir); + } + } } From 54cd1d8c6928bc136311aad8ebfb7378627cfa2e Mon Sep 17 00:00:00 2001 From: markgui Date: Thu, 10 Feb 2022 13:22:49 +0800 Subject: [PATCH 3/4] retrigger ci From 3c5f0fb3f018893982a5920cb1da572e9c87fdf3 Mon Sep 17 00:00:00 2001 From: markgui Date: Thu, 10 Feb 2022 20:30:06 +0800 Subject: [PATCH 4/4] Fix imports. --- .../org/apache/hadoop/hdds/scm/TestHddsServerUtils.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/TestHddsServerUtils.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/TestHddsServerUtils.java index 6a289bd64e86..73b879befa84 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/TestHddsServerUtils.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/TestHddsServerUtils.java @@ -32,7 +32,12 @@ import org.apache.commons.io.FileUtils; -import static org.apache.hadoop.hdds.scm.ScmConfigKeys.*; +import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_CLIENT_ADDRESS_KEY; +import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_DATANODE_ADDRESS_KEY; +import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_DATANODE_PORT_DEFAULT; +import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_NAMES; +import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_STALENODE_INTERVAL; +import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_DATANODE_ID_DIR; import static org.apache.hadoop.ozone.OzoneConsts.OZONE_SCM_DATANODE_ID_FILE_DEFAULT; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue;