[iotdb] branch master updated: Fix concurrent failure of testInsertMultiTabletPlanParallel (#5767)

[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 f40ae52eb5 Fix concurrent failure of testInsertMultiTabletPlanParallel (#5767)
f40ae52eb5 is described below

commit f40ae52eb56f5399db2afd312eed8d6c6e368c22
Author: Marcos_Zyk <38...@users.noreply.github.com>
AuthorDate: Tue May 3 15:56:04 2022 +0800

    Fix concurrent failure of testInsertMultiTabletPlanParallel (#5767)
---
 .../iotdb/db/localconfignode/LocalConfigNode.java  | 25 +++++++++++++++++++++-
 .../iotdb/db/metadata/LocalSchemaProcessor.java    | 16 ++++++--------
 .../db/metadata/schemaregion/SchemaEngine.java     | 16 +++++++++-----
 3 files changed, 42 insertions(+), 15 deletions(-)

diff --git a/server/src/main/java/org/apache/iotdb/db/localconfignode/LocalConfigNode.java b/server/src/main/java/org/apache/iotdb/db/localconfignode/LocalConfigNode.java
index 96b0fedb11..de1957f472 100644
--- a/server/src/main/java/org/apache/iotdb/db/localconfignode/LocalConfigNode.java
+++ b/server/src/main/java/org/apache/iotdb/db/localconfignode/LocalConfigNode.java
@@ -539,7 +539,30 @@ public class LocalConfigNode {
    */
   public SchemaRegionId getBelongedSchemaRegionId(PartialPath path) throws MetadataException {
     PartialPath storageGroup = storageGroupSchemaManager.getBelongedStorageGroup(path);
-    return schemaPartitionTable.getSchemaRegionId(storageGroup, path);
+    SchemaRegionId schemaRegionId = schemaPartitionTable.getSchemaRegionId(storageGroup, path);
+    // Since the creation of storageGroup, schemaRegionId and schemaRegion is not atomic or locked,
+    // any access concurrent with this creation may get null.
+    // Thread A: create sg, allocate schemaRegionId, create schemaRegion
+    // Thread B: access sg, access partitionTable to get schemaRegionId, access schemaEngine to get
+    // schemaRegion
+    // When A and B are running concurrently, B may get null while getting schemaRegionId or
+    // schemaRegion. This means B must run after A ends.
+    // To avoid this exception, please invoke getBelongedSchemaRegionIdWithAutoCreate according to
+    // the scenario.
+    if (schemaRegionId == null) {
+      throw new MetadataException(
+          String.format(
+              "Storage group %s has not been prepared well. Schema region for %s has not been allocated or is not initialized.",
+              storageGroup, path));
+    }
+    ISchemaRegion schemaRegion = schemaEngine.getSchemaRegion(schemaRegionId);
+    if (schemaRegion == null) {
+      throw new MetadataException(
+          String.format(
+              "Storage group [%s] has not been prepared well. Schema region [%s] is not initialized.",
+              storageGroup, schemaRegionId));
+    }
+    return schemaRegionId;
   }
 
   // This interface involves storage group and schema region auto creation
diff --git a/server/src/main/java/org/apache/iotdb/db/metadata/LocalSchemaProcessor.java b/server/src/main/java/org/apache/iotdb/db/metadata/LocalSchemaProcessor.java
index a34b71633e..26f5e3e5ec 100644
--- a/server/src/main/java/org/apache/iotdb/db/metadata/LocalSchemaProcessor.java
+++ b/server/src/main/java/org/apache/iotdb/db/metadata/LocalSchemaProcessor.java
@@ -1259,16 +1259,14 @@ public class LocalSchemaProcessor {
   @SuppressWarnings("squid:S3776") // Suppress high Cognitive Complexity warning
   public IMNode getSeriesSchemasAndReadLockDevice(InsertPlan plan)
       throws MetadataException, IOException {
-    try {
-      return getBelongedSchemaRegion(plan.getDevicePath()).getSeriesSchemasAndReadLockDevice(plan);
-    } catch (StorageGroupNotSetException e) {
-      if (config.isAutoCreateSchemaEnabled()) {
-        return getBelongedSchemaRegionWithAutoCreate(plan.getDevicePath())
-            .getSeriesSchemasAndReadLockDevice(plan);
-      } else {
-        throw e;
-      }
+    ISchemaRegion schemaRegion;
+    if (config.isAutoCreateSchemaEnabled()) {
+      schemaRegion = getBelongedSchemaRegionWithAutoCreate(plan.getDevicePath());
+    } else {
+      schemaRegion = getBelongedSchemaRegion(plan.getDevicePath());
     }
+
+    return schemaRegion.getSeriesSchemasAndReadLockDevice(plan);
   }
 
   // endregion
diff --git a/server/src/main/java/org/apache/iotdb/db/metadata/schemaregion/SchemaEngine.java b/server/src/main/java/org/apache/iotdb/db/metadata/schemaregion/SchemaEngine.java
index fce9c38aeb..22b09f8a00 100644
--- a/server/src/main/java/org/apache/iotdb/db/metadata/schemaregion/SchemaEngine.java
+++ b/server/src/main/java/org/apache/iotdb/db/metadata/schemaregion/SchemaEngine.java
@@ -163,11 +163,17 @@ public class SchemaEngine {
       PartialPath storageGroup, SchemaRegionId schemaRegionId) throws MetadataException {
     ISchemaRegion schemaRegion = schemaRegionMap.get(schemaRegionId);
     if (schemaRegion != null) {
-      throw new MetadataException(
-          String.format(
-              "SchemaRegion [%s] is duplicated between [%s] and [%s], "
-                  + "and the former one has been recovered.",
-              schemaRegionId, schemaRegion.getStorageGroupFullPath(), storageGroup.getFullPath()));
+      if (schemaRegion.getStorageGroupFullPath().equals(storageGroup.getFullPath())) {
+        return;
+      } else {
+        throw new MetadataException(
+            String.format(
+                "SchemaRegion [%s] is duplicated between [%s] and [%s], "
+                    + "and the former one has been recovered.",
+                schemaRegionId,
+                schemaRegion.getStorageGroupFullPath(),
+                storageGroup.getFullPath()));
+      }
     }
     schemaRegionMap.put(
         schemaRegionId, createSchemaRegionWithoutExistenceCheck(storageGroup, schemaRegionId));