[iotdb] branch master updated: [IOTDB-2476] Fix concurrent bug during entityMNode replacement (#4977)

[qi...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

qiaojialin pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/iotdb.git


The following commit(s) were added to refs/heads/master by this push:
     new 68cdf81  [IOTDB-2476] Fix concurrent bug during entityMNode replacement (#4977)
68cdf81 is described below

commit 68cdf81962c43c1b79e522b15b5fcdc5ae23dc4b
Author: Marcos_Zyk <38...@users.noreply.github.com>
AuthorDate: Wed Jan 26 13:37:29 2022 +0800

    [IOTDB-2476] Fix concurrent bug during entityMNode replacement (#4977)
---
 .../iotdb/db/metadata/mnode/InternalMNode.java     | 10 ++++++----
 .../apache/iotdb/db/metadata/mnode/MNodeTest.java  | 23 ++++++++--------------
 2 files changed, 14 insertions(+), 19 deletions(-)

diff --git a/server/src/main/java/org/apache/iotdb/db/metadata/mnode/InternalMNode.java b/server/src/main/java/org/apache/iotdb/db/metadata/mnode/InternalMNode.java
index 84677e5..8e3ee51 100644
--- a/server/src/main/java/org/apache/iotdb/db/metadata/mnode/InternalMNode.java
+++ b/server/src/main/java/org/apache/iotdb/db/metadata/mnode/InternalMNode.java
@@ -135,13 +135,16 @@ public class InternalMNode extends MNode {
   }
 
   /**
-   * replace a child of this mnode
+   * Replace a child of this mnode. New child's name must be the same as old child's name.
    *
    * @param oldChildName measurement name
    * @param newChildNode new child node
    */
   @Override
-  public void replaceChild(String oldChildName, IMNode newChildNode) {
+  public synchronized void replaceChild(String oldChildName, IMNode newChildNode) {
+    if (!oldChildName.equals(newChildNode.getName())) {
+      throw new RuntimeException("New child's name must be the same as old child's name!");
+    }
     IMNode oldChildNode = this.getChild(oldChildName);
     if (oldChildNode == null) {
       return;
@@ -170,8 +173,7 @@ public class InternalMNode extends MNode {
 
     newChildNode.setParent(this);
 
-    this.deleteChild(oldChildName);
-    this.addChild(newChildNode.getName(), newChildNode);
+    children.replace(oldChildName, newChildNode);
   }
 
   @Override
diff --git a/server/src/test/java/org/apache/iotdb/db/metadata/mnode/MNodeTest.java b/server/src/test/java/org/apache/iotdb/db/metadata/mnode/MNodeTest.java
index b66720f..af28434 100644
--- a/server/src/test/java/org/apache/iotdb/db/metadata/mnode/MNodeTest.java
+++ b/server/src/test/java/org/apache/iotdb/db/metadata/mnode/MNodeTest.java
@@ -27,9 +27,8 @@ import org.junit.Test;
 import java.util.List;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
-import java.util.concurrent.TimeUnit;
 
-import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.*;
 
 public class MNodeTest {
   private static ExecutorService service;
@@ -43,8 +42,7 @@ public class MNodeTest {
   }
 
   @Test
-  public void testReplaceChild() throws InterruptedException {
-    // after replacing a with c, the timeseries root.a.b becomes root.c.b
+  public void testReplaceChild() {
     InternalMNode rootNode = new InternalMNode(null, "root");
 
     IEntityMNode aNode = new EntityMNode(rootNode, "a");
@@ -55,19 +53,14 @@ public class MNodeTest {
     aNode.addChild(bNode.getName(), bNode);
     aNode.addAlias("aliasOfb", bNode);
 
-    for (int i = 0; i < 500; i++) {
-      service.submit(
-          new Thread(() -> rootNode.replaceChild(aNode.getName(), new EntityMNode(null, "c"))));
-    }
-
-    if (!service.isShutdown()) {
-      service.shutdown();
-      service.awaitTermination(30, TimeUnit.SECONDS);
-    }
+    IEntityMNode newANode = new EntityMNode(null, "a");
+    rootNode.replaceChild(aNode.getName(), newANode);
 
     List<String> multiFullPaths = MetaUtils.getMultiFullPaths(rootNode);
-    assertEquals("root.c.b", multiFullPaths.get(0));
-    assertEquals("root.c.b", rootNode.getChild("c").getChild("aliasOfb").getFullPath());
+    assertEquals("root.a.b", multiFullPaths.get(0));
+    assertEquals("root.a.b", rootNode.getChild("a").getChild("aliasOfb").getFullPath());
+    assertNotSame(aNode, rootNode.getChild("a"));
+    assertSame(newANode, rootNode.getChild("a"));
   }
 
   @Test