diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java index 100c9feb9b67..1c2066bcc1e2 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java @@ -181,7 +181,6 @@ import org.apache.hadoop.ipc_.RPC; import org.apache.hadoop.metrics2.MetricsSystem; import org.apache.hadoop.metrics2.util.MBeans; -import org.apache.hadoop.net.CachedDNSToSwitchMapping; import org.apache.hadoop.net.DNSToSwitchMapping; import org.apache.hadoop.net.NetUtils; import org.apache.hadoop.net.ScriptBasedMapping; @@ -755,15 +754,7 @@ private void initializeSystemManagers(OzoneConfiguration conf, .build(); } - Class dnsToSwitchMappingClass = - conf.getClass( - ScmConfigKeys.NET_TOPOLOGY_NODE_SWITCH_MAPPING_IMPL_KEY, - ScriptBasedMapping.class, DNSToSwitchMapping.class); - DNSToSwitchMapping newInstance = ReflectionUtils.newInstance( - dnsToSwitchMappingClass, conf); - dnsToSwitchMapping = - ((newInstance instanceof CachedDNSToSwitchMapping) ? newInstance - : new CachedDNSToSwitchMapping(newInstance)); + dnsToSwitchMapping = createDNSToSwitchMapping(conf); if (configurator.getScmNodeManager() != null) { scmNodeManager = configurator.getScmNodeManager(); @@ -2375,4 +2366,12 @@ public String resolveNodeLocation(String hostname) { } } + static DNSToSwitchMapping createDNSToSwitchMapping(OzoneConfiguration conf) { + Class mappingClass = + conf.getClass( + ScmConfigKeys.NET_TOPOLOGY_NODE_SWITCH_MAPPING_IMPL_KEY, + ScriptBasedMapping.class, DNSToSwitchMapping.class); + return ReflectionUtils.newInstance(mappingClass, conf); + } + } diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/server/TestStorageContainerManager.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/server/TestStorageContainerManager.java new file mode 100644 index 000000000000..5953d59c657c --- /dev/null +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/server/TestStorageContainerManager.java @@ -0,0 +1,56 @@ +/* + * 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.hdds.scm.server; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; + +import org.apache.hadoop.hdds.conf.OzoneConfiguration; +import org.apache.hadoop.hdds.scm.ScmConfigKeys; +import org.apache.hadoop.net.CachedDNSToSwitchMapping; +import org.apache.hadoop.net.DNSToSwitchMapping; +import org.apache.hadoop.net.ScriptBasedMapping; +import org.apache.hadoop.net.StaticMapping; +import org.junit.jupiter.api.Test; + +class TestStorageContainerManager { + + @Test + void defaultMappingKeepsCachedBehavior() { + DNSToSwitchMapping mapping = + StorageContainerManager.createDNSToSwitchMapping( + new OzoneConfiguration()); + + assertInstanceOf(ScriptBasedMapping.class, mapping); + assertInstanceOf(CachedDNSToSwitchMapping.class, mapping); + } + + @Test + void configuredMappingIsUsedDirectly() { + OzoneConfiguration conf = new OzoneConfiguration(); + conf.setClass(ScmConfigKeys.NET_TOPOLOGY_NODE_SWITCH_MAPPING_IMPL_KEY, + StaticMapping.class, DNSToSwitchMapping.class); + + DNSToSwitchMapping mapping = + StorageContainerManager.createDNSToSwitchMapping(conf); + + assertInstanceOf(StaticMapping.class, mapping); + assertFalse(mapping instanceof CachedDNSToSwitchMapping, + "Configured mapping should not be wrapped in CachedDNSToSwitchMapping"); + } +} diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java index f24a5c470fb7..75dec9360f0f 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java @@ -144,7 +144,6 @@ import org.apache.hadoop.hdds.utils.db.TableIterator; import org.apache.hadoop.hdds.utils.db.cache.CacheKey; import org.apache.hadoop.hdds.utils.db.cache.CacheValue; -import org.apache.hadoop.net.CachedDNSToSwitchMapping; import org.apache.hadoop.net.DNSToSwitchMapping; import org.apache.hadoop.net.ScriptBasedMapping; import org.apache.hadoop.ozone.OmUtils; @@ -379,15 +378,15 @@ public void start(OzoneConfiguration configuration) { keyLifecycleService.start(); } - Class dnsToSwitchMappingClass = - configuration.getClass( + dnsToSwitchMapping = createDNSToSwitchMapping(configuration); + } + + static DNSToSwitchMapping createDNSToSwitchMapping(OzoneConfiguration conf) { + Class mappingClass = + conf.getClass( ScmConfigKeys.NET_TOPOLOGY_NODE_SWITCH_MAPPING_IMPL_KEY, ScriptBasedMapping.class, DNSToSwitchMapping.class); - DNSToSwitchMapping newInstance = ReflectionUtils.newInstance( - dnsToSwitchMappingClass, configuration); - dnsToSwitchMapping = - ((newInstance instanceof CachedDNSToSwitchMapping) ? newInstance - : new CachedDNSToSwitchMapping(newInstance)); + return ReflectionUtils.newInstance(mappingClass, conf); } /** diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerImpl.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerImpl.java index 4883591d4013..7aaf8e6c0831 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerImpl.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerImpl.java @@ -21,6 +21,8 @@ import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.DELETED_TABLE; import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.SNAPSHOT_RENAMED_TABLE; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; @@ -35,11 +37,17 @@ import java.util.stream.Collectors; import java.util.stream.Stream; import org.apache.hadoop.hdds.conf.OzoneConfiguration; +import org.apache.hadoop.hdds.scm.ScmConfigKeys; import org.apache.hadoop.hdds.utils.MapBackedTableIterator; import org.apache.hadoop.hdds.utils.db.Table; +import org.apache.hadoop.net.CachedDNSToSwitchMapping; +import org.apache.hadoop.net.DNSToSwitchMapping; +import org.apache.hadoop.net.ScriptBasedMapping; +import org.apache.hadoop.net.StaticMapping; import org.apache.hadoop.ozone.om.helpers.OmKeyInfo; import org.apache.hadoop.ozone.om.helpers.RepeatedOmKeyInfo; import org.apache.ratis.util.function.CheckedFunction; +import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.MethodSource; @@ -47,6 +55,31 @@ * Test class for unit tests KeyManagerImpl. */ public class TestKeyManagerImpl { + + @Test + void defaultMappingKeepsCachedBehavior() { + DNSToSwitchMapping mapping = + KeyManagerImpl.createDNSToSwitchMapping( + new OzoneConfiguration()); + + assertInstanceOf(ScriptBasedMapping.class, mapping); + assertInstanceOf(CachedDNSToSwitchMapping.class, mapping); + } + + @Test + void configuredMappingIsUsedDirectly() { + OzoneConfiguration conf = new OzoneConfiguration(); + conf.setClass(ScmConfigKeys.NET_TOPOLOGY_NODE_SWITCH_MAPPING_IMPL_KEY, + StaticMapping.class, DNSToSwitchMapping.class); + + DNSToSwitchMapping mapping = + KeyManagerImpl.createDNSToSwitchMapping(conf); + + assertInstanceOf(StaticMapping.class, mapping); + assertFalse(mapping instanceof CachedDNSToSwitchMapping, + "Configured mapping should not be wrapped in CachedDNSToSwitchMapping"); + } + private static Stream getSuccessfulTableIteratorParameters() { return Stream.of( TestCase.newBuilder("Fetch first 50 entries for volume 0, bucket 0")