From df4c96d6e19691416e6318cd6100e3e09a21d02f Mon Sep 17 00:00:00 2001 From: tanvipenumudy Date: Mon, 18 Dec 2023 12:27:45 +0530 Subject: [PATCH 01/23] HDDS-9389. Introduce new API and cache refresh for serving network topology schema to OM --- hadoop-hdds/common/pom.xml | 4 + .../hadoop/hdds/protocol/DatanodeDetails.java | 16 +++ .../apache/hadoop/hdds/scm/net/InnerNode.java | 13 ++ .../hadoop/hdds/scm/net/InnerNodeImpl.java | 121 +++++++++++++++- .../hadoop/hdds/scm/net/NetworkTopology.java | 2 + .../hdds/scm/net/NetworkTopologyImpl.java | 14 ++ .../org/apache/hadoop/hdds/scm/net/Node.java | 20 +++ .../apache/hadoop/hdds/scm/net/NodeImpl.java | 43 ++++++ .../hdds/scm/net/NodeSchemaManager.java | 15 ++ .../apache/hadoop/ozone/OzoneConfigKeys.java | 6 + .../src/main/resources/ozone-default.xml | 8 ++ .../hdds/scm/client/ScmTopologyClient.java | 131 ++++++++++++++++++ .../hadoop/hdds/scm/client/package-info.java | 24 ++++ .../protocol/ScmBlockLocationProtocol.java | 3 + ...ocationProtocolClientSideTranslatorPB.java | 20 +++ .../src/main/proto/ScmServerProtocol.proto | 59 +++++++- .../hadoop/hdds/scm/node/DatanodeInfo.java | 91 ++++++++++++ .../hadoop/hdds/scm/node/NodeStatus.java | 16 +++ ...ocationProtocolServerSideTranslatorPB.java | 15 ++ .../scm/server/SCMBlockProtocolServer.java | 5 + .../hadoop/ozone/TestDelegationToken.java | 4 + .../ozone/TestGetClusterTreeInformation.java | 86 ++++++++++++ .../hadoop/ozone/TestSecureOzoneCluster.java | 11 ++ .../om/TestOmContainerLocationCache.java | 6 + .../om/TestOzoneManagerListVolumesSecure.java | 3 + .../hadoop/ozone/om/KeyManagerImpl.java | 42 +++--- .../apache/hadoop/ozone/om/OzoneManager.java | 43 ++++++ .../hadoop/ozone/om/OmTestManagers.java | 5 + .../om/ScmBlockLocationTestingClient.java | 11 ++ .../hadoop/ozone/om/TestKeyManagerUnit.java | 6 + 30 files changed, 822 insertions(+), 21 deletions(-) create mode 100644 hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/client/ScmTopologyClient.java create mode 100644 hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/client/package-info.java create mode 100644 hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestGetClusterTreeInformation.java diff --git a/hadoop-hdds/common/pom.xml b/hadoop-hdds/common/pom.xml index b1dc93d31ff..c1194140f97 100644 --- a/hadoop-hdds/common/pom.xml +++ b/hadoop-hdds/common/pom.xml @@ -240,6 +240,10 @@ https://maven.apache.org/xsd/maven-4.0.0.xsd"> ${io.grpc.version} compile + + org.apache.ozone + hdds-interface-server + diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/protocol/DatanodeDetails.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/protocol/DatanodeDetails.java index 739f6ebd656..7e8b7bef9e6 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/protocol/DatanodeDetails.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/protocol/DatanodeDetails.java @@ -34,7 +34,9 @@ import org.apache.hadoop.hdds.protocol.DatanodeDetails.Port.Name; import org.apache.hadoop.hdds.protocol.proto.HddsProtos; import org.apache.hadoop.hdds.protocol.proto.HddsProtos.ExtendedDatanodeDetailsProto; +import org.apache.hadoop.hdds.protocol.proto.ScmBlockLocationProtocolProtos; import org.apache.hadoop.hdds.scm.net.NetConstants; +import org.apache.hadoop.hdds.scm.net.Node; import org.apache.hadoop.hdds.scm.net.NodeImpl; import com.google.common.base.Preconditions; @@ -1020,4 +1022,18 @@ public String getBuildDate() { public void setBuildDate(String date) { this.buildDate = date; } + + public ScmBlockLocationProtocolProtos.NodeType toProtobuf(int clientVersion) { + ScmBlockLocationProtocolProtos.NodeType nodeType = + ScmBlockLocationProtocolProtos.NodeType.newBuilder() + .setDatanodeDetails(toProtoBuilder(clientVersion).build()).build(); + return nodeType; + } + + public static Node fromProtobuf( + ScmBlockLocationProtocolProtos.NodeType nodeType) { + return nodeType.hasDatanodeDetails() + ? DatanodeDetails.getFromProtoBuf(nodeType.getDatanodeDetails()) + : null; + } } diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/InnerNode.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/InnerNode.java index c87d826d252..0293ffc6e9e 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/InnerNode.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/InnerNode.java @@ -20,6 +20,8 @@ import java.util.Collection; import java.util.List; +import org.apache.hadoop.hdds.protocol.proto.ScmBlockLocationProtocolProtos; + /** * The interface defines an inner node in a network topology. * An inner node represents network topology entities, such as data center, @@ -89,4 +91,15 @@ N newInnerNode(String name, String location, InnerNode parent, int level, */ Node getLeaf(int leafIndex, List excludedScopes, Collection excludedNodes, int ancestorGen); + + ScmBlockLocationProtocolProtos.NodeType toProtobuf(int clientVersion); + + boolean equals(Object o); + + int hashCode(); + + static InnerNode fromProtobuf( + ScmBlockLocationProtocolProtos.InnerNode innerNode) { + return InnerNodeImpl.fromProtobuf(innerNode); + } } diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/InnerNodeImpl.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/InnerNodeImpl.java index f2648f3d294..df5a92c4f7d 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/InnerNodeImpl.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/InnerNodeImpl.java @@ -27,6 +27,7 @@ import java.util.Map; import com.google.common.base.Preconditions; +import org.apache.hadoop.hdds.protocol.proto.ScmBlockLocationProtocolProtos; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -47,10 +48,10 @@ public InnerNodeImpl newInnerNode(String name, String location, } } - static final Factory FACTORY = new Factory(); + public static final Factory FACTORY = new Factory(); // a map of node's network name to Node for quick search and keep // the insert order - private final HashMap childrenMap = + private HashMap childrenMap = new LinkedHashMap(); // number of descendant leaves under this node private int numOfLeaves; @@ -66,6 +67,69 @@ protected InnerNodeImpl(String name, String location, InnerNode parent, super(name, location, parent, level, cost); } + protected InnerNodeImpl(String name, String location, int cost, + HashMap childrenMap, int numOfLeaves) { + super(name, location, cost); + this.childrenMap = childrenMap; + this.numOfLeaves = numOfLeaves; + } + + /** + * InnerNodeImpl Builder to help construct an InnerNodeImpl object from + * protobuf objects. + */ + public static class Builder { + private String name; + private String location; + private int cost; + private HashMap childrenMap = new LinkedHashMap<>(); + private int numOfLeaves; + + public Builder setName(String name) { + this.name = name; + return this; + } + + public Builder setLocation(String location) { + this.location = location; + return this; + } + + public Builder setCost(int cost) { + this.cost = cost; + return this; + } + + public Builder setChildrenMap(HashMap childrenMap) { + this.childrenMap = childrenMap; + return this; + } + + public Builder setChildrenMap( + List childrenMapList) { + HashMap newChildrenMap = new HashMap<>(); + for (ScmBlockLocationProtocolProtos.ChildrenMap childrenMapProto : + childrenMapList) { + String networkName = childrenMapProto.getNetworkName(); + ScmBlockLocationProtocolProtos.NodeType nodeType = + childrenMapProto.getNodeType(); + Node node = Node.fromProtobuf(nodeType); + newChildrenMap.put(networkName, node); + } + this.childrenMap = newChildrenMap; + return this; + } + + public Builder setNumOfLeaves(int numOfLeaves) { + this.numOfLeaves = numOfLeaves; + return this; + } + + public InnerNodeImpl build() { + return new InnerNodeImpl(name, location, cost, childrenMap, numOfLeaves); + } + } + /** @return the number of children this node has */ private int getNumOfChildren() { return childrenMap.size(); @@ -389,6 +453,59 @@ public Node getLeaf(int leafIndex, List excludedScopes, return null; } + @Override + public ScmBlockLocationProtocolProtos.NodeType toProtobuf(int clientVersion) { + + ScmBlockLocationProtocolProtos.InnerNode.Builder innerNode = + ScmBlockLocationProtocolProtos.InnerNode.newBuilder() + .setNumOfLeaves(numOfLeaves) + .setNodeImpl( + NodeImpl.toProtobuf(getNetworkName(), getNetworkLocation(), + getCost())); + + if (childrenMap != null || !childrenMap.isEmpty()) { + for (Map.Entry entry : childrenMap.entrySet()) { + if (entry.getValue() != null) { + ScmBlockLocationProtocolProtos.ChildrenMap childrenMapProto = + ScmBlockLocationProtocolProtos.ChildrenMap.newBuilder() + .setNetworkName(entry.getKey()) + .setNodeType(entry.getValue().toProtobuf(clientVersion)) + .build(); + innerNode.addChildrenMap(childrenMapProto); + } + } + } + innerNode.build(); + + ScmBlockLocationProtocolProtos.NodeType nodeType = + ScmBlockLocationProtocolProtos.NodeType.newBuilder() + .setInnerNode(innerNode).build(); + + return nodeType; + } + + public static Node fromProtobuf( + ScmBlockLocationProtocolProtos.NodeType nodeType) { + return nodeType.hasInnerNode() + ? InnerNodeImpl.fromProtobuf(nodeType.getInnerNode()) + : null; + } + + public static InnerNode fromProtobuf( + ScmBlockLocationProtocolProtos.InnerNode innerNode) { + + ScmBlockLocationProtocolProtos.NodeImpl nodeImpl = innerNode.getNodeImpl(); + InnerNodeImpl.Builder builder = new InnerNodeImpl.Builder() + .setName(nodeImpl.getName()) + .setLocation(nodeImpl.getLocation()) + .setCost(nodeImpl.getCost()) + .setChildrenMap(innerNode.getChildrenMapList()) + .setNumOfLeaves(innerNode.getNumOfLeaves()); + + InnerNode res = builder.build(); + return res; + } + @Override public boolean equals(Object to) { if (to == null) { diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/NetworkTopology.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/NetworkTopology.java index 5aa9e17825c..218ac14e28e 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/NetworkTopology.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/NetworkTopology.java @@ -96,6 +96,8 @@ public InvalidTopologyException(String msg) { */ Node getNode(String loc); + InnerNode getClusterTree(); + /** * Given a string representation of a InnerNode, return its leaf nodes count. * @param loc a path-like string representation of a InnerNode diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/NetworkTopologyImpl.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/NetworkTopologyImpl.java index 5e0697eaafd..f5d9289c0fc 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/NetworkTopologyImpl.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/NetworkTopologyImpl.java @@ -75,6 +75,15 @@ public NetworkTopologyImpl(ConfigurationSource conf) { schemaManager.getCost(NetConstants.ROOT_LEVEL)); } + public NetworkTopologyImpl(String schemaFile, InnerNode clusterTree) { + schemaManager = NodeSchemaManager.getInstance(); + schemaManager.init(schemaFile); + maxLevel = schemaManager.getMaxLevel(); + shuffleOperation = Collections::shuffle; + factory = InnerNodeImpl.FACTORY; + this.clusterTree = clusterTree; + } + @VisibleForTesting public NetworkTopologyImpl(NodeSchemaManager manager, Consumer> shuffleOperation) { @@ -306,6 +315,11 @@ public Node getNode(String loc) { } } + @Override + public InnerNode getClusterTree() { + return clusterTree; + } + /** * Given a string representation of Node, return its leaf nodes count. * @param loc a path-like string representation of Node diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/Node.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/Node.java index 9884888a1dd..39283486e00 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/Node.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/Node.java @@ -17,6 +17,9 @@ */ package org.apache.hadoop.hdds.scm.net; +import org.apache.hadoop.hdds.protocol.DatanodeDetails; +import org.apache.hadoop.hdds.protocol.proto.ScmBlockLocationProtocolProtos; + /** * The interface defines a node in a network topology. * A node may be a leave representing a data node or an inner @@ -126,4 +129,21 @@ public interface Node { * @return true if this node is under a specific scope */ boolean isDescendant(String nodePath); + + ScmBlockLocationProtocolProtos.NodeType toProtobuf(int clientVersion); + + static Node fromProtobuf( + ScmBlockLocationProtocolProtos.NodeType nodeTypeProto) { + + if (nodeTypeProto.hasDatanodeDetails()) { + return DatanodeDetails.getFromProtoBuf( + nodeTypeProto.getDatanodeDetails()); + } else if (nodeTypeProto.hasNodeImpl()) { + return NodeImpl.fromProtobuf(nodeTypeProto.getNodeImpl()); + } else if (nodeTypeProto.hasInnerNode()) { + return InnerNode.fromProtobuf(nodeTypeProto.getInnerNode()); + } else { + return null; + } + } } diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/NodeImpl.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/NodeImpl.java index e7a45f649b6..5b0f9dfb43e 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/NodeImpl.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/NodeImpl.java @@ -18,6 +18,7 @@ package org.apache.hadoop.hdds.scm.net; import com.google.common.base.Preconditions; +import org.apache.hadoop.hdds.protocol.proto.ScmBlockLocationProtocolProtos; import static org.apache.hadoop.hdds.scm.net.NetConstants.ROOT; import static org.apache.hadoop.hdds.scm.net.NetConstants.PATH_SEPARATOR_STR; @@ -229,6 +230,48 @@ public boolean isDescendant(String nodePath) { NetUtils.addSuffix(nodePath)); } + @Override + public ScmBlockLocationProtocolProtos.NodeType toProtobuf(int clientVersion) { + ScmBlockLocationProtocolProtos.NodeImpl nodeImpl = + ScmBlockLocationProtocolProtos.NodeImpl.newBuilder() + .setName(name) + .setLocation(location) + .setCost(cost) + .build(); + + ScmBlockLocationProtocolProtos.NodeType nodeType = + ScmBlockLocationProtocolProtos.NodeType.newBuilder() + .setNodeImpl(nodeImpl).build(); + + return nodeType; + } + + public static ScmBlockLocationProtocolProtos.NodeImpl toProtobuf(String name, + String location, int cost) { + + ScmBlockLocationProtocolProtos.NodeImpl.Builder nodeImplBuilder = + ScmBlockLocationProtocolProtos.NodeImpl.newBuilder() + .setName(name) + .setLocation(location) + .setCost(cost); + + ScmBlockLocationProtocolProtos.NodeImpl nodeImpl = nodeImplBuilder.build(); + return nodeImpl; + } + + public static Node fromProtobuf( + ScmBlockLocationProtocolProtos.NodeType nodeType) { + return nodeType.hasNodeImpl() + ? InnerNodeImpl.fromProtobuf(nodeType.getNodeImpl()) + : null; + } + + public static NodeImpl fromProtobuf( + ScmBlockLocationProtocolProtos.NodeImpl nodeImpl) { + return new NodeImpl(nodeImpl.getName(), nodeImpl.getLocation(), + nodeImpl.getCost()); + } + @Override public boolean equals(Object to) { if (to == null) { diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/NodeSchemaManager.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/NodeSchemaManager.java index eecd7987672..09443bbe46a 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/NodeSchemaManager.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/NodeSchemaManager.java @@ -76,6 +76,21 @@ public void init(ConfigurationSource conf) { } } + public void init(String schemaFile) { + NodeSchemaLoadResult result; + try { + result = NodeSchemaLoader.getInstance().loadSchemaFromFile(schemaFile); + allSchema = result.getSchemaList(); + enforcePrefix = result.isEnforePrefix(); + maxLevel = allSchema.size(); + } catch (Throwable e) { + String msg = "Failed to load schema file:" + schemaFile + + ", error: " + e.getMessage(); + LOG.error(msg, e); + throw new RuntimeException(msg, e); + } + } + @VisibleForTesting public void init(NodeSchema[] schemas, boolean enforce) { allSchema = new ArrayList<>(); diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConfigKeys.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConfigKeys.java index f124e24141f..f842bcda126 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConfigKeys.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConfigKeys.java @@ -668,6 +668,12 @@ public final class OzoneConfigKeys { public static final String OZONE_SCM_CLOSE_CONTAINER_WAIT_DURATION = "ozone.scm.close.container.wait.duration"; + public static final String + OZONE_SCM_NETWORK_TOPOLOGY_CLUSTER_TREE_REFRESH_DURATION = + "ozone.scm.network.topology.cluster.tree.refresh.duration"; + public static final String + OZONE_SCM_NETWORK_TOPOLOGY_CLUSTER_TREE_REFRESH_DURATION_DEFAULT = "1h"; + /** * There is no need to instantiate this class. */ diff --git a/hadoop-hdds/common/src/main/resources/ozone-default.xml b/hadoop-hdds/common/src/main/resources/ozone-default.xml index 2d410fecd96..56b6afb3e6d 100644 --- a/hadoop-hdds/common/src/main/resources/ozone-default.xml +++ b/hadoop-hdds/common/src/main/resources/ozone-default.xml @@ -3684,6 +3684,14 @@ Wait duration before which close container is send to DN. + + ozone.scm.network.topology.cluster.tree.refresh.duration + 1h + SCM, OZONE, OM + The duration at which we periodically fetch the updated network + topology cluster tree from SCM. + + ozone.scm.ha.ratis.server.snapshot.creation.gap 1024 diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/client/ScmTopologyClient.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/client/ScmTopologyClient.java new file mode 100644 index 00000000000..c0f94447481 --- /dev/null +++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/client/ScmTopologyClient.java @@ -0,0 +1,131 @@ +/** + * 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.client; + +import com.google.common.util.concurrent.ThreadFactoryBuilder; +import org.apache.hadoop.hdds.conf.ConfigurationSource; +import org.apache.hadoop.hdds.scm.net.InnerNode; +import org.apache.hadoop.hdds.scm.protocol.ScmBlockLocationProtocol; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.io.IOException; +import java.io.UncheckedIOException; +import java.time.Duration; +import java.time.Instant; +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ThreadFactory; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; + +import static java.util.Objects.requireNonNull; +import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_SCM_NETWORK_TOPOLOGY_CLUSTER_TREE_REFRESH_DURATION; +import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_SCM_NETWORK_TOPOLOGY_CLUSTER_TREE_REFRESH_DURATION_DEFAULT; + +/** + * This client implements a background thread which periodically checks and + * gets the latest network topology cluster tree from SCM. + */ +public class ScmTopologyClient { + private static final Logger LOG = + LoggerFactory.getLogger(ScmTopologyClient.class); + + private final ScmBlockLocationProtocol scmBlockLocationProtocol; + private final AtomicReference cache = new AtomicReference<>(); + private ScheduledExecutorService executorService; + + public ScmTopologyClient( + ScmBlockLocationProtocol scmBlockLocationProtocol) { + this.scmBlockLocationProtocol = scmBlockLocationProtocol; + } + + public InnerNode getClusterTree() { + return requireNonNull(cache.get(), + "ScmBlockLocationClient must have been initialized already."); + } + + public void refetchClusterTree() { + checkAndRefresh(); + } + + public void start(ConfigurationSource conf) throws IOException { + final InnerNode initialTopology = + scmBlockLocationProtocol.getClusterTree(); + LOG.info("Initial network topology fetched from SCM: {}.", + initialTopology); + cache.set(initialTopology); + scheduleNetworkTopologyPoller(conf, Instant.now()); + } + + public void stop() { + if (executorService != null) { + executorService.shutdown(); + try { + if (executorService.awaitTermination(1, TimeUnit.MINUTES)) { + executorService.shutdownNow(); + } + } catch (InterruptedException e) { + LOG.error("Interrupted while shutting down executor service.", e); + Thread.currentThread().interrupt(); + } + } + } + + private void scheduleNetworkTopologyPoller(ConfigurationSource conf, + Instant initialInvocation) { + Duration refreshDuration = parseRefreshDuration(conf); + Instant nextRefresh = initialInvocation.plus(refreshDuration); + ThreadFactory threadFactory = new ThreadFactoryBuilder() + .setNameFormat("NetworkTopologyPoller") + .setDaemon(true) + .build(); + executorService = Executors.newScheduledThreadPool(1, threadFactory); + Duration initialDelay = Duration.between(Instant.now(), nextRefresh); + + LOG.info("Scheduling NetworkTopologyPoller with an initial delay of {}.", + initialDelay); + executorService.scheduleAtFixedRate(() -> checkAndRefresh(), + initialDelay.toMillis(), refreshDuration.toMillis(), + TimeUnit.MILLISECONDS); + } + + public static Duration parseRefreshDuration(ConfigurationSource conf) { + long refreshDurationInMs = conf.getTimeDuration( + OZONE_SCM_NETWORK_TOPOLOGY_CLUSTER_TREE_REFRESH_DURATION, + OZONE_SCM_NETWORK_TOPOLOGY_CLUSTER_TREE_REFRESH_DURATION_DEFAULT, + TimeUnit.MILLISECONDS); + return Duration.ofMillis(refreshDurationInMs); + } + + private synchronized void checkAndRefresh() { + InnerNode current = cache.get(); + try { + InnerNode newTopology = scmBlockLocationProtocol.getClusterTree(); + if (!newTopology.equals(current)) { + cache.set(newTopology); + LOG.info("Updated network topology schema file fetched from " + + "SCM: {}.", newTopology); + } + } catch (IOException e) { + throw new UncheckedIOException( + "Error fetching updated network topology schema file from SCM", e); + } + } +} diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/client/package-info.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/client/package-info.java new file mode 100644 index 00000000000..8dc9cb3cca2 --- /dev/null +++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/client/package-info.java @@ -0,0 +1,24 @@ +/** + * 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. + *

+ * Freon related helper classes used for load testing. + */ + +/** + * Contains SCM client related classes. + */ +package org.apache.hadoop.hdds.scm.client; diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/protocol/ScmBlockLocationProtocol.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/protocol/ScmBlockLocationProtocol.java index ef2585488fa..d9d1a65a527 100644 --- a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/protocol/ScmBlockLocationProtocol.java +++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/protocol/ScmBlockLocationProtocol.java @@ -22,6 +22,7 @@ import org.apache.hadoop.hdds.scm.AddSCMRequest; import org.apache.hadoop.hdds.scm.ScmConfig; import org.apache.hadoop.hdds.scm.container.common.helpers.ExcludeList; +import org.apache.hadoop.hdds.scm.net.InnerNode; import org.apache.hadoop.security.KerberosInfo; import org.apache.hadoop.hdds.scm.ScmInfo; import org.apache.hadoop.hdds.scm.container.common.helpers.AllocatedBlock; @@ -138,4 +139,6 @@ List allocateBlock(long size, int numBlocks, */ List sortDatanodes(List nodes, String clientMachine) throws IOException; + + InnerNode getClusterTree() throws IOException; } diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/protocolPB/ScmBlockLocationProtocolClientSideTranslatorPB.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/protocolPB/ScmBlockLocationProtocolClientSideTranslatorPB.java index b072c3690b7..4969a8cf6af 100644 --- a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/protocolPB/ScmBlockLocationProtocolClientSideTranslatorPB.java +++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/protocolPB/ScmBlockLocationProtocolClientSideTranslatorPB.java @@ -38,6 +38,8 @@ import org.apache.hadoop.hdds.protocol.proto.ScmBlockLocationProtocolProtos.AllocateScmBlockResponseProto; import org.apache.hadoop.hdds.protocol.proto.ScmBlockLocationProtocolProtos.DeleteScmKeyBlocksRequestProto; import org.apache.hadoop.hdds.protocol.proto.ScmBlockLocationProtocolProtos.DeleteScmKeyBlocksResponseProto; +import org.apache.hadoop.hdds.protocol.proto.ScmBlockLocationProtocolProtos.GetClusterTreeRequestProto; +import org.apache.hadoop.hdds.protocol.proto.ScmBlockLocationProtocolProtos.GetClusterTreeResponseProto; import org.apache.hadoop.hdds.protocol.proto.ScmBlockLocationProtocolProtos.KeyBlocks; import org.apache.hadoop.hdds.protocol.proto.ScmBlockLocationProtocolProtos .SortDatanodesRequestProto; @@ -48,6 +50,8 @@ import org.apache.hadoop.hdds.scm.container.common.helpers.AllocatedBlock; import org.apache.hadoop.hdds.scm.container.common.helpers.ExcludeList; import org.apache.hadoop.hdds.scm.exceptions.SCMException; +import org.apache.hadoop.hdds.scm.net.InnerNode; +import org.apache.hadoop.hdds.scm.net.InnerNodeImpl; import org.apache.hadoop.hdds.scm.pipeline.Pipeline; import org.apache.hadoop.hdds.scm.protocol.ScmBlockLocationProtocol; import org.apache.hadoop.hdds.scm.proxy.SCMBlockLocationFailoverProxyProvider; @@ -324,6 +328,22 @@ public List sortDatanodes(List nodes, return results; } + @Override + public InnerNode getClusterTree() throws IOException { + GetClusterTreeRequestProto request = + GetClusterTreeRequestProto.newBuilder().build(); + SCMBlockLocationRequest wrapper = createSCMBlockRequest(Type.GetClusterTree) + .setGetClusterTreeRequest(request) + .build(); + + final SCMBlockLocationResponse wrappedResponse = + handleError(submitRequest(wrapper)); + GetClusterTreeResponseProto resp = + wrappedResponse.getGetClusterTreeResponse(); + + return InnerNodeImpl.fromProtobuf(resp.getClusterTree()); + } + @Override public Object getUnderlyingProxyObject() { return rpcProxy; diff --git a/hadoop-hdds/interface-server/src/main/proto/ScmServerProtocol.proto b/hadoop-hdds/interface-server/src/main/proto/ScmServerProtocol.proto index 307c23a5620..60cbfe8a1b5 100644 --- a/hadoop-hdds/interface-server/src/main/proto/ScmServerProtocol.proto +++ b/hadoop-hdds/interface-server/src/main/proto/ScmServerProtocol.proto @@ -29,7 +29,7 @@ option java_generate_equals_and_hash = true; package hadoop.hdds.block; import "hdds.proto"; - +import "ScmServerDatanodeHeartbeatProtocol.proto"; // SCM Block protocol @@ -39,6 +39,7 @@ enum Type { GetScmInfo = 13; SortDatanodes = 14; AddScm = 15; + GetClusterTree = 16; } message SCMBlockLocationRequest { @@ -56,6 +57,7 @@ message SCMBlockLocationRequest { optional hadoop.hdds.GetScmInfoRequestProto getScmInfoRequest = 13; optional SortDatanodesRequestProto sortDatanodesRequest = 14; optional hadoop.hdds.AddScmRequestProto addScmRequestProto = 15; + optional GetClusterTreeRequestProto getClusterTreeRequest = 16; } message SCMBlockLocationResponse { @@ -80,6 +82,7 @@ message SCMBlockLocationResponse { optional hadoop.hdds.GetScmInfoResponseProto getScmInfoResponse = 13; optional SortDatanodesResponseProto sortDatanodesResponse = 14; optional hadoop.hdds.AddScmResponseProto addScmResponse = 15; + optional GetClusterTreeResponseProto getClusterTreeResponse = 16; } /** @@ -230,6 +233,60 @@ message SortDatanodesResponseProto{ repeated DatanodeDetailsProto node = 1; } +message NodeImpl { + required string name = 1; + required string location = 2; + required uint32 cost = 3; +} + +message CommandCount { + required SCMCommandProto.Type command = 1; + required uint32 count = 2; +} + +message NodeStatus { + required NodeOperationalState operationalState = 1; + required NodeState health = 2; + required uint64 opStateExpiryEpochSeconds = 3; +} + +message DatanodeInfo { + required DatanodeDetailsProto datanodeDetails = 1; + required uint64 lastHeartbeatTime = 2; + required uint64 lastStatsUpdatedTime = 3; + required uint32 failedVolumeCount = 4; + repeated StorageReportProto storageReportProto = 5; + repeated MetadataStorageReportProto metadataStorageReportProto = 6; + required LayoutVersionProto layoutVersionProto = 7; + repeated CommandCount commandCount = 8; + required NodeStatus nodeStatus = 9; +} + +message NodeType { + optional DatanodeDetailsProto datanodeDetails = 1; + optional DatanodeInfo datanodeInfo = 2; + optional InnerNode innerNode = 3; + optional NodeImpl nodeImpl = 4; +} + +message ChildrenMap { + required string networkName = 1; + required NodeType nodeType = 2; +} + +message InnerNode { + repeated ChildrenMap childrenMap = 1; + optional uint32 numOfLeaves = 2; + required NodeImpl nodeImpl = 3; +} + +message GetClusterTreeRequestProto { +} + +message GetClusterTreeResponseProto { + required InnerNode clusterTree = 1; +} + /** * Protocol used from OzoneManager to StorageContainerManager. * See request and response messages for details of the RPC calls. diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/DatanodeInfo.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/DatanodeInfo.java index 7893e90812d..152b92eaaba 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/DatanodeInfo.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/DatanodeInfo.java @@ -22,6 +22,7 @@ import com.google.common.annotations.VisibleForTesting; import org.apache.hadoop.hdds.protocol.DatanodeDetails; +import org.apache.hadoop.hdds.protocol.proto.ScmBlockLocationProtocolProtos; import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.SCMCommandProto; import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.CommandQueueReportProto; import org.apache.hadoop.hdds.protocol.proto @@ -30,6 +31,8 @@ .StorageContainerDatanodeProtocolProtos.StorageReportProto; import org.apache.hadoop.hdds.protocol.proto .StorageContainerDatanodeProtocolProtos.MetadataStorageReportProto; +import org.apache.hadoop.hdds.scm.net.Node; +import org.apache.hadoop.hdds.scm.net.NodeImpl; import org.apache.hadoop.util.Time; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -82,6 +85,26 @@ public DatanodeInfo(DatanodeDetails datanodeDetails, NodeStatus nodeStatus, this.commandCounts = new HashMap<>(); } + @SuppressWarnings("parameternumber") + private DatanodeInfo(DatanodeDetails datanodeDetails, long lastHeartbeatTime, + long lastStatsUpdatedTime, int failedVolumeCount, + List storageReports, + List metadataStorageReports, + LayoutVersionProto lastKnownLayoutVersion, + Map commandCounts, + NodeStatus nodeStatus) { + super(datanodeDetails); + this.lock = new ReentrantReadWriteLock(); + this.lastHeartbeatTime = lastHeartbeatTime; + this.lastStatsUpdatedTime = lastStatsUpdatedTime; + this.failedVolumeCount = failedVolumeCount; + this.storageReports = storageReports; + this.metadataStorageReports = metadataStorageReports; + this.lastKnownLayoutVersion = lastKnownLayoutVersion; + this.commandCounts = commandCounts; + this.nodeStatus = nodeStatus; + } + /** * Updates the last heartbeat time with current time. */ @@ -355,4 +378,72 @@ public boolean equals(Object obj) { return super.equals(obj); } + @Override + public ScmBlockLocationProtocolProtos.NodeType toProtobuf(int clientVersion) { + ScmBlockLocationProtocolProtos.DatanodeInfo.Builder datanodeInfo = + ScmBlockLocationProtocolProtos.DatanodeInfo.newBuilder() + .setLastHeartbeatTime(lastHeartbeatTime) + .setLastStatsUpdatedTime(lastStatsUpdatedTime) + .setFailedVolumeCount(failedVolumeCount) + .setLayoutVersionProto(lastKnownLayoutVersion) + .setNodeStatus(nodeStatus.toProtobuf()); + + for (StorageReportProto storageReport : storageReports) { + datanodeInfo.addStorageReportProto(storageReport); + } + + for (MetadataStorageReportProto metadataStorageReport : + metadataStorageReports) { + datanodeInfo.addMetadataStorageReportProto(metadataStorageReport); + } + + for (Map.Entry entry : + commandCounts.entrySet()) { + ScmBlockLocationProtocolProtos.CommandCount commandCountProto = + ScmBlockLocationProtocolProtos.CommandCount.newBuilder() + .setCommand(entry.getKey()) + .setCount(entry.getValue()) + .build(); + datanodeInfo.addCommandCount(commandCountProto); + } + + ScmBlockLocationProtocolProtos.NodeType nodeType = + ScmBlockLocationProtocolProtos.NodeType.newBuilder() + .setDatanodeInfo(datanodeInfo).build(); + + return nodeType; + } + + public static Node fromProtobuf( + ScmBlockLocationProtocolProtos.NodeType nodeType) { + return nodeType.hasDatanodeInfo() + ? DatanodeInfo.fromProtobuf(nodeType.getDatanodeInfo()) + : null; + } + + public static NodeImpl fromProtobuf( + ScmBlockLocationProtocolProtos.DatanodeInfo datanodeInfo) { + return new DatanodeInfo( + DatanodeDetails.getFromProtoBuf(datanodeInfo.getDatanodeDetails()), + datanodeInfo.getLastHeartbeatTime(), + datanodeInfo.getLastStatsUpdatedTime(), + datanodeInfo.getFailedVolumeCount(), + datanodeInfo.getStorageReportProtoList(), + datanodeInfo.getMetadataStorageReportProtoList(), + datanodeInfo.getLayoutVersionProto(), + DatanodeInfo.fromProtobuf(datanodeInfo.getCommandCountList()), + NodeStatus.fromProtobuf(datanodeInfo.getNodeStatus())); + } + + public static Map fromProtobuf( + List commandCountList) { + Map commandCounts = new HashMap<>(); + for (ScmBlockLocationProtocolProtos.CommandCount commandCountProto : + commandCountList) { + SCMCommandProto.Type commandType = commandCountProto.getCommand(); + int count = commandCountProto.getCount(); + commandCounts.put(commandType, count); + } + return commandCounts; + } } diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/NodeStatus.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/NodeStatus.java index 7b1d6dd27d3..a5c8bb6c246 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/NodeStatus.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/NodeStatus.java @@ -20,6 +20,7 @@ import com.google.common.collect.ImmutableSet; import org.apache.hadoop.hdds.protocol.proto.HddsProtos; +import org.apache.hadoop.hdds.protocol.proto.ScmBlockLocationProtocolProtos; import java.util.EnumSet; import java.util.Objects; @@ -262,4 +263,19 @@ public int compareTo(NodeStatus o) { return order; } + public ScmBlockLocationProtocolProtos.NodeStatus toProtobuf() { + ScmBlockLocationProtocolProtos.NodeStatus.Builder nodeStatus = + ScmBlockLocationProtocolProtos.NodeStatus.newBuilder() + .setOperationalState(operationalState) + .setHealth(health) + .setOpStateExpiryEpochSeconds(opStateExpiryEpochSeconds); + + return nodeStatus.build(); + } + + public static NodeStatus fromProtobuf( + ScmBlockLocationProtocolProtos.NodeStatus nodeStatus) { + return new NodeStatus(nodeStatus.getOperationalState(), + nodeStatus.getHealth(), nodeStatus.getOpStateExpiryEpochSeconds()); + } } diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/protocol/ScmBlockLocationProtocolServerSideTranslatorPB.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/protocol/ScmBlockLocationProtocolServerSideTranslatorPB.java index 0914cdd90b2..a1b1c93e4b1 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/protocol/ScmBlockLocationProtocolServerSideTranslatorPB.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/protocol/ScmBlockLocationProtocolServerSideTranslatorPB.java @@ -32,6 +32,7 @@ import org.apache.hadoop.hdds.protocol.proto.ScmBlockLocationProtocolProtos.DeleteKeyBlocksResultProto; import org.apache.hadoop.hdds.protocol.proto.ScmBlockLocationProtocolProtos.DeleteScmKeyBlocksRequestProto; import org.apache.hadoop.hdds.protocol.proto.ScmBlockLocationProtocolProtos.DeleteScmKeyBlocksResponseProto; +import org.apache.hadoop.hdds.protocol.proto.ScmBlockLocationProtocolProtos.GetClusterTreeResponseProto; import org.apache.hadoop.hdds.protocol.proto.ScmBlockLocationProtocolProtos.SCMBlockLocationRequest; import org.apache.hadoop.hdds.protocol.proto.ScmBlockLocationProtocolProtos.SCMBlockLocationResponse; import org.apache.hadoop.hdds.protocol.proto.ScmBlockLocationProtocolProtos.SortDatanodesRequestProto; @@ -43,6 +44,7 @@ import org.apache.hadoop.hdds.scm.container.common.helpers.ExcludeList; import org.apache.hadoop.hdds.scm.exceptions.SCMException; import org.apache.hadoop.hdds.scm.ha.RatisUtil; +import org.apache.hadoop.hdds.scm.net.InnerNode; import org.apache.hadoop.hdds.scm.protocolPB.ScmBlockLocationProtocolPB; import org.apache.hadoop.hdds.scm.protocolPB.StorageContainerLocationProtocolPB; import org.apache.hadoop.hdds.scm.server.StorageContainerManager; @@ -159,6 +161,10 @@ private SCMBlockLocationResponse processMessage( request.getSortDatanodesRequest(), request.getVersion() )); break; + case GetClusterTree: + response.setGetClusterTreeResponse( + getClusterTree(request.getVersion())); + break; default: // Should never happen throw new IOException("Unknown Operation " + request.getCmdType() + @@ -276,4 +282,13 @@ public SortDatanodesResponseProto sortDatanodes( throw new ServiceException(ex); } } + + public GetClusterTreeResponseProto getClusterTree(int clientVersion) + throws IOException { + GetClusterTreeResponseProto.Builder resp = + GetClusterTreeResponseProto.newBuilder(); + InnerNode clusterTree = impl.getClusterTree(); + resp.setClusterTree(clusterTree.toProtobuf(clientVersion).getInnerNode()); + return resp.build(); + } } diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMBlockProtocolServer.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMBlockProtocolServer.java index c610c95d2b2..70cf8e01b86 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMBlockProtocolServer.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMBlockProtocolServer.java @@ -407,6 +407,11 @@ private Node getOtherNode(String clientMachine) { return null; } + @Override + public InnerNode getClusterTree() { + return scm.getClusterMap().getClusterTree(); + } + @Override public AuditMessage buildAuditMessageForSuccess( AuditAction op, Map auditMap) { diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestDelegationToken.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestDelegationToken.java index cb6bbc9dd0d..7db57a6b40c 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestDelegationToken.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestDelegationToken.java @@ -31,6 +31,7 @@ import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.hdds.scm.HddsTestUtils; import org.apache.hadoop.hdds.scm.ScmConfig; +import org.apache.hadoop.hdds.scm.client.ScmTopologyClient; import org.apache.hadoop.hdds.scm.ha.HASecurityUtils; import org.apache.hadoop.hdds.scm.server.SCMHTTPServerConfig; import org.apache.hadoop.hdds.scm.server.SCMStorageConfig; @@ -46,6 +47,7 @@ import org.apache.hadoop.minikdc.MiniKdc; import org.apache.hadoop.ozone.om.OMStorage; import org.apache.hadoop.ozone.om.OzoneManager; +import org.apache.hadoop.ozone.om.ScmBlockLocationTestingClient; import org.apache.hadoop.ozone.om.exceptions.OMException; import org.apache.hadoop.ozone.om.protocolPB.OmTransportFactory; import org.apache.hadoop.ozone.om.protocolPB.OzoneManagerProtocolClientSideTranslatorPB; @@ -312,6 +314,8 @@ public void testDelegationToken(boolean useIp) throws Exception { try { // Start OM om.setCertClient(new CertificateClientTestImpl(conf)); + om.setScmTopologyClient(new ScmTopologyClient( + new ScmBlockLocationTestingClient(null, null, 0))); om.start(); UserGroupInformation ugi = UserGroupInformation.getCurrentUser(); String username = ugi.getUserName(); diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestGetClusterTreeInformation.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestGetClusterTreeInformation.java new file mode 100644 index 00000000000..112965830a1 --- /dev/null +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestGetClusterTreeInformation.java @@ -0,0 +1,86 @@ +/** + * 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; + +import org.apache.hadoop.hdds.scm.net.InnerNode; +import org.apache.hadoop.hdds.scm.node.NodeManager; +import org.apache.hadoop.hdds.scm.protocolPB.ScmBlockLocationProtocolClientSideTranslatorPB; +import org.apache.hadoop.hdds.scm.proxy.SCMBlockLocationFailoverProxyProvider; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.Assertions; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.apache.hadoop.hdds.scm.server.StorageContainerManager; +import org.apache.hadoop.hdds.conf.OzoneConfiguration; + +import java.io.IOException; +import java.util.concurrent.TimeoutException; + +/** + * + * This class is to test the serialization/deserialization of cluster tree + * information from SCM. + */ +public class TestGetClusterTreeInformation { + + public static final Logger LOG = + LoggerFactory.getLogger(TestGetClusterTreeInformation.class); + private static int numOfDatanodes = 3; + private static MiniOzoneCluster cluster; + private static OzoneConfiguration conf; + private static StorageContainerManager scm; + private static NodeManager nodeManager; + private static ClassLoader classLoader = + Thread.currentThread().getContextClassLoader(); + + @BeforeAll + public static void init() throws IOException, TimeoutException, + InterruptedException { + conf = new OzoneConfiguration(); + cluster = MiniOzoneCluster.newBuilder(conf) + .setNumDatanodes(numOfDatanodes) + .setNumOfOzoneManagers(3) + .setNumOfStorageContainerManagers(3) + .build(); + cluster.waitForClusterToBeReady(); + scm = cluster.getStorageContainerManager(); + } + + @AfterAll + public static void shutdown() { + if (cluster != null) { + cluster.shutdown(); + } + } + + @Test + public void testGetClusterTreeInformation() throws IOException { + SCMBlockLocationFailoverProxyProvider failoverProxyProvider = + new SCMBlockLocationFailoverProxyProvider(conf); + failoverProxyProvider.changeCurrentProxy(scm.getSCMNodeId()); + ScmBlockLocationProtocolClientSideTranslatorPB scmBlockLocationClient = + new ScmBlockLocationProtocolClientSideTranslatorPB( + failoverProxyProvider); + + InnerNode expectedInnerNode = scm.getClusterMap().getClusterTree(); + InnerNode actualInnerNode = scmBlockLocationClient.getClusterTree(); + Assertions.assertEquals(expectedInnerNode, actualInnerNode); + } +} diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestSecureOzoneCluster.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestSecureOzoneCluster.java index 717a2fafa19..ec46e4c0862 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestSecureOzoneCluster.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestSecureOzoneCluster.java @@ -50,10 +50,12 @@ import org.apache.hadoop.hdds.scm.ScmConfigKeys; import org.apache.hadoop.hdds.scm.ScmInfo; import org.apache.hadoop.hdds.scm.HddsTestUtils; +import org.apache.hadoop.hdds.scm.client.ScmTopologyClient; import org.apache.hadoop.hdds.scm.ha.HASecurityUtils; import org.apache.hadoop.hdds.scm.ha.SCMHANodeDetails; import org.apache.hadoop.hdds.scm.ha.SCMHAUtils; import org.apache.hadoop.hdds.scm.ha.SCMRatisServerImpl; +import org.apache.hadoop.hdds.scm.protocol.ScmBlockLocationProtocol; import org.apache.hadoop.hdds.scm.protocol.StorageContainerLocationProtocol; import org.apache.hadoop.hdds.scm.server.SCMHTTPServerConfig; import org.apache.hadoop.hdds.scm.server.SCMStorageConfig; @@ -87,6 +89,7 @@ import org.apache.hadoop.ozone.om.OMConfigKeys; import org.apache.hadoop.ozone.om.OMStorage; import org.apache.hadoop.ozone.om.OzoneManager; +import org.apache.hadoop.ozone.om.ScmBlockLocationTestingClient; import org.apache.hadoop.ozone.om.exceptions.OMException; import org.apache.hadoop.ozone.om.helpers.OmVolumeArgs; import org.apache.hadoop.ozone.om.helpers.S3SecretValue; @@ -208,6 +211,7 @@ final class TestSecureOzoneCluster { private File testUserKeytab; private String testUserPrincipal; private StorageContainerManager scm; + private ScmBlockLocationProtocol scmBlockClient; private OzoneManager om; private HddsProtos.OzoneManagerDetailsProto omInfo; private String host; @@ -264,6 +268,7 @@ public void init() { clusterId = UUID.randomUUID().toString(); scmId = UUID.randomUUID().toString(); omId = UUID.randomUUID().toString(); + scmBlockClient = new ScmBlockLocationTestingClient(null, null, 0); startMiniKdc(); setSecureConfig(); @@ -609,6 +614,7 @@ public void testAccessControlExceptionOnClient() throws Exception { setupOm(conf); om.setCertClient(new CertificateClientTestImpl(conf)); + om.setScmTopologyClient(new ScmTopologyClient(scmBlockClient)); om.start(); } catch (Exception ex) { // Expects timeout failure from scmClient in om but om user login via @@ -680,6 +686,7 @@ public void testDelegationTokenRenewal() throws Exception { setupOm(conf); OzoneManager.setTestSecureOmFlag(true); om.setCertClient(new CertificateClientTestImpl(conf)); + om.setScmTopologyClient(new ScmTopologyClient(scmBlockClient)); om.start(); UserGroupInformation ugi = UserGroupInformation.getCurrentUser(); @@ -768,6 +775,7 @@ public void testGetSetRevokeS3Secret() throws Exception { setupOm(conf); // Start OM om.setCertClient(new CertificateClientTestImpl(conf)); + om.setScmTopologyClient(new ScmTopologyClient(scmBlockClient)); om.start(); UserGroupInformation ugi = UserGroupInformation.getCurrentUser(); String username = ugi.getUserName(); @@ -1011,6 +1019,7 @@ public void testCertificateRotation() throws Exception { // create Ozone Manager instance, it will start the monitor task conf.set(OZONE_SCM_CLIENT_ADDRESS_KEY, "localhost"); om = OzoneManager.createOm(conf); + om.setScmTopologyClient(new ScmTopologyClient(scmBlockClient)); om.setCertClient(client); // check after renew, client will have the new cert ID @@ -1176,6 +1185,7 @@ public void testCertificateRotationUnRecoverableFailure() throws Exception { // create Ozone Manager instance, it will start the monitor task conf.set(OZONE_SCM_CLIENT_ADDRESS_KEY, "localhost"); om = OzoneManager.createOm(conf); + om.setScmTopologyClient(new ScmTopologyClient(scmBlockClient)); om.setCertClient(mockClient); // check error message during renew @@ -1214,6 +1224,7 @@ public void testDelegationTokenRenewCrossCertificateRenew() throws Exception { String omCertId1 = omCert.getSerialNumber().toString(); // Start OM om.setCertClient(certClient); + om.setScmTopologyClient(new ScmTopologyClient(scmBlockClient)); om.start(); GenericTestUtils.waitFor(() -> om.isLeaderReady(), 100, 10000); diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmContainerLocationCache.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmContainerLocationCache.java index 62931e03b63..6011a8ecd3d 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmContainerLocationCache.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmContainerLocationCache.java @@ -50,6 +50,9 @@ import org.apache.hadoop.hdds.scm.container.common.helpers.AllocatedBlock; import org.apache.hadoop.hdds.scm.container.common.helpers.ContainerWithPipeline; import org.apache.hadoop.hdds.scm.container.common.helpers.ExcludeList; +import org.apache.hadoop.hdds.scm.net.InnerNode; +import org.apache.hadoop.hdds.scm.net.InnerNodeImpl; +import org.apache.hadoop.hdds.scm.net.NetConstants; import org.apache.hadoop.hdds.scm.pipeline.Pipeline; import org.apache.hadoop.hdds.scm.pipeline.PipelineID; import org.apache.hadoop.hdds.scm.protocol.ScmBlockLocationProtocol; @@ -165,6 +168,9 @@ public static void setUp() throws Exception { mockScmBlockLocationProtocol = mock(ScmBlockLocationProtocol.class); mockScmContainerClient = Mockito.mock(StorageContainerLocationProtocol.class); + InnerNode.Factory factory = InnerNodeImpl.FACTORY; + when(mockScmBlockLocationProtocol.getClusterTree()).thenReturn( + factory.newInnerNode("", "", null, NetConstants.ROOT_LEVEL, 1)); OmTestManagers omTestManagers = new OmTestManagers(conf, mockScmBlockLocationProtocol, mockScmContainerClient); diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerListVolumesSecure.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerListVolumesSecure.java index 977bb0d4e9b..c9acac20c14 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerListVolumesSecure.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerListVolumesSecure.java @@ -46,6 +46,7 @@ import org.apache.commons.lang3.RandomStringUtils; import org.apache.hadoop.hdds.conf.OzoneConfiguration; +import org.apache.hadoop.hdds.scm.client.ScmTopologyClient; import org.apache.hadoop.hdds.security.x509.certificate.client.CertificateClientTestImpl; import org.apache.hadoop.minikdc.MiniKdc; import org.apache.hadoop.ozone.OzoneAcl; @@ -197,6 +198,8 @@ private void setupEnvironment(boolean aclEnabled, OzoneManager.setTestSecureOmFlag(true); om = OzoneManager.createOm(conf); + om.setScmTopologyClient(new ScmTopologyClient( + new ScmBlockLocationTestingClient(null, null, 0))); om.setCertClient(new CertificateClientTestImpl(conf)); om.start(); 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 7e4cfa00867..0b2b8e7593d 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 @@ -48,6 +48,7 @@ import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.hdds.protocol.DatanodeDetails; import org.apache.hadoop.hdds.scm.container.common.helpers.ContainerWithPipeline; +import org.apache.hadoop.hdds.scm.net.Node; import org.apache.hadoop.hdds.scm.pipeline.Pipeline; import org.apache.hadoop.hdds.scm.storage.BlockLocationInfo; import org.apache.hadoop.hdds.utils.BackgroundService; @@ -1836,8 +1837,7 @@ void sortDatanodes(String clientMachine, OmKeyInfo... keyInfos) { LOG.warn("No datanodes in pipeline {}", pipeline.getId()); continue; } - sortedNodes = sortDatanodes(clientMachine, nodes, keyInfo, - uuidList); + sortedNodes = sortDatanodes(clientMachine, nodes); if (sortedNodes != null) { sortedPipelines.put(uuidSet, sortedNodes); } @@ -1851,24 +1851,30 @@ void sortDatanodes(String clientMachine, OmKeyInfo... keyInfos) { } } - private List sortDatanodes(String clientMachine, - List nodes, OmKeyInfo keyInfo, List nodeList) { - List sortedNodes = null; - try { - sortedNodes = scmClient.getBlockClient() - .sortDatanodes(nodeList, clientMachine); - if (LOG.isDebugEnabled()) { - LOG.debug("Sorted datanodes {} for client {}, result: {}", nodes, - clientMachine, sortedNodes); + public List sortDatanodes(String clientMachine, + List nodes) { + DatanodeDetails client = null; + List possibleClients = + getClientNodesByAddress(clientMachine, nodes); + if (possibleClients.size() > 0) { + client = possibleClients.get(0); + } + List sortedNodeList = ozoneManager.getClusterMap() + .sortByDistanceCost(client, nodes, nodes.size()); + List ret = new ArrayList<>(); + sortedNodeList.stream().forEach(node -> ret.add((DatanodeDetails) node)); + return ret; + } + + private List getClientNodesByAddress(String clientMachine, + List nodes) { + List matchingNodes = new ArrayList<>(); + for (DatanodeDetails node : nodes) { + if (node.getIpAddress().equals(clientMachine)) { + matchingNodes.add(node); } - } catch (IOException e) { - LOG.warn("Unable to sort datanodes based on distance to client, " - + " volume={}, bucket={}, key={}, client={}, datanodes={}, " - + " exception={}", - keyInfo.getVolumeName(), keyInfo.getBucketName(), - keyInfo.getKeyName(), clientMachine, nodeList, e.getMessage()); } - return sortedNodes; + return matchingNodes; } private static List toNodeUuid(Collection nodes) { diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java index 053afbed3c5..52ca2e8ded5 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java @@ -81,7 +81,11 @@ import org.apache.hadoop.hdds.scm.ScmConfigKeys; import org.apache.hadoop.hdds.scm.ScmInfo; import org.apache.hadoop.hdds.scm.client.HddsClientUtils; +import org.apache.hadoop.hdds.scm.client.ScmTopologyClient; import org.apache.hadoop.hdds.scm.ha.SCMHAUtils; +import org.apache.hadoop.hdds.scm.net.InnerNode; +import org.apache.hadoop.hdds.scm.net.NetworkTopology; +import org.apache.hadoop.hdds.scm.net.NetworkTopologyImpl; import org.apache.hadoop.hdds.server.OzoneAdmins; import org.apache.hadoop.hdds.utils.db.Table; import org.apache.hadoop.hdds.utils.db.Table.KeyValue; @@ -355,6 +359,7 @@ public final class OzoneManager extends ServiceRuntimeInfoImpl private OzoneBlockTokenSecretManager blockTokenMgr; private CertificateClient certClient; private SecretKeySignerClient secretKeyClient; + private ScmTopologyClient scmTopologyClient; private final Text omRpcAddressTxt; private OzoneConfiguration configuration; private RPC.Server omRpcServer; @@ -389,6 +394,7 @@ public final class OzoneManager extends ServiceRuntimeInfoImpl private final ScmBlockLocationProtocol scmBlockClient; private final StorageContainerLocationProtocol scmContainerClient; private ObjectName omInfoBeanName; + private NetworkTopology clusterMap; private Timer metricsTimer; private ScheduleOMMetricsWriteTask scheduleOMMetricsWriteTask; private static final ObjectWriter WRITER = @@ -605,6 +611,7 @@ private OzoneManager(OzoneConfiguration conf, StartupOption startupOption) scmContainerClient = getScmContainerClient(configuration); // verifies that the SCM info in the OM Version file is correct. scmBlockClient = getScmBlockClient(configuration); + scmTopologyClient = new ScmTopologyClient(scmBlockClient); this.scmClient = new ScmClient(scmBlockClient, scmContainerClient, configuration); this.ozoneLockProvider = new OzoneLockProvider(getKeyPathLockEnabled(), @@ -1096,6 +1103,11 @@ public UUID refetchSecretKey() { return secretKeyClient.getCurrentSecretKey().getId(); } + public InnerNode refetchClusterTree() { + scmTopologyClient.refetchClusterTree(); + return scmTopologyClient.getClusterTree(); + } + @VisibleForTesting public void startSecretManager() { try { @@ -1139,6 +1151,20 @@ public void setCertClient(CertificateClient newClient) throws IOException { serviceInfo = new ServiceInfoProvider(secConfig, this, certClient); } + /** + * For testing purpose only. This allows setting up ScmBlockLocationClient + * without having to fully setup a working cluster. + */ + @VisibleForTesting + public void setScmTopologyClient( + ScmTopologyClient scmTopologyClient) { + this.scmTopologyClient = scmTopologyClient; + } + + public NetworkTopology getClusterMap() { + return clusterMap; + } + /** * For testing purpose only. This allows testing token in integration test * without fully setting up a working secure cluster. @@ -1677,6 +1703,18 @@ public void start() throws IOException { keyManager.start(configuration); + try { + scmTopologyClient.start(configuration); + } catch (IOException ex) { + LOG.error("Unable to initialize network topology schema file. ", ex); + throw new UncheckedIOException(ex); + } + + clusterMap = new NetworkTopologyImpl(configuration.get( + ScmConfigKeys.OZONE_SCM_NETWORK_TOPOLOGY_SCHEMA_FILE, + ScmConfigKeys.OZONE_SCM_NETWORK_TOPOLOGY_SCHEMA_FILE_DEFAULT), + scmTopologyClient.getClusterTree()); + try { httpServer = new OzoneManagerHttpServer(configuration, this); httpServer.start(); @@ -2231,6 +2269,11 @@ public boolean stop() { } keyManager.stop(); stopSecretManager(); + + if (scmTopologyClient != null) { + scmTopologyClient.stop(); + } + if (httpServer != null) { httpServer.stop(); } diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/OmTestManagers.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/OmTestManagers.java index 17f575f43b5..c01cb3fe6af 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/OmTestManagers.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/OmTestManagers.java @@ -21,6 +21,7 @@ import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.hdds.scm.HddsWhiteboxTestUtils; import org.apache.hadoop.hdds.scm.ScmConfigKeys; +import org.apache.hadoop.hdds.scm.client.ScmTopologyClient; import org.apache.hadoop.hdds.scm.protocol.ScmBlockLocationProtocol; import org.apache.hadoop.hdds.scm.protocol.StorageContainerLocationProtocol; import org.apache.hadoop.metrics2.lib.DefaultMetricsSystem; @@ -102,12 +103,16 @@ public OmTestManagers(OzoneConfiguration conf, keyManager = (KeyManagerImpl) HddsWhiteboxTestUtils .getInternalState(om, "keyManager"); ScmClient scmClient = new ScmClient(scmBlockClient, containerClient, conf); + ScmTopologyClient scmTopologyClient = + new ScmTopologyClient(scmBlockClient); HddsWhiteboxTestUtils.setInternalState(om, "scmClient", scmClient); HddsWhiteboxTestUtils.setInternalState(keyManager, "scmClient", scmClient); HddsWhiteboxTestUtils.setInternalState(keyManager, "secretManager", Mockito.mock(OzoneBlockTokenSecretManager.class)); + HddsWhiteboxTestUtils.setInternalState(om, + "scmTopologyClient", scmTopologyClient); om.start(); writeClient = OzoneClientFactory.getRpcClient(conf) diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/ScmBlockLocationTestingClient.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/ScmBlockLocationTestingClient.java index 8847a2d51e3..2f9e1323f71 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/ScmBlockLocationTestingClient.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/ScmBlockLocationTestingClient.java @@ -31,6 +31,9 @@ import org.apache.hadoop.hdds.scm.container.common.helpers.AllocatedBlock; import org.apache.hadoop.hdds.scm.container.common.helpers.DeleteBlockResult; import org.apache.hadoop.hdds.scm.container.common.helpers.ExcludeList; +import org.apache.hadoop.hdds.scm.net.InnerNode; +import org.apache.hadoop.hdds.scm.net.InnerNodeImpl; +import org.apache.hadoop.hdds.scm.net.NetConstants; import org.apache.hadoop.hdds.scm.pipeline.Pipeline; import org.apache.hadoop.hdds.scm.pipeline.PipelineID; import org.apache.hadoop.hdds.scm.protocol.ScmBlockLocationProtocol; @@ -200,6 +203,14 @@ public List sortDatanodes(List nodes, return null; } + @Override + public InnerNode getClusterTree() { + InnerNode.Factory factory = InnerNodeImpl.FACTORY; + InnerNode clusterTree = factory.newInnerNode("", "", null, + NetConstants.ROOT_LEVEL, 1); + return clusterTree; + } + /** * Return the number of blocks puesdo deleted by this testing client. */ diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerUnit.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerUnit.java index 795f933b971..6b31c03b106 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerUnit.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerUnit.java @@ -44,6 +44,9 @@ import org.apache.hadoop.hdds.protocol.proto.HddsProtos.ReplicationFactor; import org.apache.hadoop.hdds.scm.container.common.helpers.ContainerWithPipeline; import org.apache.hadoop.hdds.scm.container.ContainerInfo; +import org.apache.hadoop.hdds.scm.net.InnerNode; +import org.apache.hadoop.hdds.scm.net.InnerNodeImpl; +import org.apache.hadoop.hdds.scm.net.NetConstants; import org.apache.hadoop.hdds.scm.pipeline.MockPipeline; import org.apache.hadoop.hdds.scm.pipeline.Pipeline; import org.apache.hadoop.hdds.scm.pipeline.PipelineID; @@ -121,6 +124,9 @@ public void init() throws Exception { testDir.toString()); containerClient = Mockito.mock(StorageContainerLocationProtocol.class); blockClient = Mockito.mock(ScmBlockLocationProtocol.class); + InnerNode.Factory factory = InnerNodeImpl.FACTORY; + when(blockClient.getClusterTree()).thenReturn( + factory.newInnerNode("", "", null, NetConstants.ROOT_LEVEL, 1)); OmTestManagers omTestManagers = new OmTestManagers(configuration, blockClient, containerClient); From 1d006b2c4d952fa029384760a48cbf374cb78edb Mon Sep 17 00:00:00 2001 From: tanvipenumudy Date: Mon, 18 Dec 2023 13:02:08 +0530 Subject: [PATCH 02/23] Fix findbugs --- .../main/java/org/apache/hadoop/hdds/scm/net/InnerNodeImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/InnerNodeImpl.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/InnerNodeImpl.java index df5a92c4f7d..399f8cfec99 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/InnerNodeImpl.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/InnerNodeImpl.java @@ -463,7 +463,7 @@ public ScmBlockLocationProtocolProtos.NodeType toProtobuf(int clientVersion) { NodeImpl.toProtobuf(getNetworkName(), getNetworkLocation(), getCost())); - if (childrenMap != null || !childrenMap.isEmpty()) { + if (childrenMap != null && !childrenMap.isEmpty()) { for (Map.Entry entry : childrenMap.entrySet()) { if (entry.getValue() != null) { ScmBlockLocationProtocolProtos.ChildrenMap childrenMapProto = From 1780d455377fb802522dfe6c81721d30fe0689e2 Mon Sep 17 00:00:00 2001 From: tanvipenumudy Date: Mon, 18 Dec 2023 23:55:25 +0530 Subject: [PATCH 03/23] Fix LOG.info typos --- .../org/apache/hadoop/hdds/scm/client/ScmTopologyClient.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/client/ScmTopologyClient.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/client/ScmTopologyClient.java index c0f94447481..946f0c6858a 100644 --- a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/client/ScmTopologyClient.java +++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/client/ScmTopologyClient.java @@ -120,12 +120,12 @@ private synchronized void checkAndRefresh() { InnerNode newTopology = scmBlockLocationProtocol.getClusterTree(); if (!newTopology.equals(current)) { cache.set(newTopology); - LOG.info("Updated network topology schema file fetched from " + + LOG.info("Updated network topology cluster tree fetched from " + "SCM: {}.", newTopology); } } catch (IOException e) { throw new UncheckedIOException( - "Error fetching updated network topology schema file from SCM", e); + "Error fetching updated network topology cluster tree from SCM", e); } } } From 36127da5300e48a519c77f799cfe59dc135db69e Mon Sep 17 00:00:00 2001 From: tanvipenumudy Date: Thu, 21 Dec 2023 12:10:02 +0530 Subject: [PATCH 04/23] Modify clusterMap object based on the refetched clusterTree from SCM --- .../org/apache/hadoop/ozone/om/OzoneManager.java | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java index 52ca2e8ded5..4854fc2ee88 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java @@ -1162,7 +1162,18 @@ public void setScmTopologyClient( } public NetworkTopology getClusterMap() { - return clusterMap; + + InnerNode currentTree = scmTopologyClient.getClusterTree(); + InnerNode refetchedTree = refetchClusterTree(); + + if (currentTree != refetchedTree) { + return new NetworkTopologyImpl(configuration.get( + ScmConfigKeys.OZONE_SCM_NETWORK_TOPOLOGY_SCHEMA_FILE, + ScmConfigKeys.OZONE_SCM_NETWORK_TOPOLOGY_SCHEMA_FILE_DEFAULT), + refetchedTree); + } else { + return clusterMap; + } } /** From c213528480f5006bbff88b4f8e9fa476733bb63d Mon Sep 17 00:00:00 2001 From: tanvipenumudy Date: Thu, 21 Dec 2023 12:54:10 +0530 Subject: [PATCH 05/23] Incorporate dfs.datanode.use.datanode.hostname while filtering matching nodes --- .../java/org/apache/hadoop/ozone/om/KeyManagerImpl.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) 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 0b2b8e7593d..1986b819e1e 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 @@ -44,6 +44,7 @@ import org.apache.hadoop.crypto.key.KeyProviderCryptoExtension; import org.apache.hadoop.crypto.key.KeyProviderCryptoExtension.EncryptedKeyVersion; import org.apache.hadoop.fs.FileEncryptionInfo; +import org.apache.hadoop.hdds.DFSConfigKeysLegacy; import org.apache.hadoop.hdds.client.ReplicationConfig; import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.hdds.protocol.DatanodeDetails; @@ -1869,8 +1870,12 @@ public List sortDatanodes(String clientMachine, private List getClientNodesByAddress(String clientMachine, List nodes) { List matchingNodes = new ArrayList<>(); + boolean useHostname = ozoneManager.getConfiguration().getBoolean( + DFSConfigKeysLegacy.DFS_DATANODE_USE_DN_HOSTNAME, + DFSConfigKeysLegacy.DFS_DATANODE_USE_DN_HOSTNAME_DEFAULT); for (DatanodeDetails node : nodes) { - if (node.getIpAddress().equals(clientMachine)) { + if ((useHostname ? node.getHostName() : node.getIpAddress()).equals( + clientMachine)) { matchingNodes.add(node); } } From b9741bcabb8ccaeb29fce4cbb2766122d37bc1c7 Mon Sep 17 00:00:00 2001 From: tanvipenumudy Date: Mon, 29 Jan 2024 23:54:31 +0530 Subject: [PATCH 06/23] import assertEquals --- .../apache/hadoop/ozone/TestGetClusterTreeInformation.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestGetClusterTreeInformation.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestGetClusterTreeInformation.java index 112965830a1..e15796289c0 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestGetClusterTreeInformation.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestGetClusterTreeInformation.java @@ -24,7 +24,7 @@ import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.Assertions; + import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.apache.hadoop.hdds.scm.server.StorageContainerManager; @@ -32,6 +32,7 @@ import java.io.IOException; import java.util.concurrent.TimeoutException; +import static org.junit.jupiter.api.Assertions.assertEquals; /** * @@ -81,6 +82,6 @@ public void testGetClusterTreeInformation() throws IOException { InnerNode expectedInnerNode = scm.getClusterMap().getClusterTree(); InnerNode actualInnerNode = scmBlockLocationClient.getClusterTree(); - Assertions.assertEquals(expectedInnerNode, actualInnerNode); + assertEquals(expectedInnerNode, actualInnerNode); } } From 9dd266a809671a2aa15ec64934ceb2bba54dd927 Mon Sep 17 00:00:00 2001 From: tanvipenumudy Date: Tue, 30 Jan 2024 02:02:44 +0530 Subject: [PATCH 07/23] Fix TestOmContainerLocationCache --- .../apache/hadoop/ozone/om/TestOmContainerLocationCache.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmContainerLocationCache.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmContainerLocationCache.java index 29897f37c5e..1970f1dc4a7 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmContainerLocationCache.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmContainerLocationCache.java @@ -253,10 +253,13 @@ private static void createVolume(String volumeName) throws IOException { } @BeforeEach - public void beforeEach() { + public void beforeEach() throws IOException { CONTAINER_ID.getAndIncrement(); reset(mockScmBlockLocationProtocol, mockScmContainerClient, mockDn1Protocol, mockDn2Protocol); + InnerNode.Factory factory = InnerNodeImpl.FACTORY; + when(mockScmBlockLocationProtocol.getClusterTree()).thenReturn( + factory.newInnerNode("", "", null, NetConstants.ROOT_LEVEL, 1)); when(mockDn1Protocol.getPipeline()).thenReturn(createPipeline(DN1)); when(mockDn2Protocol.getPipeline()).thenReturn(createPipeline(DN2)); } From f623f9da7be08981b722c6b64353486fbbde1687 Mon Sep 17 00:00:00 2001 From: tanvipenumudy Date: Tue, 30 Jan 2024 02:32:40 +0530 Subject: [PATCH 08/23] Fix TestBlockOutputStreamWithFailures --- .../main/java/org/apache/hadoop/hdds/scm/node/DatanodeInfo.java | 1 + 1 file changed, 1 insertion(+) diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/DatanodeInfo.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/DatanodeInfo.java index 152b92eaaba..22bf255ff5c 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/DatanodeInfo.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/DatanodeInfo.java @@ -382,6 +382,7 @@ public boolean equals(Object obj) { public ScmBlockLocationProtocolProtos.NodeType toProtobuf(int clientVersion) { ScmBlockLocationProtocolProtos.DatanodeInfo.Builder datanodeInfo = ScmBlockLocationProtocolProtos.DatanodeInfo.newBuilder() + .setDatanodeDetails(toProtoBuilder(clientVersion).build()) .setLastHeartbeatTime(lastHeartbeatTime) .setLastStatsUpdatedTime(lastStatsUpdatedTime) .setFailedVolumeCount(failedVolumeCount) From 3d9c241ff14ddbdd98f8df9ba09798ab1dcccbd7 Mon Sep 17 00:00:00 2001 From: tanvipenumudy Date: Mon, 5 Feb 2024 11:22:55 +0530 Subject: [PATCH 09/23] Address review comments --- .../hadoop/hdds/scm/net/NetworkTopology.java | 2 -- .../hdds/scm/net/NetworkTopologyImpl.java | 5 ----- .../hadoop/hdds/scm/net/NodeSchemaManager.java | 17 +++++------------ .../apache/hadoop/ozone/OzoneConfigKeys.java | 6 +++--- .../common/src/main/resources/ozone-default.xml | 2 +- .../hdds/scm/client/ScmTopologyClient.java | 16 ++++++++-------- .../scm/protocol/ScmBlockLocationProtocol.java | 7 ++++++- ...kLocationProtocolClientSideTranslatorPB.java | 2 +- ...kLocationProtocolServerSideTranslatorPB.java | 2 +- .../hdds/scm/server/SCMBlockProtocolServer.java | 5 +++-- .../ozone/TestGetClusterTreeInformation.java | 6 ++++-- .../ozone/om/TestOmContainerLocationCache.java | 4 ++-- .../ozone/om/ScmBlockLocationTestingClient.java | 2 +- .../hadoop/ozone/om/TestKeyManagerUnit.java | 2 +- 14 files changed, 36 insertions(+), 42 deletions(-) diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/NetworkTopology.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/NetworkTopology.java index 218ac14e28e..5aa9e17825c 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/NetworkTopology.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/NetworkTopology.java @@ -96,8 +96,6 @@ public InvalidTopologyException(String msg) { */ Node getNode(String loc); - InnerNode getClusterTree(); - /** * Given a string representation of a InnerNode, return its leaf nodes count. * @param loc a path-like string representation of a InnerNode diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/NetworkTopologyImpl.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/NetworkTopologyImpl.java index ae787f16336..5600c82b02b 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/NetworkTopologyImpl.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/NetworkTopologyImpl.java @@ -315,11 +315,6 @@ public Node getNode(String loc) { } } - @Override - public InnerNode getClusterTree() { - return clusterTree; - } - /** * Given a string representation of Node, return its leaf nodes count. * @param loc a path-like string representation of Node diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/NodeSchemaManager.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/NodeSchemaManager.java index 09443bbe46a..fb37b214cad 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/NodeSchemaManager.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/NodeSchemaManager.java @@ -62,21 +62,14 @@ public void init(ConfigurationSource conf) { String schemaFile = conf.get( ScmConfigKeys.OZONE_SCM_NETWORK_TOPOLOGY_SCHEMA_FILE, ScmConfigKeys.OZONE_SCM_NETWORK_TOPOLOGY_SCHEMA_FILE_DEFAULT); - NodeSchemaLoadResult result; - try { - result = NodeSchemaLoader.getInstance().loadSchemaFromFile(schemaFile); - allSchema = result.getSchemaList(); - enforcePrefix = result.isEnforePrefix(); - maxLevel = allSchema.size(); - } catch (Throwable e) { - String msg = "Failed to load schema file:" + schemaFile - + ", error: " + e.getMessage(); - LOG.error(msg, e); - throw new RuntimeException(msg, e); - } + loadSchemaFile(schemaFile); } public void init(String schemaFile) { + loadSchemaFile(schemaFile); + } + + private void loadSchemaFile(String schemaFile) { NodeSchemaLoadResult result; try { result = NodeSchemaLoader.getInstance().loadSchemaFromFile(schemaFile); diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConfigKeys.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConfigKeys.java index 95971f4e678..680301c7c6b 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConfigKeys.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConfigKeys.java @@ -676,10 +676,10 @@ public final class OzoneConfigKeys { "hdds.scmclient.failover.max.retry"; public static final String - OZONE_SCM_NETWORK_TOPOLOGY_CLUSTER_TREE_REFRESH_DURATION = - "ozone.scm.network.topology.cluster.tree.refresh.duration"; + OZONE_OM_NETWORK_TOPOLOGY_REFRESH_DURATION = + "ozone.om.network.topology.refresh.duration"; public static final String - OZONE_SCM_NETWORK_TOPOLOGY_CLUSTER_TREE_REFRESH_DURATION_DEFAULT = "1h"; + OZONE_OM_NETWORK_TOPOLOGY_REFRESH_DURATION_DEFAULT = "1h"; /** * There is no need to instantiate this class. diff --git a/hadoop-hdds/common/src/main/resources/ozone-default.xml b/hadoop-hdds/common/src/main/resources/ozone-default.xml index c09ad988993..f0feb55873d 100644 --- a/hadoop-hdds/common/src/main/resources/ozone-default.xml +++ b/hadoop-hdds/common/src/main/resources/ozone-default.xml @@ -3753,7 +3753,7 @@ is send to DN. - ozone.scm.network.topology.cluster.tree.refresh.duration + ozone.om.network.topology.refresh.duration 1h SCM, OZONE, OM The duration at which we periodically fetch the updated network diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/client/ScmTopologyClient.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/client/ScmTopologyClient.java index 946f0c6858a..de33bef14ee 100644 --- a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/client/ScmTopologyClient.java +++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/client/ScmTopologyClient.java @@ -36,8 +36,8 @@ import java.util.concurrent.atomic.AtomicReference; import static java.util.Objects.requireNonNull; -import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_SCM_NETWORK_TOPOLOGY_CLUSTER_TREE_REFRESH_DURATION; -import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_SCM_NETWORK_TOPOLOGY_CLUSTER_TREE_REFRESH_DURATION_DEFAULT; +import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_OM_NETWORK_TOPOLOGY_REFRESH_DURATION; +import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_OM_NETWORK_TOPOLOGY_REFRESH_DURATION_DEFAULT; /** * This client implements a background thread which periodically checks and @@ -67,7 +67,7 @@ public void refetchClusterTree() { public void start(ConfigurationSource conf) throws IOException { final InnerNode initialTopology = - scmBlockLocationProtocol.getClusterTree(); + scmBlockLocationProtocol.getNetworkTopology(); LOG.info("Initial network topology fetched from SCM: {}.", initialTopology); cache.set(initialTopology); @@ -78,7 +78,7 @@ public void stop() { if (executorService != null) { executorService.shutdown(); try { - if (executorService.awaitTermination(1, TimeUnit.MINUTES)) { + if (executorService.awaitTermination(5, TimeUnit.SECONDS)) { executorService.shutdownNow(); } } catch (InterruptedException e) { @@ -99,7 +99,7 @@ private void scheduleNetworkTopologyPoller(ConfigurationSource conf, executorService = Executors.newScheduledThreadPool(1, threadFactory); Duration initialDelay = Duration.between(Instant.now(), nextRefresh); - LOG.info("Scheduling NetworkTopologyPoller with an initial delay of {}.", + LOG.debug("Scheduling NetworkTopologyPoller with an initial delay of {}.", initialDelay); executorService.scheduleAtFixedRate(() -> checkAndRefresh(), initialDelay.toMillis(), refreshDuration.toMillis(), @@ -108,8 +108,8 @@ private void scheduleNetworkTopologyPoller(ConfigurationSource conf, public static Duration parseRefreshDuration(ConfigurationSource conf) { long refreshDurationInMs = conf.getTimeDuration( - OZONE_SCM_NETWORK_TOPOLOGY_CLUSTER_TREE_REFRESH_DURATION, - OZONE_SCM_NETWORK_TOPOLOGY_CLUSTER_TREE_REFRESH_DURATION_DEFAULT, + OZONE_OM_NETWORK_TOPOLOGY_REFRESH_DURATION, + OZONE_OM_NETWORK_TOPOLOGY_REFRESH_DURATION_DEFAULT, TimeUnit.MILLISECONDS); return Duration.ofMillis(refreshDurationInMs); } @@ -117,7 +117,7 @@ public static Duration parseRefreshDuration(ConfigurationSource conf) { private synchronized void checkAndRefresh() { InnerNode current = cache.get(); try { - InnerNode newTopology = scmBlockLocationProtocol.getClusterTree(); + InnerNode newTopology = scmBlockLocationProtocol.getNetworkTopology(); if (!newTopology.equals(current)) { cache.set(newTopology); LOG.info("Updated network topology cluster tree fetched from " + diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/protocol/ScmBlockLocationProtocol.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/protocol/ScmBlockLocationProtocol.java index d9d1a65a527..8c84af859b4 100644 --- a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/protocol/ScmBlockLocationProtocol.java +++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/protocol/ScmBlockLocationProtocol.java @@ -140,5 +140,10 @@ List allocateBlock(long size, int numBlocks, List sortDatanodes(List nodes, String clientMachine) throws IOException; - InnerNode getClusterTree() throws IOException; + /** + * Retrieves the hierarchical cluster tree representing the network topology. + * @return the root node of the network topology cluster tree. + * @throws IOException + */ + InnerNode getNetworkTopology() throws IOException; } diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/protocolPB/ScmBlockLocationProtocolClientSideTranslatorPB.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/protocolPB/ScmBlockLocationProtocolClientSideTranslatorPB.java index 66be7f0e2c5..12ba88fd207 100644 --- a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/protocolPB/ScmBlockLocationProtocolClientSideTranslatorPB.java +++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/protocolPB/ScmBlockLocationProtocolClientSideTranslatorPB.java @@ -333,7 +333,7 @@ public List sortDatanodes(List nodes, } @Override - public InnerNode getClusterTree() throws IOException { + public InnerNode getNetworkTopology() throws IOException { GetClusterTreeRequestProto request = GetClusterTreeRequestProto.newBuilder().build(); SCMBlockLocationRequest wrapper = createSCMBlockRequest(Type.GetClusterTree) diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/protocol/ScmBlockLocationProtocolServerSideTranslatorPB.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/protocol/ScmBlockLocationProtocolServerSideTranslatorPB.java index a1b1c93e4b1..e77e2aebb31 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/protocol/ScmBlockLocationProtocolServerSideTranslatorPB.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/protocol/ScmBlockLocationProtocolServerSideTranslatorPB.java @@ -287,7 +287,7 @@ public GetClusterTreeResponseProto getClusterTree(int clientVersion) throws IOException { GetClusterTreeResponseProto.Builder resp = GetClusterTreeResponseProto.newBuilder(); - InnerNode clusterTree = impl.getClusterTree(); + InnerNode clusterTree = impl.getNetworkTopology(); resp.setClusterTree(clusterTree.toProtobuf(clientVersion).getInnerNode()); return resp.build(); } diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMBlockProtocolServer.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMBlockProtocolServer.java index 50ea46090bc..a0d0a85bbcb 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMBlockProtocolServer.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMBlockProtocolServer.java @@ -73,6 +73,7 @@ import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_BLOCK_HANDLER_COUNT_KEY; import static org.apache.hadoop.hdds.scm.exceptions.SCMException.ResultCodes.IO_EXCEPTION; import static org.apache.hadoop.hdds.scm.net.NetConstants.NODE_COST_DEFAULT; +import static org.apache.hadoop.hdds.scm.net.NetConstants.ROOT; import static org.apache.hadoop.hdds.scm.server.StorageContainerManager.startRpcServer; import static org.apache.hadoop.hdds.server.ServerUtils.getRemoteUserName; import static org.apache.hadoop.hdds.server.ServerUtils.updateRPCListenAddress; @@ -413,8 +414,8 @@ private Node getOtherNode(String clientMachine) { } @Override - public InnerNode getClusterTree() { - return scm.getClusterMap().getClusterTree(); + public InnerNode getNetworkTopology() { + return (InnerNode) scm.getClusterMap().getNode(ROOT); } @Override diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestGetClusterTreeInformation.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestGetClusterTreeInformation.java index e15796289c0..2f985ed9773 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestGetClusterTreeInformation.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestGetClusterTreeInformation.java @@ -32,6 +32,8 @@ import java.io.IOException; import java.util.concurrent.TimeoutException; + +import static org.apache.hadoop.hdds.scm.net.NetConstants.ROOT; import static org.junit.jupiter.api.Assertions.assertEquals; /** @@ -80,8 +82,8 @@ public void testGetClusterTreeInformation() throws IOException { new ScmBlockLocationProtocolClientSideTranslatorPB( failoverProxyProvider); - InnerNode expectedInnerNode = scm.getClusterMap().getClusterTree(); - InnerNode actualInnerNode = scmBlockLocationClient.getClusterTree(); + InnerNode expectedInnerNode = (InnerNode) scm.getClusterMap().getNode(ROOT); + InnerNode actualInnerNode = scmBlockLocationClient.getNetworkTopology(); assertEquals(expectedInnerNode, actualInnerNode); } } diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmContainerLocationCache.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmContainerLocationCache.java index 1970f1dc4a7..2ba58b4f078 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmContainerLocationCache.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmContainerLocationCache.java @@ -166,7 +166,7 @@ public static void setUp() throws Exception { mockScmContainerClient = mock(StorageContainerLocationProtocol.class); InnerNode.Factory factory = InnerNodeImpl.FACTORY; - when(mockScmBlockLocationProtocol.getClusterTree()).thenReturn( + when(mockScmBlockLocationProtocol.getNetworkTopology()).thenReturn( factory.newInnerNode("", "", null, NetConstants.ROOT_LEVEL, 1)); OmTestManagers omTestManagers = new OmTestManagers(conf, @@ -258,7 +258,7 @@ public void beforeEach() throws IOException { reset(mockScmBlockLocationProtocol, mockScmContainerClient, mockDn1Protocol, mockDn2Protocol); InnerNode.Factory factory = InnerNodeImpl.FACTORY; - when(mockScmBlockLocationProtocol.getClusterTree()).thenReturn( + when(mockScmBlockLocationProtocol.getNetworkTopology()).thenReturn( factory.newInnerNode("", "", null, NetConstants.ROOT_LEVEL, 1)); when(mockDn1Protocol.getPipeline()).thenReturn(createPipeline(DN1)); when(mockDn2Protocol.getPipeline()).thenReturn(createPipeline(DN2)); diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/ScmBlockLocationTestingClient.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/ScmBlockLocationTestingClient.java index 2f9e1323f71..8ba5ca779c1 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/ScmBlockLocationTestingClient.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/ScmBlockLocationTestingClient.java @@ -204,7 +204,7 @@ public List sortDatanodes(List nodes, } @Override - public InnerNode getClusterTree() { + public InnerNode getNetworkTopology() { InnerNode.Factory factory = InnerNodeImpl.FACTORY; InnerNode clusterTree = factory.newInnerNode("", "", null, NetConstants.ROOT_LEVEL, 1); diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerUnit.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerUnit.java index 947a3c4293a..6df9730e224 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerUnit.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerUnit.java @@ -128,7 +128,7 @@ public void init() throws Exception { containerClient = mock(StorageContainerLocationProtocol.class); blockClient = mock(ScmBlockLocationProtocol.class); InnerNode.Factory factory = InnerNodeImpl.FACTORY; - when(blockClient.getClusterTree()).thenReturn( + when(blockClient.getNetworkTopology()).thenReturn( factory.newInnerNode("", "", null, NetConstants.ROOT_LEVEL, 1)); OmTestManagers omTestManagers From a1c6ff87db899d2bf718fa89de9bdbc1c7f102ea Mon Sep 17 00:00:00 2001 From: tanvipenumudy Date: Mon, 12 Feb 2024 20:51:19 +0530 Subject: [PATCH 10/23] Include level, parent information, handle case of read from non-datanode host --- .../hadoop/hdds/protocol/DatanodeDetails.java | 16 +++++++ .../hadoop/hdds/scm/net/InnerNodeImpl.java | 47 ++++++++++++++----- .../hdds/scm/net/NetworkTopologyImpl.java | 6 +-- .../apache/hadoop/hdds/scm/net/NodeImpl.java | 8 ++-- ...ocationProtocolClientSideTranslatorPB.java | 25 +++++++++- .../src/main/proto/hdds.proto | 1 + .../src/main/proto/ScmServerProtocol.proto | 1 + .../ozone/TestGetClusterTreeInformation.java | 4 -- .../hadoop/ozone/om/KeyManagerImpl.java | 20 +++++++- .../apache/hadoop/ozone/om/OzoneManager.java | 2 +- 10 files changed, 105 insertions(+), 25 deletions(-) diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/protocol/DatanodeDetails.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/protocol/DatanodeDetails.java index 7e8b7bef9e6..c6753529f5a 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/protocol/DatanodeDetails.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/protocol/DatanodeDetails.java @@ -367,6 +367,9 @@ public static DatanodeDetails.Builder newBuilder( if (datanodeDetailsProto.hasNetworkLocation()) { builder.setNetworkLocation(datanodeDetailsProto.getNetworkLocation()); } + if (datanodeDetailsProto.hasLevel()) { + builder.setLevel(datanodeDetailsProto.getLevel()); + } if (datanodeDetailsProto.hasPersistedOpState()) { builder.setPersistedOpState(datanodeDetailsProto.getPersistedOpState()); } @@ -459,6 +462,9 @@ public HddsProtos.DatanodeDetailsProto.Builder toProtoBuilder( if (!Strings.isNullOrEmpty(getNetworkLocation())) { builder.setNetworkLocation(getNetworkLocation()); } + if (getLevel() > 0) { + builder.setLevel(getLevel()); + } if (persistedOpState != null) { builder.setPersistedOpState(persistedOpState); } @@ -596,6 +602,7 @@ public static final class Builder { private String hostName; private String networkName; private String networkLocation; + private int level; private List ports; private String certSerialId; private String version; @@ -627,6 +634,7 @@ public Builder setDatanodeDetails(DatanodeDetails details) { this.hostName = details.getHostName(); this.networkName = details.getNetworkName(); this.networkLocation = details.getNetworkLocation(); + this.level = details.getLevel(); this.ports = details.getPorts(); this.certSerialId = details.getCertSerialId(); this.version = details.getVersion(); @@ -694,6 +702,11 @@ public Builder setNetworkLocation(String loc) { return this; } + public Builder setLevel(int level) { + this.level = level; + return this; + } + /** * Adds a DataNode Port. * @@ -818,6 +831,9 @@ public DatanodeDetails build() { if (networkName != null) { dn.setNetworkName(networkName); } + if (level > 0) { + dn.setLevel(level); + } return dn; } } diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/InnerNodeImpl.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/InnerNodeImpl.java index 399f8cfec99..35adda83c7a 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/InnerNodeImpl.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/InnerNodeImpl.java @@ -67,9 +67,14 @@ protected InnerNodeImpl(String name, String location, InnerNode parent, super(name, location, parent, level, cost); } - protected InnerNodeImpl(String name, String location, int cost, + /** + * Construct an InnerNode from its name, network location, level, cost, + * childrenMap and number of leaves. This constructor is used as part of + * protobuf deserialization. + */ + protected InnerNodeImpl(String name, String location, int level, int cost, HashMap childrenMap, int numOfLeaves) { - super(name, location, cost); + super(name, location, null, level, cost); this.childrenMap = childrenMap; this.numOfLeaves = numOfLeaves; } @@ -82,6 +87,7 @@ public static class Builder { private String name; private String location; private int cost; + private int level; private HashMap childrenMap = new LinkedHashMap<>(); private int numOfLeaves; @@ -100,6 +106,11 @@ public Builder setCost(int cost) { return this; } + public Builder setLevel(int level) { + this.level = level; + return this; + } + public Builder setChildrenMap(HashMap childrenMap) { this.childrenMap = childrenMap; return this; @@ -107,7 +118,7 @@ public Builder setChildrenMap(HashMap childrenMap) { public Builder setChildrenMap( List childrenMapList) { - HashMap newChildrenMap = new HashMap<>(); + HashMap newChildrenMap = new LinkedHashMap<>(); for (ScmBlockLocationProtocolProtos.ChildrenMap childrenMapProto : childrenMapList) { String networkName = childrenMapProto.getNetworkName(); @@ -126,7 +137,8 @@ public Builder setNumOfLeaves(int numOfLeaves) { } public InnerNodeImpl build() { - return new InnerNodeImpl(name, location, cost, childrenMap, numOfLeaves); + return new InnerNodeImpl(name, location, level, cost, childrenMap, + numOfLeaves); } } @@ -141,6 +153,11 @@ public int getNumOfLeaves() { return numOfLeaves; } + /** @return a map of node's network name to Node. */ + public HashMap getChildrenMap() { + return childrenMap; + } + /** * @return number of its all nodes at level level. Here level is a * relative level. If level is 1, means node itself. If level is 2, means its @@ -461,7 +478,7 @@ public ScmBlockLocationProtocolProtos.NodeType toProtobuf(int clientVersion) { .setNumOfLeaves(numOfLeaves) .setNodeImpl( NodeImpl.toProtobuf(getNetworkName(), getNetworkLocation(), - getCost())); + getLevel(), getCost())); if (childrenMap != null && !childrenMap.isEmpty()) { for (Map.Entry entry : childrenMap.entrySet()) { @@ -499,6 +516,7 @@ public static InnerNode fromProtobuf( .setName(nodeImpl.getName()) .setLocation(nodeImpl.getLocation()) .setCost(nodeImpl.getCost()) + .setLevel(nodeImpl.getLevel()) .setChildrenMap(innerNode.getChildrenMapList()) .setNumOfLeaves(innerNode.getNumOfLeaves()); @@ -507,14 +525,21 @@ public static InnerNode fromProtobuf( } @Override - public boolean equals(Object to) { - if (to == null) { - return false; - } - if (this == to) { + public boolean equals(Object o) { + if (this == o) { return true; } - return this.toString().equals(to.toString()); + if (o == null || getClass() != o.getClass()) { + return false; + } + InnerNodeImpl innerNode = (InnerNodeImpl) o; + return this.getNetworkName().equals(innerNode.getNetworkName()) && + this.getNetworkLocation().equals(innerNode.getNetworkLocation()) && + this.getLevel() == innerNode.getLevel() && + this.getCost() == innerNode.getCost() && + this.numOfLeaves == innerNode.numOfLeaves && + this.childrenMap.size() == innerNode.childrenMap.size() && + this.childrenMap.equals(innerNode.childrenMap); } @Override diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/NetworkTopologyImpl.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/NetworkTopologyImpl.java index 5600c82b02b..75db2b7adab 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/NetworkTopologyImpl.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/NetworkTopologyImpl.java @@ -735,8 +735,8 @@ public int getDistanceCost(Node node1, Node node2) { int cost = 0; netlock.readLock().lock(); try { - if ((node1.getAncestor(level1 - 1) != clusterTree) || - (node2.getAncestor(level2 - 1) != clusterTree)) { + if (!(node1.getAncestor(level1 - 1).equals(clusterTree)) || + !(node2.getAncestor(level2 - 1).equals(clusterTree))) { LOG.debug("One of the nodes is outside of network topology"); return Integer.MAX_VALUE; } @@ -750,7 +750,7 @@ public int getDistanceCost(Node node1, Node node2) { level2--; cost += node2 == null ? 0 : node2.getCost(); } - while (node1 != null && node2 != null && node1 != node2) { + while (node1 != null && node2 != null && !node1.equals(node2)) { node1 = node1.getParent(); node2 = node2.getParent(); cost += node1 == null ? 0 : node1.getCost(); diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/NodeImpl.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/NodeImpl.java index 5b0f9dfb43e..f6aac09227f 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/NodeImpl.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/NodeImpl.java @@ -237,6 +237,7 @@ public ScmBlockLocationProtocolProtos.NodeType toProtobuf(int clientVersion) { .setName(name) .setLocation(location) .setCost(cost) + .setLevel(level) .build(); ScmBlockLocationProtocolProtos.NodeType nodeType = @@ -247,12 +248,13 @@ public ScmBlockLocationProtocolProtos.NodeType toProtobuf(int clientVersion) { } public static ScmBlockLocationProtocolProtos.NodeImpl toProtobuf(String name, - String location, int cost) { + String location, int level, int cost) { ScmBlockLocationProtocolProtos.NodeImpl.Builder nodeImplBuilder = ScmBlockLocationProtocolProtos.NodeImpl.newBuilder() .setName(name) .setLocation(location) + .setLevel(level) .setCost(cost); ScmBlockLocationProtocolProtos.NodeImpl nodeImpl = nodeImplBuilder.build(); @@ -268,8 +270,8 @@ public static Node fromProtobuf( public static NodeImpl fromProtobuf( ScmBlockLocationProtocolProtos.NodeImpl nodeImpl) { - return new NodeImpl(nodeImpl.getName(), nodeImpl.getLocation(), - nodeImpl.getCost()); + return new NodeImpl(nodeImpl.getName(), nodeImpl.getLocation(), null, + nodeImpl.getLevel(), nodeImpl.getCost()); } @Override diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/protocolPB/ScmBlockLocationProtocolClientSideTranslatorPB.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/protocolPB/ScmBlockLocationProtocolClientSideTranslatorPB.java index 12ba88fd207..1f114304cca 100644 --- a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/protocolPB/ScmBlockLocationProtocolClientSideTranslatorPB.java +++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/protocolPB/ScmBlockLocationProtocolClientSideTranslatorPB.java @@ -20,6 +20,7 @@ import java.io.IOException; import java.util.ArrayList; import java.util.List; +import java.util.Map; import java.util.stream.Collectors; import org.apache.commons.lang3.StringUtils; @@ -53,6 +54,7 @@ import org.apache.hadoop.hdds.scm.exceptions.SCMException; import org.apache.hadoop.hdds.scm.net.InnerNode; import org.apache.hadoop.hdds.scm.net.InnerNodeImpl; +import org.apache.hadoop.hdds.scm.net.Node; import org.apache.hadoop.hdds.scm.pipeline.Pipeline; import org.apache.hadoop.hdds.scm.protocol.ScmBlockLocationProtocol; import org.apache.hadoop.hdds.scm.proxy.SCMBlockLocationFailoverProxyProvider; @@ -345,7 +347,28 @@ public InnerNode getNetworkTopology() throws IOException { GetClusterTreeResponseProto resp = wrappedResponse.getGetClusterTreeResponse(); - return InnerNodeImpl.fromProtobuf(resp.getClusterTree()); + return (InnerNode) setParent( + InnerNodeImpl.fromProtobuf(resp.getClusterTree())); + } + + /** + * Sets the parent field for the clusterTree nodes recursively. + * + * @param node cluster tree without parents set. + * @return updated cluster tree with parents set. + */ + private Node setParent(Node node) { + if (node instanceof InnerNodeImpl) { + InnerNodeImpl innerNode = (InnerNodeImpl) node; + if (innerNode.getChildrenMap() != null) { + for (Map.Entry child : innerNode.getChildrenMap() + .entrySet()) { + child.getValue().setParent(innerNode); + setParent(child.getValue()); + } + } + } + return node; } @Override diff --git a/hadoop-hdds/interface-client/src/main/proto/hdds.proto b/hadoop-hdds/interface-client/src/main/proto/hdds.proto index 3f346300b3e..2699a8abdc0 100644 --- a/hadoop-hdds/interface-client/src/main/proto/hdds.proto +++ b/hadoop-hdds/interface-client/src/main/proto/hdds.proto @@ -47,6 +47,7 @@ message DatanodeDetailsProto { optional int64 persistedOpStateExpiry = 9; // The seconds after the epoch when the OpState should expire // TODO(runzhiwang): when uuid is gone, specify 1 as the index of uuid128 and mark as required optional UUID uuid128 = 100; // UUID with 128 bits assigned to the Datanode. + optional uint32 level = 101; } /** diff --git a/hadoop-hdds/interface-server/src/main/proto/ScmServerProtocol.proto b/hadoop-hdds/interface-server/src/main/proto/ScmServerProtocol.proto index 60cbfe8a1b5..07a3a29030f 100644 --- a/hadoop-hdds/interface-server/src/main/proto/ScmServerProtocol.proto +++ b/hadoop-hdds/interface-server/src/main/proto/ScmServerProtocol.proto @@ -237,6 +237,7 @@ message NodeImpl { required string name = 1; required string location = 2; required uint32 cost = 3; + required uint32 level = 4; } message CommandCount { diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestGetClusterTreeInformation.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestGetClusterTreeInformation.java index 2f985ed9773..3f6377e58f8 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestGetClusterTreeInformation.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestGetClusterTreeInformation.java @@ -18,7 +18,6 @@ package org.apache.hadoop.ozone; import org.apache.hadoop.hdds.scm.net.InnerNode; -import org.apache.hadoop.hdds.scm.node.NodeManager; import org.apache.hadoop.hdds.scm.protocolPB.ScmBlockLocationProtocolClientSideTranslatorPB; import org.apache.hadoop.hdds.scm.proxy.SCMBlockLocationFailoverProxyProvider; import org.junit.jupiter.api.BeforeAll; @@ -49,9 +48,6 @@ public class TestGetClusterTreeInformation { private static MiniOzoneCluster cluster; private static OzoneConfiguration conf; private static StorageContainerManager scm; - private static NodeManager nodeManager; - private static ClassLoader classLoader = - Thread.currentThread().getContextClassLoader(); @BeforeAll public static void init() throws IOException, TimeoutException, 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 cc3e2fd024c..a12072b67d8 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 @@ -1834,7 +1834,8 @@ void sortDatanodes(String clientMachine, OmKeyInfo... keyInfos) { LOG.warn("No datanodes in pipeline {}", pipeline.getId()); continue; } - sortedNodes = sortDatanodes(clientMachine, nodes); + sortedNodes = sortDatanodes(clientMachine, nodes, keyInfo, + uuidList); if (sortedNodes != null) { sortedPipelines.put(uuidSet, sortedNodes); } @@ -1849,12 +1850,27 @@ void sortDatanodes(String clientMachine, OmKeyInfo... keyInfos) { } public List sortDatanodes(String clientMachine, - List nodes) { + List nodes, OmKeyInfo keyInfo, List nodeList) { DatanodeDetails client = null; List possibleClients = getClientNodesByAddress(clientMachine, nodes); if (possibleClients.size() > 0) { client = possibleClients.get(0); + } else { + /** + * In case of read from a non-datanode host, return + * {@link ScmBlockLocationProtocol#sortDatanodes(List, String)}. + */ + try { + return scmClient.getBlockClient() + .sortDatanodes(nodeList, clientMachine); + } catch (IOException e) { + LOG.warn("Unable to sort datanodes based on distance to client, " + + " volume={}, bucket={}, key={}, client={}, datanodes={}, " + + " exception={}", + keyInfo.getVolumeName(), keyInfo.getBucketName(), + keyInfo.getKeyName(), clientMachine, nodeList, e.getMessage()); + } } List sortedNodeList = ozoneManager.getClusterMap() .sortByDistanceCost(client, nodes, nodes.size()); diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java index 2ab5f802eea..76f10f6a514 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java @@ -1163,7 +1163,7 @@ public NetworkTopology getClusterMap() { InnerNode currentTree = scmTopologyClient.getClusterTree(); InnerNode refetchedTree = refetchClusterTree(); - if (currentTree != refetchedTree) { + if (!currentTree.equals(refetchedTree)) { return new NetworkTopologyImpl(configuration.get( ScmConfigKeys.OZONE_SCM_NETWORK_TOPOLOGY_SCHEMA_FILE, ScmConfigKeys.OZONE_SCM_NETWORK_TOPOLOGY_SCHEMA_FILE_DEFAULT), From 91e12c5503c98204af46622480f1d592a934c319 Mon Sep 17 00:00:00 2001 From: tanvipenumudy Date: Mon, 12 Feb 2024 23:29:25 +0530 Subject: [PATCH 11/23] Refactor proto objects, address reviews --- hadoop-hdds/common/pom.xml | 4 - .../hadoop/hdds/protocol/DatanodeDetails.java | 11 ++- .../apache/hadoop/hdds/scm/net/InnerNode.java | 7 +- .../hadoop/hdds/scm/net/InnerNodeImpl.java | 30 +++--- .../org/apache/hadoop/hdds/scm/net/Node.java | 12 +-- .../apache/hadoop/hdds/scm/net/NodeImpl.java | 39 ++------ .../src/main/proto/hdds.proto | 23 +++++ .../src/main/proto/ScmServerProtocol.proto | 49 ---------- .../hadoop/hdds/scm/node/DatanodeInfo.java | 93 ------------------- .../hadoop/hdds/scm/node/NodeStatus.java | 17 ---- 10 files changed, 63 insertions(+), 222 deletions(-) diff --git a/hadoop-hdds/common/pom.xml b/hadoop-hdds/common/pom.xml index 3b3d6456efb..3d05f60e101 100644 --- a/hadoop-hdds/common/pom.xml +++ b/hadoop-hdds/common/pom.xml @@ -206,10 +206,6 @@ https://maven.apache.org/xsd/maven-4.0.0.xsd"> junit-platform-launcher test - - org.apache.ozone - hdds-interface-server - diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/protocol/DatanodeDetails.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/protocol/DatanodeDetails.java index c6753529f5a..7b7c632cf9d 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/protocol/DatanodeDetails.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/protocol/DatanodeDetails.java @@ -34,7 +34,6 @@ import org.apache.hadoop.hdds.protocol.DatanodeDetails.Port.Name; import org.apache.hadoop.hdds.protocol.proto.HddsProtos; import org.apache.hadoop.hdds.protocol.proto.HddsProtos.ExtendedDatanodeDetailsProto; -import org.apache.hadoop.hdds.protocol.proto.ScmBlockLocationProtocolProtos; import org.apache.hadoop.hdds.scm.net.NetConstants; import org.apache.hadoop.hdds.scm.net.Node; import org.apache.hadoop.hdds.scm.net.NodeImpl; @@ -1039,15 +1038,17 @@ public void setBuildDate(String date) { this.buildDate = date; } - public ScmBlockLocationProtocolProtos.NodeType toProtobuf(int clientVersion) { - ScmBlockLocationProtocolProtos.NodeType nodeType = - ScmBlockLocationProtocolProtos.NodeType.newBuilder() + @Override + public HddsProtos.NodeInterface toProtobuf( + int clientVersion) { + HddsProtos.NodeInterface nodeType = + HddsProtos.NodeInterface.newBuilder() .setDatanodeDetails(toProtoBuilder(clientVersion).build()).build(); return nodeType; } public static Node fromProtobuf( - ScmBlockLocationProtocolProtos.NodeType nodeType) { + HddsProtos.NodeInterface nodeType) { return nodeType.hasDatanodeDetails() ? DatanodeDetails.getFromProtoBuf(nodeType.getDatanodeDetails()) : null; diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/InnerNode.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/InnerNode.java index 0293ffc6e9e..c8256341db5 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/InnerNode.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/InnerNode.java @@ -20,7 +20,7 @@ import java.util.Collection; import java.util.List; -import org.apache.hadoop.hdds.protocol.proto.ScmBlockLocationProtocolProtos; +import org.apache.hadoop.hdds.protocol.proto.HddsProtos; /** * The interface defines an inner node in a network topology. @@ -92,14 +92,15 @@ N newInnerNode(String name, String location, InnerNode parent, int level, Node getLeaf(int leafIndex, List excludedScopes, Collection excludedNodes, int ancestorGen); - ScmBlockLocationProtocolProtos.NodeType toProtobuf(int clientVersion); + @Override + HddsProtos.NodeInterface toProtobuf(int clientVersion); boolean equals(Object o); int hashCode(); static InnerNode fromProtobuf( - ScmBlockLocationProtocolProtos.InnerNode innerNode) { + HddsProtos.InnerNode innerNode) { return InnerNodeImpl.fromProtobuf(innerNode); } } diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/InnerNodeImpl.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/InnerNodeImpl.java index 35adda83c7a..e5af3c78b6b 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/InnerNodeImpl.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/InnerNodeImpl.java @@ -27,7 +27,7 @@ import java.util.Map; import com.google.common.base.Preconditions; -import org.apache.hadoop.hdds.protocol.proto.ScmBlockLocationProtocolProtos; +import org.apache.hadoop.hdds.protocol.proto.HddsProtos; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -117,12 +117,12 @@ public Builder setChildrenMap(HashMap childrenMap) { } public Builder setChildrenMap( - List childrenMapList) { + List childrenMapList) { HashMap newChildrenMap = new LinkedHashMap<>(); - for (ScmBlockLocationProtocolProtos.ChildrenMap childrenMapProto : + for (HddsProtos.ChildrenMap childrenMapProto : childrenMapList) { String networkName = childrenMapProto.getNetworkName(); - ScmBlockLocationProtocolProtos.NodeType nodeType = + HddsProtos.NodeInterface nodeType = childrenMapProto.getNodeType(); Node node = Node.fromProtobuf(nodeType); newChildrenMap.put(networkName, node); @@ -471,10 +471,11 @@ public Node getLeaf(int leafIndex, List excludedScopes, } @Override - public ScmBlockLocationProtocolProtos.NodeType toProtobuf(int clientVersion) { + public HddsProtos.NodeInterface toProtobuf( + int clientVersion) { - ScmBlockLocationProtocolProtos.InnerNode.Builder innerNode = - ScmBlockLocationProtocolProtos.InnerNode.newBuilder() + HddsProtos.InnerNode.Builder innerNode = + HddsProtos.InnerNode.newBuilder() .setNumOfLeaves(numOfLeaves) .setNodeImpl( NodeImpl.toProtobuf(getNetworkName(), getNetworkLocation(), @@ -483,8 +484,8 @@ public ScmBlockLocationProtocolProtos.NodeType toProtobuf(int clientVersion) { if (childrenMap != null && !childrenMap.isEmpty()) { for (Map.Entry entry : childrenMap.entrySet()) { if (entry.getValue() != null) { - ScmBlockLocationProtocolProtos.ChildrenMap childrenMapProto = - ScmBlockLocationProtocolProtos.ChildrenMap.newBuilder() + HddsProtos.ChildrenMap childrenMapProto = + HddsProtos.ChildrenMap.newBuilder() .setNetworkName(entry.getKey()) .setNodeType(entry.getValue().toProtobuf(clientVersion)) .build(); @@ -494,24 +495,25 @@ public ScmBlockLocationProtocolProtos.NodeType toProtobuf(int clientVersion) { } innerNode.build(); - ScmBlockLocationProtocolProtos.NodeType nodeType = - ScmBlockLocationProtocolProtos.NodeType.newBuilder() + HddsProtos.NodeInterface nodeType = + HddsProtos.NodeInterface.newBuilder() .setInnerNode(innerNode).build(); return nodeType; } public static Node fromProtobuf( - ScmBlockLocationProtocolProtos.NodeType nodeType) { + HddsProtos.NodeInterface nodeType) { return nodeType.hasInnerNode() ? InnerNodeImpl.fromProtobuf(nodeType.getInnerNode()) : null; } public static InnerNode fromProtobuf( - ScmBlockLocationProtocolProtos.InnerNode innerNode) { + HddsProtos.InnerNode innerNode) { - ScmBlockLocationProtocolProtos.NodeImpl nodeImpl = innerNode.getNodeImpl(); + HddsProtos.NodeImpl nodeImpl = + innerNode.getNodeImpl(); InnerNodeImpl.Builder builder = new InnerNodeImpl.Builder() .setName(nodeImpl.getName()) .setLocation(nodeImpl.getLocation()) diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/Node.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/Node.java index 39283486e00..88614659d8e 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/Node.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/Node.java @@ -18,7 +18,7 @@ package org.apache.hadoop.hdds.scm.net; import org.apache.hadoop.hdds.protocol.DatanodeDetails; -import org.apache.hadoop.hdds.protocol.proto.ScmBlockLocationProtocolProtos; +import org.apache.hadoop.hdds.protocol.proto.HddsProtos; /** * The interface defines a node in a network topology. @@ -130,16 +130,16 @@ public interface Node { */ boolean isDescendant(String nodePath); - ScmBlockLocationProtocolProtos.NodeType toProtobuf(int clientVersion); + default HddsProtos.NodeInterface toProtobuf( + int clientVersion) { + return null; + } static Node fromProtobuf( - ScmBlockLocationProtocolProtos.NodeType nodeTypeProto) { - + HddsProtos.NodeInterface nodeTypeProto) { if (nodeTypeProto.hasDatanodeDetails()) { return DatanodeDetails.getFromProtoBuf( nodeTypeProto.getDatanodeDetails()); - } else if (nodeTypeProto.hasNodeImpl()) { - return NodeImpl.fromProtobuf(nodeTypeProto.getNodeImpl()); } else if (nodeTypeProto.hasInnerNode()) { return InnerNode.fromProtobuf(nodeTypeProto.getInnerNode()); } else { diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/NodeImpl.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/NodeImpl.java index f6aac09227f..a3191c4f0dc 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/NodeImpl.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/NodeImpl.java @@ -18,7 +18,7 @@ package org.apache.hadoop.hdds.scm.net; import com.google.common.base.Preconditions; -import org.apache.hadoop.hdds.protocol.proto.ScmBlockLocationProtocolProtos; +import org.apache.hadoop.hdds.protocol.proto.HddsProtos; import static org.apache.hadoop.hdds.scm.net.NetConstants.ROOT; import static org.apache.hadoop.hdds.scm.net.NetConstants.PATH_SEPARATOR_STR; @@ -230,46 +230,23 @@ public boolean isDescendant(String nodePath) { NetUtils.addSuffix(nodePath)); } - @Override - public ScmBlockLocationProtocolProtos.NodeType toProtobuf(int clientVersion) { - ScmBlockLocationProtocolProtos.NodeImpl nodeImpl = - ScmBlockLocationProtocolProtos.NodeImpl.newBuilder() - .setName(name) - .setLocation(location) - .setCost(cost) - .setLevel(level) - .build(); - - ScmBlockLocationProtocolProtos.NodeType nodeType = - ScmBlockLocationProtocolProtos.NodeType.newBuilder() - .setNodeImpl(nodeImpl).build(); + public static HddsProtos.NodeImpl toProtobuf( + String name, String location, int level, int cost) { - return nodeType; - } - - public static ScmBlockLocationProtocolProtos.NodeImpl toProtobuf(String name, - String location, int level, int cost) { - - ScmBlockLocationProtocolProtos.NodeImpl.Builder nodeImplBuilder = - ScmBlockLocationProtocolProtos.NodeImpl.newBuilder() + HddsProtos.NodeImpl.Builder nodeImplBuilder = + HddsProtos.NodeImpl.newBuilder() .setName(name) .setLocation(location) .setLevel(level) .setCost(cost); - ScmBlockLocationProtocolProtos.NodeImpl nodeImpl = nodeImplBuilder.build(); + HddsProtos.NodeImpl nodeImpl = + nodeImplBuilder.build(); return nodeImpl; } - public static Node fromProtobuf( - ScmBlockLocationProtocolProtos.NodeType nodeType) { - return nodeType.hasNodeImpl() - ? InnerNodeImpl.fromProtobuf(nodeType.getNodeImpl()) - : null; - } - public static NodeImpl fromProtobuf( - ScmBlockLocationProtocolProtos.NodeImpl nodeImpl) { + HddsProtos.NodeImpl nodeImpl) { return new NodeImpl(nodeImpl.getName(), nodeImpl.getLocation(), null, nodeImpl.getLevel(), nodeImpl.getCost()); } diff --git a/hadoop-hdds/interface-client/src/main/proto/hdds.proto b/hadoop-hdds/interface-client/src/main/proto/hdds.proto index 2699a8abdc0..928eb88ca6b 100644 --- a/hadoop-hdds/interface-client/src/main/proto/hdds.proto +++ b/hadoop-hdds/interface-client/src/main/proto/hdds.proto @@ -498,3 +498,26 @@ message CompactionLogEntryProto { repeated CompactionFileInfoProto outputFileIntoList = 4; optional string compactionReason = 5; } + +message NodeImpl { + required string name = 1; + required string location = 2; + required uint32 cost = 3; + required uint32 level = 4; +} + +message NodeInterface { + optional DatanodeDetailsProto datanodeDetails = 1; + optional InnerNode innerNode = 3; +} + +message ChildrenMap { + required string networkName = 1; + required NodeInterface nodeType = 2; +} + +message InnerNode { + required NodeImpl nodeImpl = 1; + required uint32 numOfLeaves = 2; + repeated ChildrenMap childrenMap = 3; +} diff --git a/hadoop-hdds/interface-server/src/main/proto/ScmServerProtocol.proto b/hadoop-hdds/interface-server/src/main/proto/ScmServerProtocol.proto index 07a3a29030f..3d281975f2b 100644 --- a/hadoop-hdds/interface-server/src/main/proto/ScmServerProtocol.proto +++ b/hadoop-hdds/interface-server/src/main/proto/ScmServerProtocol.proto @@ -29,7 +29,6 @@ option java_generate_equals_and_hash = true; package hadoop.hdds.block; import "hdds.proto"; -import "ScmServerDatanodeHeartbeatProtocol.proto"; // SCM Block protocol @@ -233,54 +232,6 @@ message SortDatanodesResponseProto{ repeated DatanodeDetailsProto node = 1; } -message NodeImpl { - required string name = 1; - required string location = 2; - required uint32 cost = 3; - required uint32 level = 4; -} - -message CommandCount { - required SCMCommandProto.Type command = 1; - required uint32 count = 2; -} - -message NodeStatus { - required NodeOperationalState operationalState = 1; - required NodeState health = 2; - required uint64 opStateExpiryEpochSeconds = 3; -} - -message DatanodeInfo { - required DatanodeDetailsProto datanodeDetails = 1; - required uint64 lastHeartbeatTime = 2; - required uint64 lastStatsUpdatedTime = 3; - required uint32 failedVolumeCount = 4; - repeated StorageReportProto storageReportProto = 5; - repeated MetadataStorageReportProto metadataStorageReportProto = 6; - required LayoutVersionProto layoutVersionProto = 7; - repeated CommandCount commandCount = 8; - required NodeStatus nodeStatus = 9; -} - -message NodeType { - optional DatanodeDetailsProto datanodeDetails = 1; - optional DatanodeInfo datanodeInfo = 2; - optional InnerNode innerNode = 3; - optional NodeImpl nodeImpl = 4; -} - -message ChildrenMap { - required string networkName = 1; - required NodeType nodeType = 2; -} - -message InnerNode { - repeated ChildrenMap childrenMap = 1; - optional uint32 numOfLeaves = 2; - required NodeImpl nodeImpl = 3; -} - message GetClusterTreeRequestProto { } diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/DatanodeInfo.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/DatanodeInfo.java index 22bf255ff5c..ab296fc52bf 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/DatanodeInfo.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/DatanodeInfo.java @@ -22,7 +22,6 @@ import com.google.common.annotations.VisibleForTesting; import org.apache.hadoop.hdds.protocol.DatanodeDetails; -import org.apache.hadoop.hdds.protocol.proto.ScmBlockLocationProtocolProtos; import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.SCMCommandProto; import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.CommandQueueReportProto; import org.apache.hadoop.hdds.protocol.proto @@ -31,8 +30,6 @@ .StorageContainerDatanodeProtocolProtos.StorageReportProto; import org.apache.hadoop.hdds.protocol.proto .StorageContainerDatanodeProtocolProtos.MetadataStorageReportProto; -import org.apache.hadoop.hdds.scm.net.Node; -import org.apache.hadoop.hdds.scm.net.NodeImpl; import org.apache.hadoop.util.Time; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -85,26 +82,6 @@ public DatanodeInfo(DatanodeDetails datanodeDetails, NodeStatus nodeStatus, this.commandCounts = new HashMap<>(); } - @SuppressWarnings("parameternumber") - private DatanodeInfo(DatanodeDetails datanodeDetails, long lastHeartbeatTime, - long lastStatsUpdatedTime, int failedVolumeCount, - List storageReports, - List metadataStorageReports, - LayoutVersionProto lastKnownLayoutVersion, - Map commandCounts, - NodeStatus nodeStatus) { - super(datanodeDetails); - this.lock = new ReentrantReadWriteLock(); - this.lastHeartbeatTime = lastHeartbeatTime; - this.lastStatsUpdatedTime = lastStatsUpdatedTime; - this.failedVolumeCount = failedVolumeCount; - this.storageReports = storageReports; - this.metadataStorageReports = metadataStorageReports; - this.lastKnownLayoutVersion = lastKnownLayoutVersion; - this.commandCounts = commandCounts; - this.nodeStatus = nodeStatus; - } - /** * Updates the last heartbeat time with current time. */ @@ -377,74 +354,4 @@ public int hashCode() { public boolean equals(Object obj) { return super.equals(obj); } - - @Override - public ScmBlockLocationProtocolProtos.NodeType toProtobuf(int clientVersion) { - ScmBlockLocationProtocolProtos.DatanodeInfo.Builder datanodeInfo = - ScmBlockLocationProtocolProtos.DatanodeInfo.newBuilder() - .setDatanodeDetails(toProtoBuilder(clientVersion).build()) - .setLastHeartbeatTime(lastHeartbeatTime) - .setLastStatsUpdatedTime(lastStatsUpdatedTime) - .setFailedVolumeCount(failedVolumeCount) - .setLayoutVersionProto(lastKnownLayoutVersion) - .setNodeStatus(nodeStatus.toProtobuf()); - - for (StorageReportProto storageReport : storageReports) { - datanodeInfo.addStorageReportProto(storageReport); - } - - for (MetadataStorageReportProto metadataStorageReport : - metadataStorageReports) { - datanodeInfo.addMetadataStorageReportProto(metadataStorageReport); - } - - for (Map.Entry entry : - commandCounts.entrySet()) { - ScmBlockLocationProtocolProtos.CommandCount commandCountProto = - ScmBlockLocationProtocolProtos.CommandCount.newBuilder() - .setCommand(entry.getKey()) - .setCount(entry.getValue()) - .build(); - datanodeInfo.addCommandCount(commandCountProto); - } - - ScmBlockLocationProtocolProtos.NodeType nodeType = - ScmBlockLocationProtocolProtos.NodeType.newBuilder() - .setDatanodeInfo(datanodeInfo).build(); - - return nodeType; - } - - public static Node fromProtobuf( - ScmBlockLocationProtocolProtos.NodeType nodeType) { - return nodeType.hasDatanodeInfo() - ? DatanodeInfo.fromProtobuf(nodeType.getDatanodeInfo()) - : null; - } - - public static NodeImpl fromProtobuf( - ScmBlockLocationProtocolProtos.DatanodeInfo datanodeInfo) { - return new DatanodeInfo( - DatanodeDetails.getFromProtoBuf(datanodeInfo.getDatanodeDetails()), - datanodeInfo.getLastHeartbeatTime(), - datanodeInfo.getLastStatsUpdatedTime(), - datanodeInfo.getFailedVolumeCount(), - datanodeInfo.getStorageReportProtoList(), - datanodeInfo.getMetadataStorageReportProtoList(), - datanodeInfo.getLayoutVersionProto(), - DatanodeInfo.fromProtobuf(datanodeInfo.getCommandCountList()), - NodeStatus.fromProtobuf(datanodeInfo.getNodeStatus())); - } - - public static Map fromProtobuf( - List commandCountList) { - Map commandCounts = new HashMap<>(); - for (ScmBlockLocationProtocolProtos.CommandCount commandCountProto : - commandCountList) { - SCMCommandProto.Type commandType = commandCountProto.getCommand(); - int count = commandCountProto.getCount(); - commandCounts.put(commandType, count); - } - return commandCounts; - } } diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/NodeStatus.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/NodeStatus.java index a5c8bb6c246..3aff2f456e4 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/NodeStatus.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/NodeStatus.java @@ -20,7 +20,6 @@ import com.google.common.collect.ImmutableSet; import org.apache.hadoop.hdds.protocol.proto.HddsProtos; -import org.apache.hadoop.hdds.protocol.proto.ScmBlockLocationProtocolProtos; import java.util.EnumSet; import java.util.Objects; @@ -262,20 +261,4 @@ public int compareTo(NodeStatus o) { } return order; } - - public ScmBlockLocationProtocolProtos.NodeStatus toProtobuf() { - ScmBlockLocationProtocolProtos.NodeStatus.Builder nodeStatus = - ScmBlockLocationProtocolProtos.NodeStatus.newBuilder() - .setOperationalState(operationalState) - .setHealth(health) - .setOpStateExpiryEpochSeconds(opStateExpiryEpochSeconds); - - return nodeStatus.build(); - } - - public static NodeStatus fromProtobuf( - ScmBlockLocationProtocolProtos.NodeStatus nodeStatus) { - return new NodeStatus(nodeStatus.getOperationalState(), - nodeStatus.getHealth(), nodeStatus.getOpStateExpiryEpochSeconds()); - } } From 113d0451aedd21c9e576f014d7a186bfa80559aa Mon Sep 17 00:00:00 2001 From: tanvipenumudy Date: Tue, 13 Feb 2024 10:48:47 +0530 Subject: [PATCH 12/23] NetworkTopologyImpl#getDistanceCost: account for null objects, fix failing tests --- .../apache/hadoop/hdds/scm/net/NetworkTopologyImpl.java | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/NetworkTopologyImpl.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/NetworkTopologyImpl.java index 75db2b7adab..f6f013259c5 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/NetworkTopologyImpl.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/NetworkTopologyImpl.java @@ -735,8 +735,13 @@ public int getDistanceCost(Node node1, Node node2) { int cost = 0; netlock.readLock().lock(); try { - if (!(node1.getAncestor(level1 - 1).equals(clusterTree)) || - !(node2.getAncestor(level2 - 1).equals(clusterTree))) { + Node ancestor1 = node1.getAncestor(level1 - 1); + boolean node1Topology = (ancestor1 != null && clusterTree != null && + !ancestor1.equals(clusterTree)) || (ancestor1 != clusterTree); + Node ancestor2 = node2.getAncestor(level2 - 1); + boolean node2Topology = (ancestor2 != null && clusterTree != null && + !ancestor2.equals(clusterTree)) || (ancestor2 != clusterTree); + if (node1Topology || node2Topology) { LOG.debug("One of the nodes is outside of network topology"); return Integer.MAX_VALUE; } From f4594a30df90a49739c55f7f1b5c642baadd3fb6 Mon Sep 17 00:00:00 2001 From: tanvipenumudy Date: Tue, 13 Feb 2024 11:39:17 +0530 Subject: [PATCH 13/23] Change NodeType to NetworkNode --- .../hadoop/hdds/protocol/DatanodeDetails.java | 14 ++++++------ .../apache/hadoop/hdds/scm/net/InnerNode.java | 2 +- .../hadoop/hdds/scm/net/InnerNodeImpl.java | 22 +++++++++---------- .../org/apache/hadoop/hdds/scm/net/Node.java | 12 +++++----- .../src/main/proto/hdds.proto | 4 ++-- .../hadoop/ozone/om/KeyManagerImpl.java | 2 +- 6 files changed, 28 insertions(+), 28 deletions(-) diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/protocol/DatanodeDetails.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/protocol/DatanodeDetails.java index 7b7c632cf9d..8b5518cad35 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/protocol/DatanodeDetails.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/protocol/DatanodeDetails.java @@ -1039,18 +1039,18 @@ public void setBuildDate(String date) { } @Override - public HddsProtos.NodeInterface toProtobuf( + public HddsProtos.NetworkNode toProtobuf( int clientVersion) { - HddsProtos.NodeInterface nodeType = - HddsProtos.NodeInterface.newBuilder() + HddsProtos.NetworkNode networkNode = + HddsProtos.NetworkNode.newBuilder() .setDatanodeDetails(toProtoBuilder(clientVersion).build()).build(); - return nodeType; + return networkNode; } public static Node fromProtobuf( - HddsProtos.NodeInterface nodeType) { - return nodeType.hasDatanodeDetails() - ? DatanodeDetails.getFromProtoBuf(nodeType.getDatanodeDetails()) + HddsProtos.NetworkNode networkNode) { + return networkNode.hasDatanodeDetails() + ? DatanodeDetails.getFromProtoBuf(networkNode.getDatanodeDetails()) : null; } } diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/InnerNode.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/InnerNode.java index c8256341db5..6074e7da0af 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/InnerNode.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/InnerNode.java @@ -93,7 +93,7 @@ Node getLeaf(int leafIndex, List excludedScopes, Collection excludedNodes, int ancestorGen); @Override - HddsProtos.NodeInterface toProtobuf(int clientVersion); + HddsProtos.NetworkNode toProtobuf(int clientVersion); boolean equals(Object o); diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/InnerNodeImpl.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/InnerNodeImpl.java index e5af3c78b6b..e3964ff11d9 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/InnerNodeImpl.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/InnerNodeImpl.java @@ -122,9 +122,9 @@ public Builder setChildrenMap( for (HddsProtos.ChildrenMap childrenMapProto : childrenMapList) { String networkName = childrenMapProto.getNetworkName(); - HddsProtos.NodeInterface nodeType = - childrenMapProto.getNodeType(); - Node node = Node.fromProtobuf(nodeType); + HddsProtos.NetworkNode networkNode = + childrenMapProto.getNetworkNode(); + Node node = Node.fromProtobuf(networkNode); newChildrenMap.put(networkName, node); } this.childrenMap = newChildrenMap; @@ -471,7 +471,7 @@ public Node getLeaf(int leafIndex, List excludedScopes, } @Override - public HddsProtos.NodeInterface toProtobuf( + public HddsProtos.NetworkNode toProtobuf( int clientVersion) { HddsProtos.InnerNode.Builder innerNode = @@ -487,7 +487,7 @@ public HddsProtos.NodeInterface toProtobuf( HddsProtos.ChildrenMap childrenMapProto = HddsProtos.ChildrenMap.newBuilder() .setNetworkName(entry.getKey()) - .setNodeType(entry.getValue().toProtobuf(clientVersion)) + .setNetworkNode(entry.getValue().toProtobuf(clientVersion)) .build(); innerNode.addChildrenMap(childrenMapProto); } @@ -495,17 +495,17 @@ public HddsProtos.NodeInterface toProtobuf( } innerNode.build(); - HddsProtos.NodeInterface nodeType = - HddsProtos.NodeInterface.newBuilder() + HddsProtos.NetworkNode networkNode = + HddsProtos.NetworkNode.newBuilder() .setInnerNode(innerNode).build(); - return nodeType; + return networkNode; } public static Node fromProtobuf( - HddsProtos.NodeInterface nodeType) { - return nodeType.hasInnerNode() - ? InnerNodeImpl.fromProtobuf(nodeType.getInnerNode()) + HddsProtos.NetworkNode networkNode) { + return networkNode.hasInnerNode() + ? InnerNodeImpl.fromProtobuf(networkNode.getInnerNode()) : null; } diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/Node.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/Node.java index 88614659d8e..50f702cce08 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/Node.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/Node.java @@ -130,18 +130,18 @@ public interface Node { */ boolean isDescendant(String nodePath); - default HddsProtos.NodeInterface toProtobuf( + default HddsProtos.NetworkNode toProtobuf( int clientVersion) { return null; } static Node fromProtobuf( - HddsProtos.NodeInterface nodeTypeProto) { - if (nodeTypeProto.hasDatanodeDetails()) { + HddsProtos.NetworkNode networkNode) { + if (networkNode.hasDatanodeDetails()) { return DatanodeDetails.getFromProtoBuf( - nodeTypeProto.getDatanodeDetails()); - } else if (nodeTypeProto.hasInnerNode()) { - return InnerNode.fromProtobuf(nodeTypeProto.getInnerNode()); + networkNode.getDatanodeDetails()); + } else if (networkNode.hasInnerNode()) { + return InnerNode.fromProtobuf(networkNode.getInnerNode()); } else { return null; } diff --git a/hadoop-hdds/interface-client/src/main/proto/hdds.proto b/hadoop-hdds/interface-client/src/main/proto/hdds.proto index 928eb88ca6b..d7099a46d9e 100644 --- a/hadoop-hdds/interface-client/src/main/proto/hdds.proto +++ b/hadoop-hdds/interface-client/src/main/proto/hdds.proto @@ -506,14 +506,14 @@ message NodeImpl { required uint32 level = 4; } -message NodeInterface { +message NetworkNode { optional DatanodeDetailsProto datanodeDetails = 1; optional InnerNode innerNode = 3; } message ChildrenMap { required string networkName = 1; - required NodeInterface nodeType = 2; + required NetworkNode networkNode = 2; } message InnerNode { 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 a12072b67d8..5c0d4278112 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 @@ -1865,7 +1865,7 @@ public List sortDatanodes(String clientMachine, return scmClient.getBlockClient() .sortDatanodes(nodeList, clientMachine); } catch (IOException e) { - LOG.warn("Unable to sort datanodes based on distance to client, " + LOG.debug("Unable to sort datanodes based on distance to client, " + " volume={}, bucket={}, key={}, client={}, datanodes={}, " + " exception={}", keyInfo.getVolumeName(), keyInfo.getBucketName(), From 80d5a6ed84cfa1b9bcb71ee1f901d10cc2fce690 Mon Sep 17 00:00:00 2001 From: tanvipenumudy Date: Tue, 13 Feb 2024 12:00:27 +0530 Subject: [PATCH 14/23] Change NodeImpl to NodeTopology --- .../hadoop/hdds/scm/net/InnerNodeImpl.java | 14 +++++++------- .../apache/hadoop/hdds/scm/net/NodeImpl.java | 18 +++++++++--------- .../interface-client/src/main/proto/hdds.proto | 4 ++-- 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/InnerNodeImpl.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/InnerNodeImpl.java index e3964ff11d9..ee92eaa6603 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/InnerNodeImpl.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/InnerNodeImpl.java @@ -477,7 +477,7 @@ public HddsProtos.NetworkNode toProtobuf( HddsProtos.InnerNode.Builder innerNode = HddsProtos.InnerNode.newBuilder() .setNumOfLeaves(numOfLeaves) - .setNodeImpl( + .setNodeTopology( NodeImpl.toProtobuf(getNetworkName(), getNetworkLocation(), getLevel(), getCost())); @@ -512,13 +512,13 @@ public static Node fromProtobuf( public static InnerNode fromProtobuf( HddsProtos.InnerNode innerNode) { - HddsProtos.NodeImpl nodeImpl = - innerNode.getNodeImpl(); + HddsProtos.NodeTopology nodeTopology = + innerNode.getNodeTopology(); InnerNodeImpl.Builder builder = new InnerNodeImpl.Builder() - .setName(nodeImpl.getName()) - .setLocation(nodeImpl.getLocation()) - .setCost(nodeImpl.getCost()) - .setLevel(nodeImpl.getLevel()) + .setName(nodeTopology.getName()) + .setLocation(nodeTopology.getLocation()) + .setCost(nodeTopology.getCost()) + .setLevel(nodeTopology.getLevel()) .setChildrenMap(innerNode.getChildrenMapList()) .setNumOfLeaves(innerNode.getNumOfLeaves()); diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/NodeImpl.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/NodeImpl.java index a3191c4f0dc..18fa0b01bde 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/NodeImpl.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/NodeImpl.java @@ -230,25 +230,25 @@ public boolean isDescendant(String nodePath) { NetUtils.addSuffix(nodePath)); } - public static HddsProtos.NodeImpl toProtobuf( + public static HddsProtos.NodeTopology toProtobuf( String name, String location, int level, int cost) { - HddsProtos.NodeImpl.Builder nodeImplBuilder = - HddsProtos.NodeImpl.newBuilder() + HddsProtos.NodeTopology.Builder nodeTopologyBuilder = + HddsProtos.NodeTopology.newBuilder() .setName(name) .setLocation(location) .setLevel(level) .setCost(cost); - HddsProtos.NodeImpl nodeImpl = - nodeImplBuilder.build(); - return nodeImpl; + HddsProtos.NodeTopology nodeTopology = + nodeTopologyBuilder.build(); + return nodeTopology; } public static NodeImpl fromProtobuf( - HddsProtos.NodeImpl nodeImpl) { - return new NodeImpl(nodeImpl.getName(), nodeImpl.getLocation(), null, - nodeImpl.getLevel(), nodeImpl.getCost()); + HddsProtos.NodeTopology nodeTopology) { + return new NodeImpl(nodeTopology.getName(), nodeTopology.getLocation(), + null, nodeTopology.getLevel(), nodeTopology.getCost()); } @Override diff --git a/hadoop-hdds/interface-client/src/main/proto/hdds.proto b/hadoop-hdds/interface-client/src/main/proto/hdds.proto index d7099a46d9e..3571a259b7e 100644 --- a/hadoop-hdds/interface-client/src/main/proto/hdds.proto +++ b/hadoop-hdds/interface-client/src/main/proto/hdds.proto @@ -499,7 +499,7 @@ message CompactionLogEntryProto { optional string compactionReason = 5; } -message NodeImpl { +message NodeTopology { required string name = 1; required string location = 2; required uint32 cost = 3; @@ -517,7 +517,7 @@ message ChildrenMap { } message InnerNode { - required NodeImpl nodeImpl = 1; + required NodeTopology nodeTopology = 1; required uint32 numOfLeaves = 2; repeated ChildrenMap childrenMap = 3; } From fff4abee94cf2a90a99fc360af74127e91452165 Mon Sep 17 00:00:00 2001 From: tanvipenumudy Date: Tue, 13 Feb 2024 14:23:00 +0530 Subject: [PATCH 15/23] Modify proto objects to optional and corresponding usages --- .../hadoop/hdds/protocol/DatanodeDetails.java | 8 --- .../hadoop/hdds/scm/net/InnerNodeImpl.java | 61 ++++++++++--------- .../apache/hadoop/hdds/scm/net/NodeImpl.java | 13 +--- .../src/main/proto/hdds.proto | 16 ++--- .../ozone/TestGetClusterTreeInformation.java | 2 +- 5 files changed, 43 insertions(+), 57 deletions(-) diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/protocol/DatanodeDetails.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/protocol/DatanodeDetails.java index 8b5518cad35..6ddd50871bd 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/protocol/DatanodeDetails.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/protocol/DatanodeDetails.java @@ -35,7 +35,6 @@ import org.apache.hadoop.hdds.protocol.proto.HddsProtos; import org.apache.hadoop.hdds.protocol.proto.HddsProtos.ExtendedDatanodeDetailsProto; import org.apache.hadoop.hdds.scm.net.NetConstants; -import org.apache.hadoop.hdds.scm.net.Node; import org.apache.hadoop.hdds.scm.net.NodeImpl; import com.google.common.base.Preconditions; @@ -1046,11 +1045,4 @@ public HddsProtos.NetworkNode toProtobuf( .setDatanodeDetails(toProtoBuilder(clientVersion).build()).build(); return networkNode; } - - public static Node fromProtobuf( - HddsProtos.NetworkNode networkNode) { - return networkNode.hasDatanodeDetails() - ? DatanodeDetails.getFromProtoBuf(networkNode.getDatanodeDetails()) - : null; - } } diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/InnerNodeImpl.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/InnerNodeImpl.java index ee92eaa6603..332dddac25c 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/InnerNodeImpl.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/InnerNodeImpl.java @@ -111,20 +111,15 @@ public Builder setLevel(int level) { return this; } - public Builder setChildrenMap(HashMap childrenMap) { - this.childrenMap = childrenMap; - return this; - } - public Builder setChildrenMap( List childrenMapList) { HashMap newChildrenMap = new LinkedHashMap<>(); for (HddsProtos.ChildrenMap childrenMapProto : childrenMapList) { - String networkName = childrenMapProto.getNetworkName(); - HddsProtos.NetworkNode networkNode = - childrenMapProto.getNetworkNode(); - Node node = Node.fromProtobuf(networkNode); + String networkName = childrenMapProto.hasNetworkName() ? + childrenMapProto.getNetworkName() : null; + Node node = childrenMapProto.hasNetworkNode() ? + Node.fromProtobuf(childrenMapProto.getNetworkNode()) : null; newChildrenMap.put(networkName, node); } this.childrenMap = newChildrenMap; @@ -502,28 +497,34 @@ public HddsProtos.NetworkNode toProtobuf( return networkNode; } - public static Node fromProtobuf( - HddsProtos.NetworkNode networkNode) { - return networkNode.hasInnerNode() - ? InnerNodeImpl.fromProtobuf(networkNode.getInnerNode()) - : null; - } + public static InnerNode fromProtobuf(HddsProtos.InnerNode innerNode) { + InnerNodeImpl.Builder builder = new InnerNodeImpl.Builder(); + + if (innerNode.hasNodeTopology()) { + HddsProtos.NodeTopology nodeTopology = innerNode.getNodeTopology(); + + if (nodeTopology.hasName()) { + builder.setName(nodeTopology.getName()); + } + if (nodeTopology.hasLocation()) { + builder.setLocation(nodeTopology.getLocation()); + } + if (nodeTopology.hasLevel()) { + builder.setLevel(nodeTopology.getLevel()); + } + if (nodeTopology.hasCost()) { + builder.setCost(nodeTopology.getCost()); + } + } + + if (!innerNode.getChildrenMapList().isEmpty()) { + builder.setChildrenMap(innerNode.getChildrenMapList()); + } + if (innerNode.hasNumOfLeaves()) { + builder.setNumOfLeaves(innerNode.getNumOfLeaves()); + } - public static InnerNode fromProtobuf( - HddsProtos.InnerNode innerNode) { - - HddsProtos.NodeTopology nodeTopology = - innerNode.getNodeTopology(); - InnerNodeImpl.Builder builder = new InnerNodeImpl.Builder() - .setName(nodeTopology.getName()) - .setLocation(nodeTopology.getLocation()) - .setCost(nodeTopology.getCost()) - .setLevel(nodeTopology.getLevel()) - .setChildrenMap(innerNode.getChildrenMapList()) - .setNumOfLeaves(innerNode.getNumOfLeaves()); - - InnerNode res = builder.build(); - return res; + return builder.build(); } @Override diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/NodeImpl.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/NodeImpl.java index 18fa0b01bde..e4d76cd3dbc 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/NodeImpl.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/net/NodeImpl.java @@ -230,8 +230,8 @@ public boolean isDescendant(String nodePath) { NetUtils.addSuffix(nodePath)); } - public static HddsProtos.NodeTopology toProtobuf( - String name, String location, int level, int cost) { + public static HddsProtos.NodeTopology toProtobuf(String name, String location, + int level, int cost) { HddsProtos.NodeTopology.Builder nodeTopologyBuilder = HddsProtos.NodeTopology.newBuilder() @@ -240,17 +240,10 @@ public static HddsProtos.NodeTopology toProtobuf( .setLevel(level) .setCost(cost); - HddsProtos.NodeTopology nodeTopology = - nodeTopologyBuilder.build(); + HddsProtos.NodeTopology nodeTopology = nodeTopologyBuilder.build(); return nodeTopology; } - public static NodeImpl fromProtobuf( - HddsProtos.NodeTopology nodeTopology) { - return new NodeImpl(nodeTopology.getName(), nodeTopology.getLocation(), - null, nodeTopology.getLevel(), nodeTopology.getCost()); - } - @Override public boolean equals(Object to) { if (to == null) { diff --git a/hadoop-hdds/interface-client/src/main/proto/hdds.proto b/hadoop-hdds/interface-client/src/main/proto/hdds.proto index 3571a259b7e..40584531235 100644 --- a/hadoop-hdds/interface-client/src/main/proto/hdds.proto +++ b/hadoop-hdds/interface-client/src/main/proto/hdds.proto @@ -500,10 +500,10 @@ message CompactionLogEntryProto { } message NodeTopology { - required string name = 1; - required string location = 2; - required uint32 cost = 3; - required uint32 level = 4; + optional string name = 1; + optional string location = 2; + optional uint32 cost = 3; + optional uint32 level = 4; } message NetworkNode { @@ -512,12 +512,12 @@ message NetworkNode { } message ChildrenMap { - required string networkName = 1; - required NetworkNode networkNode = 2; + optional string networkName = 1; + optional NetworkNode networkNode = 2; } message InnerNode { - required NodeTopology nodeTopology = 1; - required uint32 numOfLeaves = 2; + optional NodeTopology nodeTopology = 1; + optional uint32 numOfLeaves = 2; repeated ChildrenMap childrenMap = 3; } diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestGetClusterTreeInformation.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestGetClusterTreeInformation.java index 3f6377e58f8..a0140a85f46 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestGetClusterTreeInformation.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestGetClusterTreeInformation.java @@ -80,6 +80,6 @@ public void testGetClusterTreeInformation() throws IOException { InnerNode expectedInnerNode = (InnerNode) scm.getClusterMap().getNode(ROOT); InnerNode actualInnerNode = scmBlockLocationClient.getNetworkTopology(); - assertEquals(expectedInnerNode, actualInnerNode); + assertEq uals(expectedInnerNode, actualInnerNode); } } From 140e3f99ea84f5210c0ee0a10999649cccbd42f8 Mon Sep 17 00:00:00 2001 From: tanvipenumudy Date: Tue, 13 Feb 2024 14:24:01 +0530 Subject: [PATCH 16/23] Fix typo --- .../org/apache/hadoop/ozone/TestGetClusterTreeInformation.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestGetClusterTreeInformation.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestGetClusterTreeInformation.java index a0140a85f46..3f6377e58f8 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestGetClusterTreeInformation.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestGetClusterTreeInformation.java @@ -80,6 +80,6 @@ public void testGetClusterTreeInformation() throws IOException { InnerNode expectedInnerNode = (InnerNode) scm.getClusterMap().getNode(ROOT); InnerNode actualInnerNode = scmBlockLocationClient.getNetworkTopology(); - assertEq uals(expectedInnerNode, actualInnerNode); + assertEquals(expectedInnerNode, actualInnerNode); } } From 6bda4a74187789d5348c87048796daedb27b6fd5 Mon Sep 17 00:00:00 2001 From: tanvipenumudy Date: Thu, 22 Feb 2024 10:52:00 +0530 Subject: [PATCH 17/23] Resolve node location for reads from non-datanodes hosts within OM --- .../hadoop/ozone/om/KeyManagerImpl.java | 90 ++++++++++++------- 1 file changed, 58 insertions(+), 32 deletions(-) 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 5707dd9aec0..7a6f2e529bb 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 @@ -49,7 +49,9 @@ import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.hdds.protocol.DatanodeDetails; import org.apache.hadoop.hdds.scm.container.common.helpers.ContainerWithPipeline; +import org.apache.hadoop.hdds.scm.net.InnerNode; import org.apache.hadoop.hdds.scm.net.Node; +import org.apache.hadoop.hdds.scm.net.NodeImpl; import org.apache.hadoop.hdds.scm.pipeline.Pipeline; import org.apache.hadoop.hdds.scm.storage.BlockLocationInfo; import org.apache.hadoop.hdds.utils.BackgroundService; @@ -58,6 +60,9 @@ 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.TableMapping; import org.apache.hadoop.ozone.OmUtils; import org.apache.hadoop.ozone.OzoneAcl; import org.apache.hadoop.ozone.om.exceptions.OMException; @@ -96,6 +101,7 @@ import org.apache.hadoop.ozone.security.acl.OzoneObj; import org.apache.hadoop.ozone.security.acl.RequestContext; import org.apache.hadoop.security.SecurityUtil; +import org.apache.hadoop.util.ReflectionUtils; import org.apache.hadoop.util.Time; import com.google.common.annotations.VisibleForTesting; @@ -108,6 +114,7 @@ import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_BLOCK_TOKEN_ENABLED; import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_BLOCK_TOKEN_ENABLED_DEFAULT; import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.BlockTokenSecretProto.AccessModeProto.READ; +import static org.apache.hadoop.hdds.scm.net.NetConstants.NODE_COST_DEFAULT; import static org.apache.hadoop.hdds.utils.HddsServerUtil.getRemoteUser; import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_BLOCK_DELETING_SERVICE_INTERVAL; import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_BLOCK_DELETING_SERVICE_INTERVAL_DEFAULT; @@ -190,6 +197,7 @@ public class KeyManagerImpl implements KeyManager { private BackgroundService openKeyCleanupService; private BackgroundService multipartUploadCleanupService; private SnapshotDirectoryCleaningService snapshotDirectoryCleaningService; + private DNSToSwitchMapping dnsToSwitchMapping; public KeyManagerImpl(OzoneManager om, ScmClient scmClient, OzoneConfiguration conf, OMPerformanceMetrics metrics) { @@ -339,6 +347,16 @@ public void start(OzoneConfiguration configuration) { ozoneManager, configuration); multipartUploadCleanupService.start(); } + + Class dnsToSwitchMappingClass = + configuration.getClass( + DFSConfigKeysLegacy.NET_TOPOLOGY_NODE_SWITCH_MAPPING_IMPL_KEY, + TableMapping.class, DNSToSwitchMapping.class); + DNSToSwitchMapping newInstance = ReflectionUtils.newInstance( + dnsToSwitchMappingClass, configuration); + dnsToSwitchMapping = + ((newInstance instanceof CachedDNSToSwitchMapping) ? newInstance + : new CachedDNSToSwitchMapping(newInstance)); } KeyProviderCryptoExtension getKMSProvider() { @@ -1865,8 +1883,7 @@ void sortDatanodes(String clientMachine, OmKeyInfo... keyInfos) { LOG.warn("No datanodes in pipeline {}", pipeline.getId()); continue; } - sortedNodes = sortDatanodes(clientMachine, nodes, keyInfo, - uuidList); + sortedNodes = sortDatanodes(nodes, clientMachine); if (sortedNodes != null) { sortedPipelines.put(uuidSet, sortedNodes); } @@ -1880,38 +1897,15 @@ void sortDatanodes(String clientMachine, OmKeyInfo... keyInfos) { } } - public List sortDatanodes(String clientMachine, - List nodes, OmKeyInfo keyInfo, List nodeList) { - DatanodeDetails client = null; - List possibleClients = - getClientNodesByAddress(clientMachine, nodes); - if (possibleClients.size() > 0) { - client = possibleClients.get(0); - } else { - /** - * In case of read from a non-datanode host, return - * {@link ScmBlockLocationProtocol#sortDatanodes(List, String)}. - */ - try { - return scmClient.getBlockClient() - .sortDatanodes(nodeList, clientMachine); - } catch (IOException e) { - LOG.debug("Unable to sort datanodes based on distance to client, " - + " volume={}, bucket={}, key={}, client={}, datanodes={}, " - + " exception={}", - keyInfo.getVolumeName(), keyInfo.getBucketName(), - keyInfo.getKeyName(), clientMachine, nodeList, e.getMessage()); - } - } - List sortedNodeList = ozoneManager.getClusterMap() + public List sortDatanodes(List nodes, + String clientMachine) { + final Node client = getClientNode(clientMachine, nodes); + return ozoneManager.getClusterMap() .sortByDistanceCost(client, nodes, nodes.size()); - List ret = new ArrayList<>(); - sortedNodeList.stream().forEach(node -> ret.add((DatanodeDetails) node)); - return ret; } - private List getClientNodesByAddress(String clientMachine, - List nodes) { + private Node getClientNode(String clientMachine, + List nodes) { List matchingNodes = new ArrayList<>(); boolean useHostname = ozoneManager.getConfiguration().getBoolean( DFSConfigKeysLegacy.DFS_DATANODE_USE_DN_HOSTNAME, @@ -1922,7 +1916,39 @@ private List getClientNodesByAddress(String clientMachine, matchingNodes.add(node); } } - return matchingNodes; + return !matchingNodes.isEmpty() ? matchingNodes.get(0) : + getOtherNode(clientMachine); + } + + public String resolveNodeLocation(String hostname) { + List hosts = Collections.singletonList(hostname); + List resolvedHosts = dnsToSwitchMapping.resolve(hosts); + if (resolvedHosts != null && !resolvedHosts.isEmpty()) { + String location = resolvedHosts.get(0); + LOG.debug("Node {} resolved to location {}", hostname, location); + return location; + } else { + LOG.debug("Node resolution did not yield any result for {}", hostname); + return null; + } + } + + private Node getOtherNode(String clientMachine) { + try { + String clientLocation = resolveNodeLocation(clientMachine); + if (clientLocation != null) { + Node rack = ozoneManager.getClusterMap().getNode(clientLocation); + if (rack instanceof InnerNode) { + return new NodeImpl(clientMachine, clientLocation, + (InnerNode) rack, rack.getLevel() + 1, + NODE_COST_DEFAULT); + } + } + } catch (Exception e) { + LOG.info("Could not resolve client {}: {}", + clientMachine, e.getMessage()); + } + return null; } private static List toNodeUuid(Collection nodes) { From b62695f5dadc504b689d1d7a21f6f258c3e14ba7 Mon Sep 17 00:00:00 2001 From: tanvipenumudy Date: Thu, 22 Feb 2024 11:41:20 +0530 Subject: [PATCH 18/23] Remove force refetching of clusterTree information --- .../hdds/scm/client/ScmTopologyClient.java | 4 ---- .../apache/hadoop/ozone/om/OzoneManager.java | 19 ++++--------------- 2 files changed, 4 insertions(+), 19 deletions(-) diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/client/ScmTopologyClient.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/client/ScmTopologyClient.java index de33bef14ee..2e42df95734 100644 --- a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/client/ScmTopologyClient.java +++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/client/ScmTopologyClient.java @@ -61,10 +61,6 @@ public InnerNode getClusterTree() { "ScmBlockLocationClient must have been initialized already."); } - public void refetchClusterTree() { - checkAndRefresh(); - } - public void start(ConfigurationSource conf) throws IOException { final InnerNode initialTopology = scmBlockLocationProtocol.getNetworkTopology(); diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java index dfa3a045ea1..38dacba4a72 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java @@ -1099,11 +1099,6 @@ public UUID refetchSecretKey() { return secretKeyClient.getCurrentSecretKey().getId(); } - public InnerNode refetchClusterTree() { - scmTopologyClient.refetchClusterTree(); - return scmTopologyClient.getClusterTree(); - } - @VisibleForTesting public void startSecretManager() { try { @@ -1160,16 +1155,10 @@ public void setScmTopologyClient( public NetworkTopology getClusterMap() { InnerNode currentTree = scmTopologyClient.getClusterTree(); - InnerNode refetchedTree = refetchClusterTree(); - - if (!currentTree.equals(refetchedTree)) { - return new NetworkTopologyImpl(configuration.get( - ScmConfigKeys.OZONE_SCM_NETWORK_TOPOLOGY_SCHEMA_FILE, - ScmConfigKeys.OZONE_SCM_NETWORK_TOPOLOGY_SCHEMA_FILE_DEFAULT), - refetchedTree); - } else { - return clusterMap; - } + return new NetworkTopologyImpl(configuration.get( + ScmConfigKeys.OZONE_SCM_NETWORK_TOPOLOGY_SCHEMA_FILE, + ScmConfigKeys.OZONE_SCM_NETWORK_TOPOLOGY_SCHEMA_FILE_DEFAULT), + currentTree); } /** From c528224682c25cd8bc013b2e0583cf79ab9153e8 Mon Sep 17 00:00:00 2001 From: tanvipenumudy Date: Tue, 27 Feb 2024 17:51:08 +0530 Subject: [PATCH 19/23] Remove tests in TestKeyManagerUnit that reference ScmBlockLocationProtocol#sortDatanodes --- .../hadoop/ozone/om/TestKeyManagerUnit.java | 67 ------------------- 1 file changed, 67 deletions(-) diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerUnit.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerUnit.java index f65de7ee403..d244e2fcaa5 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerUnit.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerUnit.java @@ -650,9 +650,6 @@ public void listStatus() throws Exception { OMRequestTestUtils.addBucketToDB(volume, bucket, metadataManager); final Pipeline pipeline = MockPipeline.createPipeline(3); - final List nodes = pipeline.getNodes().stream() - .map(DatanodeDetails::getUuidString) - .collect(toList()); Set containerIDs = new HashSet<>(); List containersWithPipeline = new ArrayList<>(); @@ -702,7 +699,6 @@ public void listStatus() throws Exception { assertEquals(10, fileStatusList.size()); verify(containerClient).getContainerWithPipelineBatch(containerIDs); - verify(blockClient).sortDatanodes(nodes, client); // call list status the second time, and verify no more calls to // SCM. @@ -710,67 +706,4 @@ public void listStatus() throws Exception { null, Long.MAX_VALUE, client); verify(containerClient, times(1)).getContainerWithPipelineBatch(anySet()); } - - @ParameterizedTest - @ValueSource(strings = {"anyhost", ""}) - public void sortDatanodes(String client) throws Exception { - // GIVEN - int pipelineCount = 3; - int keysPerPipeline = 5; - OmKeyInfo[] keyInfos = new OmKeyInfo[pipelineCount * keysPerPipeline]; - List> expectedSortDatanodesInvocations = new ArrayList<>(); - Map> expectedSortedNodes = new HashMap<>(); - int ki = 0; - for (int p = 0; p < pipelineCount; p++) { - final Pipeline pipeline = MockPipeline.createPipeline(3); - final List nodes = pipeline.getNodes().stream() - .map(DatanodeDetails::getUuidString) - .collect(toList()); - expectedSortDatanodesInvocations.add(nodes); - final List sortedNodes = pipeline.getNodes().stream() - .sorted(comparing(DatanodeDetails::getUuidString)) - .collect(toList()); - expectedSortedNodes.put(pipeline, sortedNodes); - - when(blockClient.sortDatanodes(nodes, client)) - .thenReturn(sortedNodes); - - for (int i = 1; i <= keysPerPipeline; i++) { - OmKeyLocationInfo keyLocationInfo = new OmKeyLocationInfo.Builder() - .setBlockID(new BlockID(i, 1L)) - .setPipeline(pipeline) - .setOffset(0) - .setLength(256000) - .build(); - - OmKeyInfo keyInfo = new OmKeyInfo.Builder() - .setOmKeyLocationInfos(Arrays.asList( - new OmKeyLocationInfoGroup(0, emptyList()), - new OmKeyLocationInfoGroup(1, singletonList(keyLocationInfo)))) - .build(); - keyInfos[ki++] = keyInfo; - } - } - - // WHEN - keyManager.sortDatanodes(client, keyInfos); - - // THEN - // verify all key info locations got updated - for (OmKeyInfo keyInfo : keyInfos) { - OmKeyLocationInfoGroup locations = keyInfo.getLatestVersionLocations(); - assertNotNull(locations); - for (OmKeyLocationInfo locationInfo : locations.getLocationList()) { - Pipeline pipeline = locationInfo.getPipeline(); - List expectedOrder = expectedSortedNodes.get(pipeline); - assertEquals(expectedOrder, pipeline.getNodesInOrder()); - } - } - - // expect one invocation per pipeline - for (List nodes : expectedSortDatanodesInvocations) { - verify(blockClient).sortDatanodes(nodes, client); - } - } - } From db5beab3a045d35eb62212b9e62b43d6b71d3e5a Mon Sep 17 00:00:00 2001 From: tanvipenumudy Date: Tue, 27 Feb 2024 19:20:33 +0530 Subject: [PATCH 20/23] Fix checkstyle --- .../org/apache/hadoop/ozone/om/TestKeyManagerUnit.java | 7 ------- 1 file changed, 7 deletions(-) diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerUnit.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerUnit.java index d244e2fcaa5..5e2e27e0c1f 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerUnit.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerUnit.java @@ -23,12 +23,10 @@ import java.nio.file.Path; import java.time.Instant; import java.util.ArrayList; -import java.util.HashMap; import java.util.Arrays; import java.util.Collections; import java.util.HashSet; import java.util.List; -import java.util.Map; import java.util.Set; import java.util.UUID; import java.util.concurrent.atomic.AtomicLong; @@ -82,14 +80,9 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; import org.junit.jupiter.api.TestInstance; -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.ValueSource; import static com.google.common.collect.Sets.newHashSet; -import static java.util.Collections.emptyList; import static java.util.Collections.singletonList; -import static java.util.Comparator.comparing; -import static java.util.stream.Collectors.toList; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; From c3e04fd8a2f90f267857c7a9f4ca465d408916e8 Mon Sep 17 00:00:00 2001 From: tanvipenumudy Date: Wed, 28 Feb 2024 00:17:06 +0530 Subject: [PATCH 21/23] Add tests for KeyManagerImpl#sortDatanodes --- .../ozone/TestGetClusterTreeInformation.java | 2 + .../hadoop/ozone/TestOMSortDatanodes.java | 187 ++++++++++++++++++ .../hadoop/ozone/om/KeyManagerImpl.java | 30 +-- .../apache/hadoop/ozone/om/OzoneManager.java | 1 - 4 files changed, 204 insertions(+), 16 deletions(-) create mode 100644 hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestOMSortDatanodes.java diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestGetClusterTreeInformation.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestGetClusterTreeInformation.java index 3f6377e58f8..463c8b5ae5d 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestGetClusterTreeInformation.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestGetClusterTreeInformation.java @@ -24,6 +24,7 @@ import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.Timeout; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.apache.hadoop.hdds.scm.server.StorageContainerManager; @@ -40,6 +41,7 @@ * This class is to test the serialization/deserialization of cluster tree * information from SCM. */ +@Timeout(300) public class TestGetClusterTreeInformation { public static final Logger LOG = diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestOMSortDatanodes.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestOMSortDatanodes.java new file mode 100644 index 00000000000..6cdaa067342 --- /dev/null +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestOMSortDatanodes.java @@ -0,0 +1,187 @@ +/** + * 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; + +import com.google.common.collect.ImmutableMap; +import org.apache.commons.io.FileUtils; +import org.apache.hadoop.hdds.DFSConfigKeysLegacy; +import org.apache.hadoop.hdds.HddsConfigKeys; +import org.apache.hadoop.hdds.conf.OzoneConfiguration; +import org.apache.hadoop.hdds.protocol.DatanodeDetails; +import org.apache.hadoop.hdds.scm.HddsTestUtils; +import org.apache.hadoop.hdds.scm.ha.SCMHAManagerStub; +import org.apache.hadoop.hdds.scm.ha.SCMContext; +import org.apache.hadoop.hdds.scm.node.NodeManager; +import org.apache.hadoop.hdds.scm.protocol.StorageContainerLocationProtocol; +import org.apache.hadoop.hdds.scm.server.SCMConfigurator; +import org.apache.hadoop.hdds.scm.server.StorageContainerManager; +import org.apache.hadoop.net.StaticMapping; + +import org.apache.hadoop.ozone.om.KeyManagerImpl; +import org.apache.hadoop.ozone.om.OmTestManagers; +import org.apache.hadoop.ozone.om.OzoneManager; +import org.apache.ozone.test.GenericTestUtils; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.Timeout; + +import java.io.File; +import java.util.List; +import java.util.Map; +import java.util.ArrayList; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.apache.hadoop.fs.CommonConfigurationKeysPublic.NET_TOPOLOGY_NODE_SWITCH_MAPPING_IMPL_KEY; +import static org.apache.hadoop.hdds.protocol.MockDatanodeDetails.randomDatanodeDetails; +import static org.apache.hadoop.hdds.scm.net.NetConstants.ROOT_LEVEL; +import static org.mockito.Mockito.mock; + +/** + * {@link org.apache.hadoop.hdds.scm.server.TestSCMBlockProtocolServer} + * sortDatanodes tests for + * {@link org.apache.hadoop.ozone.om.KeyManagerImpl#sortDatanodes(List, String)}. + */ +@Timeout(300) +public class TestOMSortDatanodes { + + private static OzoneConfiguration config; + private static StorageContainerManager scm; + private static NodeManager nodeManager; + private static KeyManagerImpl keyManager; + private static StorageContainerLocationProtocol mockScmContainerClient; + private static OzoneManager om; + private static File dir; + private static final int NODE_COUNT = 10; + private static final Map EDGE_NODES = ImmutableMap.of( + "edge0", "/rack0", + "edge1", "/rack1" + ); + + @BeforeEach + void setup() throws Exception { + config = new OzoneConfiguration(); + dir = GenericTestUtils.getRandomizedTestDir(); + config.set(HddsConfigKeys.OZONE_METADATA_DIRS, dir.toString()); + config.set(NET_TOPOLOGY_NODE_SWITCH_MAPPING_IMPL_KEY, + StaticMapping.class.getName()); + config.set(OzoneConfigKeys.OZONE_NETWORK_TOPOLOGY_AWARE_READ_KEY, "true"); + List datanodes = new ArrayList<>(NODE_COUNT); + List nodeMapping = new ArrayList<>(NODE_COUNT); + for (int i = 0; i < NODE_COUNT; i++) { + DatanodeDetails dn = randomDatanodeDetails(); + final String rack = "/rack" + (i % 2); + nodeMapping.add(dn.getHostName() + "=" + rack); + nodeMapping.add(dn.getIpAddress() + "=" + rack); + datanodes.add(dn); + } + EDGE_NODES.forEach((n, r) -> nodeMapping.add(n + "=" + r)); + config.set(StaticMapping.KEY_HADOOP_CONFIGURED_NODE_MAPPING, + String.join(",", nodeMapping)); + + SCMConfigurator configurator = new SCMConfigurator(); + configurator.setSCMHAManager(SCMHAManagerStub.getInstance(true)); + configurator.setScmContext(SCMContext.emptyContext()); + scm = HddsTestUtils.getScm(config, configurator); + scm.start(); + scm.exitSafeMode(); + nodeManager = scm.getScmNodeManager(); + datanodes.forEach(dn -> nodeManager.register(dn, null, null)); + mockScmContainerClient = + mock(StorageContainerLocationProtocol.class); + OmTestManagers omTestManagers + = new OmTestManagers(config, scm.getBlockProtocolServer(), + mockScmContainerClient); + om = omTestManagers.getOzoneManager(); + keyManager = (KeyManagerImpl)omTestManagers.getKeyManager(); + } + + @AfterEach + public void cleanup() throws Exception { + if (scm != null) { + scm.stop(); + scm.join(); + } + if (om != null) { + om.stop(); + } + FileUtils.deleteDirectory(dir); + } + + @Test + public void sortDatanodesRelativeToDatanode() { + for (DatanodeDetails dn : nodeManager.getAllNodes()) { + assertEquals(ROOT_LEVEL + 2, dn.getLevel()); + List sorted = + keyManager.sortDatanodes(nodeManager.getAllNodes(), nodeAddress(dn)); + assertEquals(dn, sorted.get(0), + "Source node should be sorted very first"); + assertRackOrder(dn.getNetworkLocation(), sorted); + } + } + + @Test + public void sortDatanodesRelativeToNonDatanode() { + for (Map.Entry entry : EDGE_NODES.entrySet()) { + assertRackOrder(entry.getValue(), + keyManager.sortDatanodes(nodeManager.getAllNodes(), entry.getKey())); + } + } + + @Test + public void testSortDatanodes() { + List nodes = nodeManager.getAllNodes(); + + // sort normal datanodes + String client; + client = nodeManager.getAllNodes().get(0).getIpAddress(); + List datanodeDetails = + keyManager.sortDatanodes(nodes, client); + assertEquals(NODE_COUNT, datanodeDetails.size()); + + // illegal client 1 + client += "X"; + datanodeDetails = keyManager.sortDatanodes(nodes, client); + assertEquals(NODE_COUNT, datanodeDetails.size()); + + // illegal client 2 + client = "/default-rack"; + datanodeDetails = keyManager.sortDatanodes(nodes, client); + assertEquals(NODE_COUNT, datanodeDetails.size()); + } + + private static void assertRackOrder(String rack, List list) { + int size = list.size(); + for (int i = 0; i < size / 2; i++) { + assertEquals(rack, list.get(i).getNetworkLocation(), + "Nodes in the same rack should be sorted first"); + } + for (int i = size / 2; i < size; i++) { + assertNotEquals(rack, list.get(i).getNetworkLocation(), + "Nodes in the other rack should be sorted last"); + } + } + + private String nodeAddress(DatanodeDetails dn) { + boolean useHostname = config.getBoolean( + DFSConfigKeysLegacy.DFS_DATANODE_USE_DN_HOSTNAME, + DFSConfigKeysLegacy.DFS_DATANODE_USE_DN_HOSTNAME_DEFAULT); + return useHostname ? dn.getHostName() : dn.getIpAddress(); + } +} 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 5b7dd75fd4b..f9c665e9806 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 @@ -1870,8 +1870,7 @@ private FileEncryptionInfo getFileEncryptionInfo(OmBucketInfo bucketInfo) return encInfo; } - @VisibleForTesting - void sortDatanodes(String clientMachine, OmKeyInfo... keyInfos) { + private void sortDatanodes(String clientMachine, OmKeyInfo... keyInfos) { if (keyInfos != null && clientMachine != null) { Map, List> sortedPipelines = new HashMap<>(); for (OmKeyInfo keyInfo : keyInfos) { @@ -1905,6 +1904,7 @@ void sortDatanodes(String clientMachine, OmKeyInfo... keyInfos) { } } + @VisibleForTesting public List sortDatanodes(List nodes, String clientMachine) { final Node client = getClientNode(clientMachine, nodes); @@ -1928,19 +1928,6 @@ private Node getClientNode(String clientMachine, getOtherNode(clientMachine); } - public String resolveNodeLocation(String hostname) { - List hosts = Collections.singletonList(hostname); - List resolvedHosts = dnsToSwitchMapping.resolve(hosts); - if (resolvedHosts != null && !resolvedHosts.isEmpty()) { - String location = resolvedHosts.get(0); - LOG.debug("Node {} resolved to location {}", hostname, location); - return location; - } else { - LOG.debug("Node resolution did not yield any result for {}", hostname); - return null; - } - } - private Node getOtherNode(String clientMachine) { try { String clientLocation = resolveNodeLocation(clientMachine); @@ -1959,6 +1946,19 @@ private Node getOtherNode(String clientMachine) { return null; } + private String resolveNodeLocation(String hostname) { + List hosts = Collections.singletonList(hostname); + List resolvedHosts = dnsToSwitchMapping.resolve(hosts); + if (resolvedHosts != null && !resolvedHosts.isEmpty()) { + String location = resolvedHosts.get(0); + LOG.debug("Node {} resolved to location {}", hostname, location); + return location; + } else { + LOG.debug("Node resolution did not yield any result for {}", hostname); + return null; + } + } + private static List toNodeUuid(Collection nodes) { List nodeSet = new ArrayList<>(nodes.size()); for (DatanodeDetails node : nodes) { diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java index 68afd0db6ce..decd980aee4 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java @@ -1153,7 +1153,6 @@ public void setScmTopologyClient( } public NetworkTopology getClusterMap() { - InnerNode currentTree = scmTopologyClient.getClusterTree(); return new NetworkTopologyImpl(configuration.get( ScmConfigKeys.OZONE_SCM_NETWORK_TOPOLOGY_SCHEMA_FILE, From 1d1d5d184f9cc681483ed2f8f0d68826a504b912 Mon Sep 17 00:00:00 2001 From: tanvipenumudy Date: Wed, 28 Feb 2024 00:39:34 +0530 Subject: [PATCH 22/23] Fix findbugs --- .../java/org/apache/hadoop/ozone/TestOMSortDatanodes.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestOMSortDatanodes.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestOMSortDatanodes.java index 6cdaa067342..ba9abb411fc 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestOMSortDatanodes.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestOMSortDatanodes.java @@ -75,7 +75,7 @@ public class TestOMSortDatanodes { ); @BeforeEach - void setup() throws Exception { + public static void setup() throws Exception { config = new OzoneConfiguration(); dir = GenericTestUtils.getRandomizedTestDir(); config.set(HddsConfigKeys.OZONE_METADATA_DIRS, dir.toString()); @@ -113,7 +113,7 @@ void setup() throws Exception { } @AfterEach - public void cleanup() throws Exception { + public static void cleanup() throws Exception { if (scm != null) { scm.stop(); scm.join(); From 8fba5c97f0684a6607adea74cd2a248cf2a60b98 Mon Sep 17 00:00:00 2001 From: tanvipenumudy Date: Wed, 28 Feb 2024 11:11:30 +0530 Subject: [PATCH 23/23] Fix integration (ozone) test failure --- .../java/org/apache/hadoop/ozone/TestOMSortDatanodes.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestOMSortDatanodes.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestOMSortDatanodes.java index ba9abb411fc..cef872597e4 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestOMSortDatanodes.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestOMSortDatanodes.java @@ -36,8 +36,8 @@ import org.apache.hadoop.ozone.om.OmTestManagers; import org.apache.hadoop.ozone.om.OzoneManager; import org.apache.ozone.test.GenericTestUtils; -import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Timeout; @@ -74,7 +74,7 @@ public class TestOMSortDatanodes { "edge1", "/rack1" ); - @BeforeEach + @BeforeAll public static void setup() throws Exception { config = new OzoneConfiguration(); dir = GenericTestUtils.getRandomizedTestDir(); @@ -112,7 +112,7 @@ public static void setup() throws Exception { keyManager = (KeyManagerImpl)omTestManagers.getKeyManager(); } - @AfterEach + @AfterAll public static void cleanup() throws Exception { if (scm != null) { scm.stop();