You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@iotdb.apache.org by GitBox <gi...@apache.org> on 2021/07/23 03:48:09 UTC

[GitHub] [iotdb] LebronAl commented on a change in pull request #3269: [IOTDB-1292] Versioned MTree

LebronAl commented on a change in pull request #3269:
URL: https://github.com/apache/iotdb/pull/3269#discussion_r675286745



##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/metadata/CMManager.java
##########
@@ -732,6 +746,15 @@ public boolean createTimeseries(InsertPlan insertPlan)
       logger.error("Failed to infer storage group from deviceId {}", deviceId);
       return false;
     }
+
+    Pair<Long, Long> sgNodeVersion;
+    try {
+      sgNodeVersion = getSgNodeMajorVersion(storageGroupName);
+    } catch (MetadataException e) {

Review comment:
       StorageGroupNotSetException

##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/metadata/CMManager.java
##########
@@ -744,9 +768,27 @@ public boolean createTimeseries(InsertPlan insertPlan)
     if (unregisteredSeriesList.isEmpty()) {
       return true;
     }
-    logger.debug("Unregisterd series of {} are {}", seriesList, unregisteredSeriesList);
+    logger.debug("Unregistered series of {} are {}", seriesList, unregisteredSeriesList);
 
-    return createTimeseries(unregisteredSeriesList, seriesList, insertPlan);
+    return createTimeseries(unregisteredSeriesList, seriesList, insertPlan, sgNodeVersion);
+  }
+
+  /**
+   * @param partialPath the storage group's partialPath
+   * @return the major version and the minor version of the storage group node
+   */
+  private Pair<Long, Long> getSgNodeMajorVersion(PartialPath partialPath) throws MetadataException {
+    StorageGroupMNode node;
+    try {
+      node = getStorageGroupNodeByPath(partialPath);
+    } catch (MetadataException e) {
+      syncMetaLeader();
+      node = getStorageGroupNodeByPath(partialPath);
+    }
+    if (node == null) {

Review comment:
       same, seems redundant

##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/metadata/CMManager.java
##########
@@ -744,9 +767,27 @@ public boolean createTimeseries(InsertPlan insertPlan)
     if (unregisteredSeriesList.isEmpty()) {
       return true;
     }
-    logger.debug("Unregisterd series of {} are {}", seriesList, unregisteredSeriesList);
+    logger.debug("Unregistered series of {} are {}", seriesList, unregisteredSeriesList);
 
-    return createTimeseries(unregisteredSeriesList, seriesList, insertPlan);
+    return createTimeseries(unregisteredSeriesList, seriesList, insertPlan, sgNodeVersion);
+  }
+
+  /**
+   * @param partialPath the storage group's partialPath
+   * @return the major version and the minor version of the storage group node
+   */
+  private Pair<Long, Long> getSgNodeMajorVersion(PartialPath partialPath) throws MetadataException {

Review comment:
       change function name as we not only return `MajorVersion`? 

##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/metadata/CMManager.java
##########
@@ -1827,4 +1873,68 @@ public PartialPath getStorageGroupPath(PartialPath path) throws StorageGroupNotS
       return super.getStorageGroupPath(path);
     }
   }
+
+  @Override
+  public void createAlignedTimeSeries(CreateAlignedTimeSeriesPlan plan) throws MetadataException {
+    PartialPath path = plan.getPrefixPath();
+    if (checkSgNodeAndPlanMajorVersion(path)) {
+      super.createAlignedTimeSeries(plan);
+    }
+  }
+
+  @Override
+  public void createTimeseries(CreateTimeSeriesPlan plan) throws MetadataException {
+    PartialPath path = plan.getPath();
+    if (checkSgNodeAndPlanMajorVersion(path)) {
+      super.createTimeseries(plan);
+    }
+  }
+
+  /**
+   * @param path the partial path
+   * @return true if the sg node major version == the path's major version; false if the sg node
+   *     major version > the path's major version; throw MetadataException if the sg node major
+   *     version < the path's major version.
+   * @throws MajorVersionNotEqualException sync meta leader failed and the

Review comment:
       please fix comments

##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/metadata/CMManager.java
##########
@@ -744,9 +767,27 @@ public boolean createTimeseries(InsertPlan insertPlan)
     if (unregisteredSeriesList.isEmpty()) {
       return true;
     }
-    logger.debug("Unregisterd series of {} are {}", seriesList, unregisteredSeriesList);
+    logger.debug("Unregistered series of {} are {}", seriesList, unregisteredSeriesList);
 
-    return createTimeseries(unregisteredSeriesList, seriesList, insertPlan);
+    return createTimeseries(unregisteredSeriesList, seriesList, insertPlan, sgNodeVersion);
+  }
+
+  /**
+   * @param partialPath the storage group's partialPath
+   * @return the major version and the minor version of the storage group node
+   */
+  private Pair<Long, Long> getSgNodeMajorVersion(PartialPath partialPath) throws MetadataException {
+    StorageGroupMNode node;
+    try {
+      node = getStorageGroupNodeByPath(partialPath);
+    } catch (MetadataException e) {
+      syncMetaLeader();
+      node = getStorageGroupNodeByPath(partialPath);
+    }
+    if (node == null) {
+      throw new MetadataException("get major version failed");
+    }
+    return new Pair(node.getMajorVersion(), node.getMinorVersion());
   }
 
   private boolean createAlignedTimeseries(List<String> seriesList, InsertPlan insertPlan)

Review comment:
       add versions for `createAlignedTimeseries`, they ought to be same.

##########
File path: cluster/src/main/java/org/apache/iotdb/cluster/metadata/CMManager.java
##########
@@ -1827,4 +1873,68 @@ public PartialPath getStorageGroupPath(PartialPath path) throws StorageGroupNotS
       return super.getStorageGroupPath(path);
     }
   }
+
+  @Override
+  public void createAlignedTimeSeries(CreateAlignedTimeSeriesPlan plan) throws MetadataException {
+    PartialPath path = plan.getPrefixPath();
+    if (checkSgNodeAndPlanMajorVersion(path)) {
+      super.createAlignedTimeSeries(plan);
+    }
+  }
+
+  @Override
+  public void createTimeseries(CreateTimeSeriesPlan plan) throws MetadataException {
+    PartialPath path = plan.getPath();
+    if (checkSgNodeAndPlanMajorVersion(path)) {
+      super.createTimeseries(plan);
+    }
+  }
+
+  /**
+   * @param path the partial path
+   * @return true if the sg node major version == the path's major version; false if the sg node
+   *     major version > the path's major version; throw MetadataException if the sg node major
+   *     version < the path's major version.

Review comment:
       throw MetadataException if the sg node major version < the path's major version **after syncMetaLeader**?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org