This is an automated email from the ASF dual-hosted git repository.
adoroszlai pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ozone.git
The following commit(s) were added to refs/heads/master by this push:
new f7a421b27b HDDS-10446. Refactor Node2ObjectsMap, Node2PipelineMap,
Node2ContainerMap (#6303)
f7a421b27b is described below
commit f7a421b27bde7cce83ba18a35b0658b9ab106ba5
Author: Andrei Mikhalev <[email protected]>
AuthorDate: Tue Mar 5 12:32:30 2024 +0300
HDDS-10446. Refactor Node2ObjectsMap, Node2PipelineMap, Node2ContainerMap
(#6303)
---
.../hdds/scm/node/states/Node2ContainerMap.java | 92 -------------------
.../hdds/scm/node/states/Node2PipelineMap.java | 28 ++++--
.../hadoop/hdds/scm/container/MockNodeManager.java | 1 -
.../hdds/scm/container/Node2ContainerMap.java} | 101 +++++++++++----------
.../TestNode2ContainerMap.java | 6 +-
5 files changed, 72 insertions(+), 156 deletions(-)
diff --git
a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/states/Node2ContainerMap.java
b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/states/Node2ContainerMap.java
deleted file mode 100644
index c0f46f15fe..0000000000
---
a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/states/Node2ContainerMap.java
+++ /dev/null
@@ -1,92 +0,0 @@
-/*
- * 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.node.states;
-
-import java.util.HashSet;
-import java.util.Set;
-import java.util.UUID;
-
-import org.apache.hadoop.hdds.scm.container.ContainerID;
-import org.apache.hadoop.hdds.scm.exceptions.SCMException;
-
-import com.google.common.annotations.VisibleForTesting;
-import com.google.common.base.Preconditions;
-import static org.apache.hadoop.hdds.scm.exceptions.SCMException.ResultCodes
- .NO_SUCH_DATANODE;
-
-/**
- * This data structure maintains the list of containers that is on a datanode.
- * This information is built from the DN container reports.
- */
-public class Node2ContainerMap extends Node2ObjectsMap<ContainerID> {
-
- /**
- * Constructs a Node2ContainerMap Object.
- */
- public Node2ContainerMap() {
- super();
- }
-
- /**
- * Returns null if there no containers associated with this datanode ID.
- *
- * @param datanode - UUID
- * @return Set of containers or Null.
- */
- public Set<ContainerID> getContainers(UUID datanode) {
- return getObjects(datanode);
- }
-
- /**
- * Insert a new datanode into Node2Container Map.
- *
- * @param datanodeID -- Datanode UUID
- * @param containerIDs - List of ContainerIDs.
- */
- @Override
- public void insertNewDatanode(UUID datanodeID, Set<ContainerID> containerIDs)
- throws SCMException {
- super.insertNewDatanode(datanodeID, containerIDs);
- }
-
- /**
- * Updates the Container list of an existing DN.
- *
- * @param datanodeID - UUID of DN.
- * @param containers - Set of Containers tht is present on DN.
- * @throws SCMException - if we don't know about this datanode, for new DN
- * use addDatanodeInContainerMap.
- */
- public void setContainersForDatanode(UUID datanodeID,
- Set<ContainerID> containers) throws SCMException {
- Preconditions.checkNotNull(datanodeID);
- Preconditions.checkNotNull(containers);
- if (dn2ObjectMap
- .computeIfPresent(datanodeID, (k, v) -> new HashSet<>(containers))
- == null) {
- throw new SCMException("No such datanode", NO_SUCH_DATANODE);
- }
- }
-
- @VisibleForTesting
- @Override
- public int size() {
- return dn2ObjectMap.size();
- }
-}
diff --git
a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/states/Node2PipelineMap.java
b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/states/Node2PipelineMap.java
index 6533cb8076..35107829f8 100644
---
a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/states/Node2PipelineMap.java
+++
b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/states/Node2PipelineMap.java
@@ -18,10 +18,14 @@
package org.apache.hadoop.hdds.scm.node.states;
+import jakarta.annotation.Nonnull;
import org.apache.hadoop.hdds.protocol.DatanodeDetails;
import org.apache.hadoop.hdds.scm.pipeline.Pipeline;
import org.apache.hadoop.hdds.scm.pipeline.PipelineID;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Map;
import java.util.Set;
import java.util.UUID;
import java.util.concurrent.ConcurrentHashMap;
@@ -34,11 +38,13 @@ import java.util.concurrent.ConcurrentHashMap;
* <p>TODO: this information needs to be regenerated from pipeline reports
* on SCM restart
*/
-public class Node2PipelineMap extends Node2ObjectsMap<PipelineID> {
+public class Node2PipelineMap {
+ private final Map<UUID, Set<PipelineID>> dn2PipelineMap = new
ConcurrentHashMap<>();
- /** Constructs a Node2PipelineMap Object. */
+ /**
+ * Constructs a Node2PipelineMap Object.
+ */
public Node2PipelineMap() {
- super();
}
/**
@@ -47,17 +53,19 @@ public class Node2PipelineMap extends
Node2ObjectsMap<PipelineID> {
* @param datanode - UUID
* @return Set of pipelines or Null.
*/
- public Set<PipelineID> getPipelines(UUID datanode) {
- return getObjects(datanode);
+ public Set<PipelineID> getPipelines(@Nonnull UUID datanode) {
+ final Set<PipelineID> s = dn2PipelineMap.get(datanode);
+ return s != null ? new HashSet<>(s) : Collections.emptySet();
}
/**
* Return 0 if there are no pipelines associated with this datanode ID.
+ *
* @param datanode - UUID
* @return Number of pipelines or 0.
*/
public int getPipelinesCount(UUID datanode) {
- return getObjects(datanode).size();
+ return getPipelines(datanode).size();
}
/**
@@ -65,18 +73,18 @@ public class Node2PipelineMap extends
Node2ObjectsMap<PipelineID> {
*
* @param pipeline Pipeline to be added
*/
- public synchronized void addPipeline(Pipeline pipeline) {
+ public void addPipeline(Pipeline pipeline) {
for (DatanodeDetails details : pipeline.getNodes()) {
UUID dnId = details.getUuid();
- dn2ObjectMap.computeIfAbsent(dnId, k -> ConcurrentHashMap.newKeySet())
+ dn2PipelineMap.computeIfAbsent(dnId, k -> ConcurrentHashMap.newKeySet())
.add(pipeline.getId());
}
}
- public synchronized void removePipeline(Pipeline pipeline) {
+ public void removePipeline(Pipeline pipeline) {
for (DatanodeDetails details : pipeline.getNodes()) {
UUID dnId = details.getUuid();
- dn2ObjectMap.computeIfPresent(dnId,
+ dn2PipelineMap.computeIfPresent(dnId,
(k, v) -> {
v.remove(pipeline.getId());
return v;
diff --git
a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/MockNodeManager.java
b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/MockNodeManager.java
index 84f3684ab7..21c3f1c9a8 100644
---
a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/MockNodeManager.java
+++
b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/MockNodeManager.java
@@ -42,7 +42,6 @@ import org.apache.hadoop.hdds.scm.node.DatanodeInfo;
import org.apache.hadoop.hdds.scm.node.DatanodeUsageInfo;
import org.apache.hadoop.hdds.scm.node.NodeManager;
import org.apache.hadoop.hdds.scm.node.NodeStatus;
-import org.apache.hadoop.hdds.scm.node.states.Node2ContainerMap;
import org.apache.hadoop.hdds.scm.node.states.Node2PipelineMap;
import org.apache.hadoop.hdds.scm.node.states.NodeNotFoundException;
import org.apache.hadoop.hdds.scm.pipeline.Pipeline;
diff --git
a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/states/Node2ObjectsMap.java
b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/Node2ContainerMap.java
similarity index 63%
rename from
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/states/Node2ObjectsMap.java
rename to
hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/Node2ContainerMap.java
index 5269a7aaeb..507eb75c5d 100644
---
a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/states/Node2ObjectsMap.java
+++
b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/Node2ContainerMap.java
@@ -16,37 +16,47 @@
*
*/
-package org.apache.hadoop.hdds.scm.node.states;
+package org.apache.hadoop.hdds.scm.container;
-import com.google.common.annotations.VisibleForTesting;
-import com.google.common.base.Preconditions;
+import jakarta.annotation.Nonnull;
import org.apache.hadoop.hdds.scm.exceptions.SCMException;
+import org.apache.hadoop.hdds.scm.node.states.ReportResult;
-import java.util.UUID;
-import java.util.Set;
+import java.util.Collections;
+import java.util.HashSet;
import java.util.Map;
+import java.util.Set;
import java.util.TreeSet;
-import java.util.HashSet;
-import java.util.Collections;
-
+import java.util.UUID;
import java.util.concurrent.ConcurrentHashMap;
import static
org.apache.hadoop.hdds.scm.exceptions.SCMException.ResultCodes.DUPLICATE_DATANODE;
+import static
org.apache.hadoop.hdds.scm.exceptions.SCMException.ResultCodes.NO_SUCH_DATANODE;
/**
* This data structure maintains the list of containers that is on a datanode.
* This information is built from the DN container reports.
*/
-public class Node2ObjectsMap<T> {
+class Node2ContainerMap {
+ private final Map<UUID, Set<ContainerID>> dn2ContainerMap = new
ConcurrentHashMap<>();
- @SuppressWarnings("visibilitymodifier")
- protected final Map<UUID, Set<T>> dn2ObjectMap;
/**
* Constructs a Node2ContainerMap Object.
*/
- public Node2ObjectsMap() {
- dn2ObjectMap = new ConcurrentHashMap<>();
+ Node2ContainerMap() {
+ super();
+ }
+
+ /**
+ * Returns null if there no containers associated with this datanode ID.
+ *
+ * @param datanode - UUID
+ * @return Set of containers or Null.
+ */
+ public @Nonnull Set<ContainerID> getContainers(@Nonnull UUID datanode) {
+ final Set<ContainerID> s = dn2ContainerMap.get(datanode);
+ return s != null ? new HashSet<>(s) : Collections.emptySet();
}
/**
@@ -56,9 +66,8 @@ public class Node2ObjectsMap<T> {
* @param datanodeID - UUID of the Datanode.
* @return True if this is tracked, false if this map does not know about it.
*/
- public boolean isKnownDatanode(UUID datanodeID) {
- Preconditions.checkNotNull(datanodeID);
- return dn2ObjectMap.containsKey(datanodeID);
+ public boolean isKnownDatanode(@Nonnull UUID datanodeID) {
+ return dn2ContainerMap.containsKey(datanodeID);
}
/**
@@ -67,15 +76,10 @@ public class Node2ObjectsMap<T> {
* @param datanodeID -- Datanode UUID
* @param containerIDs - List of ContainerIDs.
*/
- @VisibleForTesting
- public void insertNewDatanode(UUID datanodeID, Set<T> containerIDs)
+ public void insertNewDatanode(@Nonnull UUID datanodeID, @Nonnull
Set<ContainerID> containerIDs)
throws SCMException {
- Preconditions.checkNotNull(containerIDs);
- Preconditions.checkNotNull(datanodeID);
- if (dn2ObjectMap.putIfAbsent(datanodeID, new HashSet<>(containerIDs))
- != null) {
- throw new SCMException("Node already exists in the map",
- DUPLICATE_DATANODE);
+ if (dn2ContainerMap.putIfAbsent(datanodeID, new HashSet<>(containerIDs))
!= null) {
+ throw new SCMException("Node already exists in the map",
DUPLICATE_DATANODE);
}
}
@@ -84,32 +88,15 @@ public class Node2ObjectsMap<T> {
*
* @param datanodeID - Datanode ID.
*/
- @VisibleForTesting
- public void removeDatanode(UUID datanodeID) {
- Preconditions.checkNotNull(datanodeID);
- dn2ObjectMap.computeIfPresent(datanodeID, (k, v) -> null);
+ public void removeDatanode(@Nonnull UUID datanodeID) {
+ dn2ContainerMap.computeIfPresent(datanodeID, (k, v) -> null);
}
- /**
- * Returns null if there no containers associated with this datanode ID.
- *
- * @param datanode - UUID
- * @return Set of containers or Null.
- */
- Set<T> getObjects(UUID datanode) {
- Preconditions.checkNotNull(datanode);
- final Set<T> s = dn2ObjectMap.get(datanode);
- return s != null ? new HashSet<>(s) : Collections.emptySet();
- }
-
- public ReportResult.ReportResultBuilder<T> newBuilder() {
+ public @Nonnull ReportResult.ReportResultBuilder<ContainerID> newBuilder() {
return new ReportResult.ReportResultBuilder<>();
}
- public ReportResult<T> processReport(UUID datanodeID, Set<T> objects) {
- Preconditions.checkNotNull(datanodeID);
- Preconditions.checkNotNull(objects);
-
+ public @Nonnull ReportResult<ContainerID> processReport(@Nonnull UUID
datanodeID, @Nonnull Set<ContainerID> objects) {
if (!isKnownDatanode(datanodeID)) {
return newBuilder()
.setStatus(ReportResult.ReportStatus.NEW_DATANODE_FOUND)
@@ -118,11 +105,11 @@ public class Node2ObjectsMap<T> {
}
// Conditions like Zero length containers should be handled by removeAll.
- Set<T> currentSet = dn2ObjectMap.get(datanodeID);
- TreeSet<T> newObjects = new TreeSet<>(objects);
+ Set<ContainerID> currentSet = dn2ContainerMap.get(datanodeID);
+ TreeSet<ContainerID> newObjects = new TreeSet<>(objects);
newObjects.removeAll(currentSet);
- TreeSet<T> missingObjects = new TreeSet<>(currentSet);
+ TreeSet<ContainerID> missingObjects = new TreeSet<>(currentSet);
missingObjects.removeAll(objects);
if (newObjects.isEmpty() && missingObjects.isEmpty()) {
@@ -159,8 +146,22 @@ public class Node2ObjectsMap<T> {
.build();
}
- @VisibleForTesting
+ /**
+ * Updates the Container list of an existing DN.
+ *
+ * @param datanodeID - UUID of DN.
+ * @param containers - Set of Containers tht is present on DN.
+ * @throws SCMException - if we don't know about this datanode, for new DN
+ * use addDatanodeInContainerMap.
+ */
+ public void setContainersForDatanode(@Nonnull UUID datanodeID, @Nonnull
Set<ContainerID> containers)
+ throws SCMException {
+ if (dn2ContainerMap.computeIfPresent(datanodeID, (k, v) -> new
HashSet<>(containers)) == null) {
+ throw new SCMException("No such datanode", NO_SUCH_DATANODE);
+ }
+ }
+
public int size() {
- return dn2ObjectMap.size();
+ return dn2ContainerMap.size();
}
}
diff --git
a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/states/TestNode2ContainerMap.java
b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/TestNode2ContainerMap.java
similarity index 99%
rename from
hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/states/TestNode2ContainerMap.java
rename to
hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/TestNode2ContainerMap.java
index 0aab0aeca8..92e0a2c494 100644
---
a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/states/TestNode2ContainerMap.java
+++
b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/TestNode2ContainerMap.java
@@ -17,10 +17,10 @@
*
*/
-package org.apache.hadoop.hdds.scm.node.states;
+package org.apache.hadoop.hdds.scm.container;
-import org.apache.hadoop.hdds.scm.container.ContainerID;
import org.apache.hadoop.hdds.scm.exceptions.SCMException;
+import org.apache.hadoop.hdds.scm.node.states.ReportResult;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
@@ -32,9 +32,9 @@ import java.util.UUID;
import java.util.concurrent.ConcurrentHashMap;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
-import static org.junit.jupiter.api.Assertions.assertFalse;
/**
* Test classes for Node2ContainerMap.
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]