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 2020/10/08 07:25:46 UTC

[GitHub] [iotdb] qiaojialin commented on a change in pull request #1736: Cluster premerge

qiaojialin commented on a change in pull request #1736:
URL: https://github.com/apache/iotdb/pull/1736#discussion_r500718885



##########
File path: server/src/assembly/resources/conf/iotdb-engine.properties
##########
@@ -63,16 +63,17 @@ force_wal_period_in_ms=100
 ### Directory Configuration
 ####################
 
-# system dir
-# If this property is unset, system will save the data in the default relative path directory under the IoTDB folder(i.e., %IOTDB_HOME%/data/system).
+# base dir
+# If this property is unset, system will save the data in the default relative path directory under the IoTDB folder(i.e., %IOTDB_HOME%/data).
 # If it is absolute, system will save the data in exact location it points to.
 # If it is relative, system will save the data in the relative path directory it indicates under the IoTDB folder.
+# Note: If sys_dir is assigned an empty string(i.e.,zero-size), it will be handled as a relative path.
 # For windows platform
 # If its prefix is a drive specifier followed by "\\", or if its prefix is "\\\\", then the path is absolute. Otherwise, it is relative.
-# system_dir=data\\system
+# base_dir=data

Review comment:
       recover this

##########
File path: server/src/main/java/org/apache/iotdb/db/engine/StorageEngine.java
##########
@@ -381,39 +385,57 @@ public void syncCloseAllProcessor() {
   }
 
   public void forceCloseAllProcessor() throws TsFileProcessorException {
-    logger.info("Start closing all storage group processor");
+    logger.info("Start force closing all storage group processor");
     for (StorageGroupProcessor processor : processorMap.values()) {
       processor.forceCloseAllWorkingTsFileProcessors();
     }
   }
 
-  public void asyncCloseProcessor(PartialPath storageGroupPath, boolean isSeq) {
+  public void closeProcessor(PartialPath storageGroupPath, boolean isSeq, boolean isSync) {

Review comment:
       ```suggestion
     public void closeStorageGroupProcessor(PartialPath storageGroupPath, boolean isSeq, boolean isSync) {
   ```

##########
File path: server/src/main/java/org/apache/iotdb/db/metadata/mnode/MNode.java
##########
@@ -85,7 +87,7 @@ public boolean hasChild(String name) {
    */
   public void addChild(String name, MNode child) {
     if (children == null) {
-      children = new LinkedHashMap<>();
+      children = new ConcurrentSkipListMap<>();

Review comment:
       wait for Kaifeng's PR

##########
File path: server/src/main/java/org/apache/iotdb/db/engine/StorageEngine.java
##########
@@ -381,39 +385,57 @@ public void syncCloseAllProcessor() {
   }
 
   public void forceCloseAllProcessor() throws TsFileProcessorException {
-    logger.info("Start closing all storage group processor");
+    logger.info("Start force closing all storage group processor");
     for (StorageGroupProcessor processor : processorMap.values()) {
       processor.forceCloseAllWorkingTsFileProcessors();
     }
   }
 
-  public void asyncCloseProcessor(PartialPath storageGroupPath, boolean isSeq) {
+  public void closeProcessor(PartialPath storageGroupPath, boolean isSeq, boolean isSync) {
     StorageGroupProcessor processor = processorMap.get(storageGroupPath);
-    if (processor != null) {
-      logger.info("async closing sg processor is called for closing {}, seq = {}", storageGroupPath,
-          isSeq);
-      processor.writeLock();
-      try {
-        if (isSeq) {
-          // to avoid concurrent modification problem, we need a new array list
-          for (TsFileProcessor tsfileProcessor : new ArrayList<>(
-              processor.getWorkSequenceTsFileProcessors())) {
+    if (processor == null) {
+      return;
+    }
+
+    logger.info("async closing sg processor is called for closing {}, seq = {}", storageGroupPath,

Review comment:
       async should be inferred from parameter

##########
File path: server/src/main/java/org/apache/iotdb/db/engine/StorageEngine.java
##########
@@ -381,39 +385,57 @@ public void syncCloseAllProcessor() {
   }
 
   public void forceCloseAllProcessor() throws TsFileProcessorException {
-    logger.info("Start closing all storage group processor");
+    logger.info("Start force closing all storage group processor");
     for (StorageGroupProcessor processor : processorMap.values()) {
       processor.forceCloseAllWorkingTsFileProcessors();
     }
   }
 
-  public void asyncCloseProcessor(PartialPath storageGroupPath, boolean isSeq) {
+  public void closeProcessor(PartialPath storageGroupPath, boolean isSeq, boolean isSync) {
     StorageGroupProcessor processor = processorMap.get(storageGroupPath);
-    if (processor != null) {
-      logger.info("async closing sg processor is called for closing {}, seq = {}", storageGroupPath,
-          isSeq);
-      processor.writeLock();
-      try {
-        if (isSeq) {
-          // to avoid concurrent modification problem, we need a new array list
-          for (TsFileProcessor tsfileProcessor : new ArrayList<>(
-              processor.getWorkSequenceTsFileProcessors())) {
+    if (processor == null) {
+      return;
+    }
+
+    logger.info("async closing sg processor is called for closing {}, seq = {}", storageGroupPath,
+        isSeq);
+    processor.writeLock();
+    try {
+      if (isSeq) {
+        // to avoid concurrent modification problem, we need a new array list
+        for (TsFileProcessor tsfileProcessor : new ArrayList<>(
+            processor.getWorkSequenceTsFileProcessors())) {
+          if (isSync) {
+            processor.syncCloseOneTsFileProcessor(true, tsfileProcessor);
+          } else {
             processor.asyncCloseOneTsFileProcessor(true, tsfileProcessor);
           }
-        } else {
-          // to avoid concurrent modification problem, we need a new array list
-          for (TsFileProcessor tsfileProcessor : new ArrayList<>(
-              processor.getWorkUnsequenceTsFileProcessor())) {
+        }
+      } else {
+        // to avoid concurrent modification problem, we need a new array list
+        for (TsFileProcessor tsfileProcessor : new ArrayList<>(
+            processor.getWorkUnsequenceTsFileProcessor())) {
+          if (isSync) {
+            processor.syncCloseOneTsFileProcessor(false, tsfileProcessor);
+          } else {
             processor.asyncCloseOneTsFileProcessor(false, tsfileProcessor);
           }
         }
-      } finally {
-        processor.writeUnlock();
       }
+    } finally {
+      processor.writeUnlock();
     }
   }
 
-  public void asyncCloseProcessor(PartialPath storageGroupPath, long partitionId, boolean isSeq)
+  /**
+   * @param storageGroupPath the storage group name
+   * @param partitionId      the partition id
+   * @param isSeq            is sequence tsfile or unsequence tsfile
+   * @param isSync           close tsfile synchronously or asynchronously
+   * @throws StorageGroupNotSetException
+   */
+  public void closeProcessor(PartialPath storageGroupPath, long partitionId, boolean isSeq,

Review comment:
       ```suggestion
     public void closeStorageGroupProcessor(PartialPath storageGroupPath, long partitionId, boolean isSeq,
   ```

##########
File path: server/src/main/java/org/apache/iotdb/db/qp/physical/crud/DeletePartitionPlan.java
##########
@@ -38,7 +39,12 @@ public DeletePartitionPlan(PartialPath storageGroupName, Set<Long> partitionId)
 
   @Override
   public List<PartialPath> getPaths() {
-    return null;
+    return Collections.emptyList();
+  }
+
+  @Override
+  public List<String> getPathsStrings() {
+    return Collections.emptyList();

Review comment:
       don't you need return storage group name?




----------------------------------------------------------------
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.

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