You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ozone.apache.org by na...@apache.org on 2021/01/12 17:00:11 UTC
[ozone] branch master updated: HDDS-4673. NodeStateMap leaks
internal representation of container sets. (#1782)
This is an automated email from the ASF dual-hosted git repository.
nanda 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 a8f4d52 HDDS-4673. NodeStateMap leaks internal representation of container sets. (#1782)
a8f4d52 is described below
commit a8f4d52727d46fbdacc54955939e3e1dbee6118c
Author: Elek, Márton <el...@users.noreply.github.com>
AuthorDate: Tue Jan 12 17:59:53 2021 +0100
HDDS-4673. NodeStateMap leaks internal representation of container sets. (#1782)
---
.../hadoop/hdds/scm/node/states/NodeStateMap.java | 22 +++++---
.../hdds/scm/node/states/TestNodeStateMap.java | 65 +++++++++++++++++++---
2 files changed, 71 insertions(+), 16 deletions(-)
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 d44f6b7..9b6e0e0 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
@@ -18,19 +18,24 @@
package org.apache.hadoop.hdds.scm.node.states;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.List;
+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.stream.Collectors;
+
import org.apache.hadoop.hdds.protocol.DatanodeDetails;
-import org.apache.hadoop.hdds.protocol.proto.HddsProtos.NodeState;
import org.apache.hadoop.hdds.protocol.proto.HddsProtos.NodeOperationalState;
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos.NodeState;
import org.apache.hadoop.hdds.scm.container.ContainerID;
import org.apache.hadoop.hdds.scm.node.DatanodeInfo;
import org.apache.hadoop.hdds.scm.node.NodeStatus;
-import java.util.*;
-import java.util.concurrent.ConcurrentHashMap;
-import java.util.concurrent.locks.ReadWriteLock;
-import java.util.concurrent.locks.ReentrantReadWriteLock;
-import java.util.stream.Collectors;
-
/**
* Maintains the state of datanodes in SCM. This class should only be used by
* NodeStateManager to maintain the state. If anyone wants to change the
@@ -334,7 +339,8 @@ public class NodeStateMap {
lock.readLock().lock();
try {
checkIfNodeExist(uuid);
- return Collections.unmodifiableSet(nodeToContainer.get(uuid));
+ return Collections
+ .unmodifiableSet(new HashSet<>(nodeToContainer.get(uuid)));
} finally {
lock.readLock().unlock();
}
diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/states/TestNodeStateMap.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/states/TestNodeStateMap.java
index 25601f7..5954f08 100644
--- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/states/TestNodeStateMap.java
+++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/states/TestNodeStateMap.java
@@ -18,19 +18,22 @@
package org.apache.hadoop.hdds.scm.node.states;
+import java.util.List;
+import java.util.UUID;
+import java.util.concurrent.CountDownLatch;
+
import org.apache.hadoop.hdds.protocol.DatanodeDetails;
+import org.apache.hadoop.hdds.protocol.MockDatanodeDetails;
import org.apache.hadoop.hdds.protocol.proto.HddsProtos.NodeOperationalState;
import org.apache.hadoop.hdds.protocol.proto.HddsProtos.NodeState;
+import org.apache.hadoop.hdds.scm.container.ContainerID;
import org.apache.hadoop.hdds.scm.node.NodeStatus;
+
+import static junit.framework.TestCase.assertEquals;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
-import java.util.List;
-import java.util.UUID;
-
-import static junit.framework.TestCase.assertEquals;
-
/**
* Class to test the NodeStateMap class, which is an internal class used by
* NodeStateManager.
@@ -119,15 +122,61 @@ public class TestNodeStateMap {
map.getNodeCount(NodeOperationalState.DECOMMISSIONING, null));
}
- private void addNodeWithState(DatanodeDetails dn,
- NodeOperationalState opState, NodeState health)
+ /**
+ * Test if container list is iterable even if it's modified from other thread.
+ */
+ @Test
+ public void testConcurrency() throws Exception {
+ NodeStateMap nodeStateMap = new NodeStateMap();
+
+ final DatanodeDetails datanodeDetails =
+ MockDatanodeDetails.randomDatanodeDetails();
+
+ nodeStateMap.addNode(datanodeDetails, NodeStatus.inServiceHealthy());
+
+ UUID dnUuid = datanodeDetails.getUuid();
+
+ nodeStateMap.addContainer(dnUuid, new ContainerID(1L));
+ nodeStateMap.addContainer(dnUuid, new ContainerID(2L));
+ nodeStateMap.addContainer(dnUuid, new ContainerID(3L));
+
+ CountDownLatch elementRemoved = new CountDownLatch(1);
+ CountDownLatch loopStarted = new CountDownLatch(1);
+
+ new Thread(() -> {
+ try {
+ loopStarted.await();
+ nodeStateMap.removeContainer(dnUuid, new ContainerID(1L));
+ elementRemoved.countDown();
+ } catch (Exception e) {
+ e.printStackTrace();
+ }
+
+ }).start();
+
+ boolean first = true;
+ for (ContainerID key : nodeStateMap.getContainers(dnUuid)) {
+ if (first) {
+ loopStarted.countDown();
+ elementRemoved.await();
+ }
+ first = false;
+ System.out.println(key);
+ }
+ }
+
+ private void addNodeWithState(
+ DatanodeDetails dn,
+ NodeOperationalState opState, NodeState health
+ )
throws NodeAlreadyExistsException {
NodeStatus status = new NodeStatus(opState, health);
map.addNode(dn, status);
}
private void addRandomNodeWithState(
- NodeOperationalState opState, NodeState health)
+ NodeOperationalState opState, NodeState health
+ )
throws NodeAlreadyExistsException {
DatanodeDetails dn = generateDatanode();
addNodeWithState(dn, opState, health);
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@ozone.apache.org
For additional commands, e-mail: commits-help@ozone.apache.org