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 49ae34b0f2 HDDS-10830. Replace ConcurrentHashMap with HashMap
protected by ReadWriteLock in NodeStateMap (#6654)
49ae34b0f2 is described below
commit 49ae34b0f2b5a1ff7ee292456b15a737fc762d8d
Author: Andrei Mikhalev <[email protected]>
AuthorDate: Thu May 9 16:15:17 2024 +0300
HDDS-10830. Replace ConcurrentHashMap with HashMap protected by
ReadWriteLock in NodeStateMap (#6654)
---
.../hadoop/hdds/scm/node/NodeStateManager.java | 2 +-
.../hadoop/hdds/scm/node/states/NodeStateMap.java | 61 +++++++++++++---------
2 files changed, 37 insertions(+), 26 deletions(-)
diff --git
a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/NodeStateManager.java
b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/NodeStateManager.java
index 78d7062645..4543a49b41 100644
---
a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/NodeStateManager.java
+++
b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/NodeStateManager.java
@@ -738,7 +738,7 @@ public class NodeStateManager implements Runnable,
Closeable {
*/
public synchronized void forceNodesToHealthyReadOnly() {
try {
- List<DatanodeInfo> nodes = nodeStateMap.filterNodes(null, HEALTHY);
+ List<DatanodeInfo> nodes = nodeStateMap.getDatanodeInfos(null, HEALTHY);
for (DatanodeInfo node : nodes) {
nodeStateMap.updateNodeHealthState(node.getUuid(),
HEALTHY_READONLY);
diff --git
a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/states/NodeStateMap.java
b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/states/NodeStateMap.java
index ae5a1c1fde..e8843fa21e 100644
---
a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/states/NodeStateMap.java
+++
b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/states/NodeStateMap.java
@@ -19,16 +19,18 @@
package org.apache.hadoop.hdds.scm.node.states;
import java.util.ArrayList;
+import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedList;
import java.util.List;
+import java.util.Map;
import java.util.Set;
import java.util.UUID;
-import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.locks.ReadWriteLock;
import java.util.concurrent.locks.ReentrantReadWriteLock;
import java.util.function.Predicate;
+import jakarta.annotation.Nonnull;
import org.apache.hadoop.hdds.protocol.DatanodeDetails;
import org.apache.hadoop.hdds.protocol.proto.HddsProtos.NodeOperationalState;
import org.apache.hadoop.hdds.protocol.proto.HddsProtos.NodeState;
@@ -43,27 +45,26 @@ import org.apache.hadoop.hdds.scm.node.NodeStatus;
* NodeStateManager to maintain the state. If anyone wants to change the
* state of a node they should call NodeStateManager, do not directly use
* this class.
+ * <p>
+ * Concurrency consideration:
+ * - thread-safe
*/
public class NodeStateMap {
/**
* Node id to node info map.
*/
- private final ConcurrentHashMap<UUID, DatanodeInfo> nodeMap;
+ private final Map<UUID, DatanodeInfo> nodeMap = new HashMap<>();
/**
* Node to set of containers on the node.
*/
- private final ConcurrentHashMap<UUID, Set<ContainerID>> nodeToContainer;
+ private final Map<UUID, Set<ContainerID>> nodeToContainer = new HashMap<>();
- private final ReadWriteLock lock;
+ private final ReadWriteLock lock = new ReentrantReadWriteLock();
/**
* Creates a new instance of NodeStateMap with no nodes.
*/
- public NodeStateMap() {
- lock = new ReentrantReadWriteLock();
- nodeMap = new ConcurrentHashMap<>();
- nodeToContainer = new ConcurrentHashMap<>();
- }
+ public NodeStateMap() { }
/**
* Adds a node to NodeStateMap.
@@ -92,6 +93,23 @@ public class NodeStateMap {
}
}
+ /**
+ * Removes a node from NodeStateMap.
+ *
+ * @param datanodeDetails DatanodeDetails
+ *
+ */
+ public void removeNode(DatanodeDetails datanodeDetails) {
+ lock.writeLock().lock();
+ try {
+ UUID uuid = datanodeDetails.getUuid();
+ nodeMap.remove(uuid);
+ nodeToContainer.remove(uuid);
+ } finally {
+ lock.writeLock().unlock();
+ }
+ }
+
/**
* Update a node in NodeStateMap.
*
@@ -129,7 +147,7 @@ public class NodeStateMap {
throws NodeNotFoundException {
try {
lock.writeLock().lock();
- DatanodeInfo dn = getNodeInfo(nodeId);
+ DatanodeInfo dn = getNodeInfoUnsafe(nodeId);
NodeStatus oldStatus = dn.getNodeStatus();
NodeStatus newStatus = new NodeStatus(
oldStatus.getOperationalState(), newHealth);
@@ -153,7 +171,7 @@ public class NodeStateMap {
throws NodeNotFoundException {
try {
lock.writeLock().lock();
- DatanodeInfo dn = getNodeInfo(nodeId);
+ DatanodeInfo dn = getNodeInfoUnsafe(nodeId);
NodeStatus oldStatus = dn.getNodeStatus();
NodeStatus newStatus = new NodeStatus(
newOpState, oldStatus.getHealth(), opStateExpiryEpochSeconds);
@@ -176,8 +194,7 @@ public class NodeStateMap {
public DatanodeInfo getNodeInfo(UUID uuid) throws NodeNotFoundException {
lock.readLock().lock();
try {
- checkIfNodeExist(uuid);
- return nodeMap.get(uuid);
+ return getNodeInfoUnsafe(uuid);
} finally {
lock.readLock().unlock();
}
@@ -413,7 +430,7 @@ public class NodeStateMap {
* @param health
* @return List of DatanodeInfo objects matching the passed state
*/
- public List<DatanodeInfo> filterNodes(
+ private List<DatanodeInfo> filterNodes(
NodeOperationalState opState, NodeState health) {
if (opState != null && health != null) {
return filterNodes(matching(new NodeStatus(opState, health)));
@@ -445,6 +462,11 @@ public class NodeStateMap {
return result;
}
+ private @Nonnull DatanodeInfo getNodeInfoUnsafe(@Nonnull UUID uuid) throws
NodeNotFoundException {
+ checkIfNodeExist(uuid);
+ return nodeMap.get(uuid);
+ }
+
private static Predicate<DatanodeInfo> matching(NodeStatus status) {
return dn -> status.equals(dn.getNodeStatus());
}
@@ -456,15 +478,4 @@ public class NodeStateMap {
private static Predicate<DatanodeInfo> matching(NodeState health) {
return dn -> health.equals(dn.getNodeStatus().getHealth());
}
-
- public void removeNode(DatanodeDetails datanodeDetails) {
- lock.writeLock().lock();
- try {
- UUID uuid = datanodeDetails.getUuid();
- nodeMap.remove(uuid);
- nodeToContainer.remove(uuid);
- } finally {
- lock.writeLock().unlock();
- }
- }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
