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 2022/07/26 12:41:41 UTC

[GitHub] [iotdb] THUMarkLau opened a new pull request, #6782: [IOTDB-3771] Fix cannot take snapshot when the data dir and snapshot dir is on different disk

THUMarkLau opened a new pull request, #6782:
URL: https://github.com/apache/iotdb/pull/6782

   See [IOTDB-3771](https://issues.apache.org/jira/browse/IOTDB-3771).


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

[GitHub] [iotdb] neuyilan commented on pull request #6782: [IOTDB-3771] Fix cannot take snapshot when the data dir and snapshot dir is on different disk

Posted by GitBox <gi...@apache.org>.

neuyilan commented on PR #6782:
URL: https://github.com/apache/iotdb/pull/6782#issuecomment-1212904203

   There are some code smells which we may pay attention to.


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

[GitHub] [iotdb] sonarcloud[bot] commented on pull request #6782: [IOTDB-3771] Fix cannot take snapshot when the data dir and snapshot dir is on different disk

Posted by GitBox <gi...@apache.org>.

sonarcloud[bot] commented on PR #6782:
URL: https://github.com/apache/iotdb/pull/6782#issuecomment-1216780350

   SonarCloud Quality Gate failed.&nbsp; &nbsp; [![Quality Gate failed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/failed-16px.png 'Quality Gate failed')](https://sonarcloud.io/dashboard?id=apache_incubator-iotdb&pullRequest=6782)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=BUG) [![C](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/C-16px.png 'C')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=BUG) [1 Bug](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=CODE_SMELL) [45 Code Smells](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=CODE_SMELL)
   
   [![54.4%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/50-16px.png '54.4%')](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=6782&metric=new_coverage&view=list) [54.4% Coverage](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=6782&metric=new_coverage&view=list)  
   [![3.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/5-16px.png '3.0%')](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=6782&metric=new_duplicated_lines_density&view=list) [3.0% Duplication](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=6782&metric=new_duplicated_lines_density&view=list)
   
   


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

[GitHub] [iotdb] THUMarkLau commented on a diff in pull request #6782: [IOTDB-3771] Fix cannot take snapshot when the data dir and snapshot dir is on different disk

Posted by GitBox <gi...@apache.org>.

THUMarkLau commented on code in PR #6782:
URL: https://github.com/apache/iotdb/pull/6782#discussion_r947675550


##########
server/src/main/java/org/apache/iotdb/db/engine/snapshot/SnapshotTaker.java:
##########
@@ -89,111 +70,131 @@ public boolean takeFullSnapshot(String snapshotDirPath, boolean flushBeforeSnaps
       dataRegion.syncCloseAllWorkingTsFileProcessors();
     }
 
-    List<Long> timePartitions = dataRegion.getTimePartitions();
+    File snapshotLog = new File(snapshotDir, SnapshotLogger.SNAPSHOT_LOG_NAME);
+    try {
+      snapshotLogger = new SnapshotLogger(snapshotLog);
+      boolean success = false;
+
+      readLockTheFile();
+      try {
+        success = createSnapshot(seqFiles, snapshotDir.getName()) || success;
+        success = createSnapshot(unseqFiles, snapshotDir.getName()) || success;
+      } finally {
+        readUnlockTheFile();
+      }
+
+      LOGGER.info(
+          "Successfully take snapshot for {}-{}, snapshot directory is {}",
+          dataRegion.getStorageGroupName(),
+          dataRegion.getDataRegionId(),
+          snapshotDirPath);
+
+      return success;
+    } catch (Exception e) {
+      LOGGER.error(
+          "Exception occurs when taking snapshot for {}-{}",
+          dataRegion.getStorageGroupName(),
+          dataRegion.getDataRegionId(),
+          e);
+      return false;
+    } finally {
+      try {
+        snapshotLogger.close();
+      } catch (Exception e) {
+        LOGGER.error("Failed to close snapshot logger", e);
+      }
+    }
+  }
+
+  private void readLockTheFile() {
     TsFileManager manager = dataRegion.getTsFileManager();
     manager.readLock();
     try {
-      for (Long timePartition : timePartitions) {
-        List<String> seqDataDirs = getAllDataDirOfOnePartition(true, timePartition);
-
-        try {
-          createFileSnapshot(seqDataDirs, true, timePartition);
-        } catch (IOException e) {
-          LOGGER.error("Fail to create snapshot", e);
-          cleanUpWhenFail(snapshotDir);
-          return false;
-        }
-
-        List<String> unseqDataDirs = getAllDataDirOfOnePartition(false, timePartition);
-
-        try {
-          createFileSnapshot(unseqDataDirs, false, timePartition);
-        } catch (IOException e) {
-          LOGGER.error("Fail to create snapshot", e);
-          cleanUpWhenFail(snapshotDir);
-          return false;
-        }
+      seqFiles = manager.getTsFileList(true);
+      unseqFiles = manager.getTsFileList(false);
+      for (TsFileResource resource : seqFiles) {
+        resource.readLock();
+      }
+      for (TsFileResource resource : unseqFiles) {
+        resource.readLock();
       }
     } finally {
       manager.readUnlock();
     }
-
-    LOGGER.info(
-        "Successfully take snapshot for {}-{}, snapshot directory is {}",
-        dataRegion.getLogicalStorageGroupName(),
-        dataRegion.getDataRegionId(),
-        snapshotDirPath);
-
-    return true;
   }
 
-  private List<String> getAllDataDirOfOnePartition(boolean sequence, long timePartition) {
-    String[] dataDirs = IoTDBDescriptor.getInstance().getConfig().getDataDirs();
-    List<String> resultDirs = new LinkedList<>();
-
-    for (String dataDir : dataDirs) {
-      resultDirs.add(
-          dataDir
-              + File.separator
-              + (sequence
-                  ? IoTDBConstant.SEQUENCE_FLODER_NAME
-                  : IoTDBConstant.UNSEQUENCE_FLODER_NAME)
-              + File.separator
-              + dataRegion.getLogicalStorageGroupName()
-              + File.separator
-              + dataRegion.getDataRegionId()
-              + File.separator
-              + timePartition
-              + File.separator);
+  private void readUnlockTheFile() {
+    for (TsFileResource resource : seqFiles) {
+      resource.readUnlock();
+    }
+    for (TsFileResource resource : unseqFiles) {
+      resource.readUnlock();
     }
-    return resultDirs;
   }
 
-  private void createFileSnapshot(List<String> sourceDirPaths, boolean sequence, long timePartition)
+  private boolean createSnapshot(List<TsFileResource> resources, String snapshotId)
       throws IOException {
-    File timePartitionDir =
-        new File(sequence ? seqBaseDir : unseqBaseDir, String.valueOf(timePartition));
-    if (!timePartitionDir.exists() && !timePartitionDir.mkdirs()) {
-      throw new IOException(
-          String.format("%s not exists and cannot create it", timePartitionDir.getAbsolutePath()));
-    }
-
-    for (String sourceDirPath : sourceDirPaths) {
-      File sourceDir = new File(sourceDirPath);
-      if (!sourceDir.exists()) {
-        continue;
-      }
-      // Collect TsFile, TsFileResource, Mods, CompactionMods
-      File[] files =
-          sourceDir.listFiles(
-              (dir, name) ->
-                  name.endsWith(".tsfile")
-                      || name.endsWith(TsFileResource.RESOURCE_SUFFIX)
-                      || name.endsWith(ModificationFile.FILE_SUFFIX)
-                      || name.endsWith(ModificationFile.COMPACTION_FILE_SUFFIX)
-                      || name.endsWith(CompactionLogger.INNER_COMPACTION_LOG_NAME_SUFFIX)
-                      || name.endsWith(CompactionLogger.CROSS_COMPACTION_LOG_NAME_SUFFIX)
-                      || name.endsWith(IoTDBConstant.INNER_COMPACTION_TMP_FILE_SUFFIX)
-                      || name.endsWith(IoTDBConstant.CROSS_COMPACTION_TMP_FILE_SUFFIX));
-      if (files == null || files.length == 0) {
-        continue;
-      }
-
-      for (File file : files) {
-        File linkFile = new File(timePartitionDir, file.getName());
-        Files.createLink(linkFile.toPath(), file.toPath());
+    for (TsFileResource resource : resources) {
+      File tsFile = resource.getTsFile();
+      File snapshotTsFile = getSnapshotFilePathForResource(tsFile, snapshotId);
+      // create hard link for tsfile, resource, mods, compaction mods

Review Comment:
   resolve



##########
server/src/main/java/org/apache/iotdb/db/engine/snapshot/SnapshotTaker.java:
##########
@@ -89,111 +70,131 @@ public boolean takeFullSnapshot(String snapshotDirPath, boolean flushBeforeSnaps
       dataRegion.syncCloseAllWorkingTsFileProcessors();
     }
 
-    List<Long> timePartitions = dataRegion.getTimePartitions();
+    File snapshotLog = new File(snapshotDir, SnapshotLogger.SNAPSHOT_LOG_NAME);
+    try {
+      snapshotLogger = new SnapshotLogger(snapshotLog);
+      boolean success = false;
+
+      readLockTheFile();
+      try {
+        success = createSnapshot(seqFiles, snapshotDir.getName()) || success;
+        success = createSnapshot(unseqFiles, snapshotDir.getName()) || success;
+      } finally {
+        readUnlockTheFile();
+      }
+
+      LOGGER.info(
+          "Successfully take snapshot for {}-{}, snapshot directory is {}",
+          dataRegion.getStorageGroupName(),
+          dataRegion.getDataRegionId(),
+          snapshotDirPath);
+
+      return success;
+    } catch (Exception e) {
+      LOGGER.error(
+          "Exception occurs when taking snapshot for {}-{}",
+          dataRegion.getStorageGroupName(),
+          dataRegion.getDataRegionId(),
+          e);
+      return false;
+    } finally {
+      try {
+        snapshotLogger.close();
+      } catch (Exception e) {
+        LOGGER.error("Failed to close snapshot logger", e);
+      }
+    }
+  }
+
+  private void readLockTheFile() {
     TsFileManager manager = dataRegion.getTsFileManager();
     manager.readLock();
     try {
-      for (Long timePartition : timePartitions) {
-        List<String> seqDataDirs = getAllDataDirOfOnePartition(true, timePartition);
-
-        try {
-          createFileSnapshot(seqDataDirs, true, timePartition);
-        } catch (IOException e) {
-          LOGGER.error("Fail to create snapshot", e);
-          cleanUpWhenFail(snapshotDir);
-          return false;
-        }
-
-        List<String> unseqDataDirs = getAllDataDirOfOnePartition(false, timePartition);
-
-        try {
-          createFileSnapshot(unseqDataDirs, false, timePartition);
-        } catch (IOException e) {
-          LOGGER.error("Fail to create snapshot", e);
-          cleanUpWhenFail(snapshotDir);
-          return false;
-        }
+      seqFiles = manager.getTsFileList(true);
+      unseqFiles = manager.getTsFileList(false);
+      for (TsFileResource resource : seqFiles) {
+        resource.readLock();
+      }
+      for (TsFileResource resource : unseqFiles) {
+        resource.readLock();
       }
     } finally {
       manager.readUnlock();
     }
-
-    LOGGER.info(
-        "Successfully take snapshot for {}-{}, snapshot directory is {}",
-        dataRegion.getLogicalStorageGroupName(),
-        dataRegion.getDataRegionId(),
-        snapshotDirPath);
-
-    return true;
   }
 
-  private List<String> getAllDataDirOfOnePartition(boolean sequence, long timePartition) {
-    String[] dataDirs = IoTDBDescriptor.getInstance().getConfig().getDataDirs();
-    List<String> resultDirs = new LinkedList<>();
-
-    for (String dataDir : dataDirs) {
-      resultDirs.add(
-          dataDir
-              + File.separator
-              + (sequence
-                  ? IoTDBConstant.SEQUENCE_FLODER_NAME
-                  : IoTDBConstant.UNSEQUENCE_FLODER_NAME)
-              + File.separator
-              + dataRegion.getLogicalStorageGroupName()
-              + File.separator
-              + dataRegion.getDataRegionId()
-              + File.separator
-              + timePartition
-              + File.separator);
+  private void readUnlockTheFile() {
+    for (TsFileResource resource : seqFiles) {
+      resource.readUnlock();
+    }
+    for (TsFileResource resource : unseqFiles) {
+      resource.readUnlock();
     }
-    return resultDirs;
   }
 
-  private void createFileSnapshot(List<String> sourceDirPaths, boolean sequence, long timePartition)
+  private boolean createSnapshot(List<TsFileResource> resources, String snapshotId)
       throws IOException {
-    File timePartitionDir =
-        new File(sequence ? seqBaseDir : unseqBaseDir, String.valueOf(timePartition));
-    if (!timePartitionDir.exists() && !timePartitionDir.mkdirs()) {
-      throw new IOException(
-          String.format("%s not exists and cannot create it", timePartitionDir.getAbsolutePath()));
-    }
-
-    for (String sourceDirPath : sourceDirPaths) {
-      File sourceDir = new File(sourceDirPath);
-      if (!sourceDir.exists()) {
-        continue;
-      }
-      // Collect TsFile, TsFileResource, Mods, CompactionMods
-      File[] files =
-          sourceDir.listFiles(
-              (dir, name) ->
-                  name.endsWith(".tsfile")
-                      || name.endsWith(TsFileResource.RESOURCE_SUFFIX)
-                      || name.endsWith(ModificationFile.FILE_SUFFIX)
-                      || name.endsWith(ModificationFile.COMPACTION_FILE_SUFFIX)
-                      || name.endsWith(CompactionLogger.INNER_COMPACTION_LOG_NAME_SUFFIX)
-                      || name.endsWith(CompactionLogger.CROSS_COMPACTION_LOG_NAME_SUFFIX)
-                      || name.endsWith(IoTDBConstant.INNER_COMPACTION_TMP_FILE_SUFFIX)
-                      || name.endsWith(IoTDBConstant.CROSS_COMPACTION_TMP_FILE_SUFFIX));
-      if (files == null || files.length == 0) {
-        continue;
-      }
-
-      for (File file : files) {
-        File linkFile = new File(timePartitionDir, file.getName());
-        Files.createLink(linkFile.toPath(), file.toPath());
+    for (TsFileResource resource : resources) {
+      File tsFile = resource.getTsFile();
+      File snapshotTsFile = getSnapshotFilePathForResource(tsFile, snapshotId);
+      // create hard link for tsfile, resource, mods, compaction mods
+      createHardLink(snapshotTsFile, tsFile);
+      createHardLink(
+          new File(snapshotTsFile.getAbsolutePath() + TsFileResource.RESOURCE_SUFFIX),
+          new File(tsFile.getAbsolutePath() + TsFileResource.RESOURCE_SUFFIX));
+      if (resource.getModFile().exists()) {
+        createHardLink(
+            new File(snapshotTsFile.getAbsolutePath() + ModificationFile.FILE_SUFFIX),
+            new File(tsFile.getAbsolutePath() + ModificationFile.FILE_SUFFIX));
       }
     }
+    return true;
   }
 
-  private void cleanUpWhenFail(File snapshotDir) {
-    File[] files = snapshotDir.listFiles();
-    if (files != null) {
-      for (File file : files) {
-        if (!file.delete()) {
-          LOGGER.error("Failed to delete link file {} after failing to create snapshot", file);
-        }
-      }
+  private void createHardLink(File target, File source) throws IOException {
+    Files.createLink(target.toPath(), source.toPath());
+    snapshotLogger.logFile(source.getAbsolutePath(), target.getAbsolutePath());
+  }
+
+  /**
+   * Construct the snapshot file path for a given tsfile, and will create the dir. Eg, given a
+   * tsfile in /data/iotdb/data/sequence/root.testsg/1/0/1-1-0-0.tsfile, with snapshotId "sm123",
+   * the snapshot location will be /data/iotdb/data/snapshot/sm123/root.testsg/1/0/1-1-0-0.tsfile
+   *
+   * @param tsFile tsfile to be taken a snapshot
+   * @param snapshotId the id for current snapshot
+   * @return the File object of the snapshot file, and its parent directory will be created
+   * @throws IOException
+   */
+  public File getSnapshotFilePathForResource(File tsFile, String snapshotId) throws IOException {

Review Comment:
   resolve



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

[GitHub] [iotdb] sonarcloud[bot] commented on pull request #6782: [IOTDB-3771] Fix cannot take snapshot when the data dir and snapshot dir is on different disk

Posted by GitBox <gi...@apache.org>.

sonarcloud[bot] commented on PR #6782:
URL: https://github.com/apache/iotdb/pull/6782#issuecomment-1218100333

   SonarCloud Quality Gate failed.&nbsp; &nbsp; [![Quality Gate failed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/failed-16px.png 'Quality Gate failed')](https://sonarcloud.io/dashboard?id=apache_incubator-iotdb&pullRequest=6782)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=BUG) [![E](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/E-16px.png 'E')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=BUG) [6 Bugs](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=CODE_SMELL) [78 Code Smells](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=CODE_SMELL)
   
   [![43.2%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/40-16px.png '43.2%')](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=6782&metric=new_coverage&view=list) [43.2% Coverage](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=6782&metric=new_coverage&view=list)  
   [![0.9%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.9%')](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=6782&metric=new_duplicated_lines_density&view=list) [0.9% Duplication](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=6782&metric=new_duplicated_lines_density&view=list)
   
   


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

[GitHub] [iotdb] THUMarkLau commented on a diff in pull request #6782: [IOTDB-3771] Fix cannot take snapshot when the data dir and snapshot dir is on different disk

Posted by GitBox <gi...@apache.org>.

THUMarkLau commented on code in PR #6782:
URL: https://github.com/apache/iotdb/pull/6782#discussion_r939766582


##########
server/src/main/java/org/apache/iotdb/db/engine/snapshot/SnapshotTaker.java:
##########
@@ -117,83 +208,150 @@ public boolean takeFullSnapshot(String snapshotDirPath, boolean flushBeforeSnaps
     } finally {
       manager.readUnlock();
     }
+    return true;
+  }
+
+  private boolean createSnapshotInAnotherDisk(File snapshotDir) {
+    for (String dataDir : IoTDBDescriptor.getInstance().getConfig().getDataDirs()) {
+      File seqDir =
+          new File(
+              dataDir,
+              "sequence"
+                  + File.separator
+                  + dataRegion.getStorageGroupName()
+                  + File.separator
+                  + dataRegion.getDataRegionId());

Review Comment:
   The core of these two functions to create hard links steps are the same, and this step has been extracted as function "createFileSnapshotToTargetOne".



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

[GitHub] [iotdb] sonarcloud[bot] commented on pull request #6782: [IOTDB-3771] Fix cannot take snapshot when the data dir and snapshot dir is on different disk

Posted by GitBox <gi...@apache.org>.

sonarcloud[bot] commented on PR #6782:
URL: https://github.com/apache/iotdb/pull/6782#issuecomment-1211862224

   SonarCloud Quality Gate failed.&nbsp; &nbsp; [![Quality Gate failed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/failed-16px.png 'Quality Gate failed')](https://sonarcloud.io/dashboard?id=apache_incubator-iotdb&pullRequest=6782)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=BUG) [![B](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/B-16px.png 'B')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=BUG) [1 Bug](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=CODE_SMELL) [41 Code Smells](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=CODE_SMELL)
   
   [![38.9%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/25-16px.png '38.9%')](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=6782&metric=new_coverage&view=list) [38.9% Coverage](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=6782&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=6782&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=6782&metric=new_duplicated_lines_density&view=list)
   
   


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

[GitHub] [iotdb] sonarcloud[bot] commented on pull request #6782: [IOTDB-3771] Fix cannot take snapshot when the data dir and snapshot dir is on different disk

Posted by GitBox <gi...@apache.org>.

sonarcloud[bot] commented on PR #6782:
URL: https://github.com/apache/iotdb/pull/6782#issuecomment-1216343663

   SonarCloud Quality Gate failed.&nbsp; &nbsp; [![Quality Gate failed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/failed-16px.png 'Quality Gate failed')](https://sonarcloud.io/dashboard?id=apache_incubator-iotdb&pullRequest=6782)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=CODE_SMELL) [32 Code Smells](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=CODE_SMELL)
   
   [![56.4%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/50-16px.png '56.4%')](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=6782&metric=new_coverage&view=list) [56.4% Coverage](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=6782&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=6782&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=6782&metric=new_duplicated_lines_density&view=list)
   
   


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

[GitHub] [iotdb] neuyilan commented on a diff in pull request #6782: [IOTDB-3771] Fix cannot take snapshot when the data dir and snapshot dir is on different disk

Posted by GitBox <gi...@apache.org>.

neuyilan commented on code in PR #6782:
URL: https://github.com/apache/iotdb/pull/6782#discussion_r935067470


##########
server/src/main/java/org/apache/iotdb/db/engine/snapshot/SnapshotLogger.java:
##########
@@ -0,0 +1,69 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iotdb.db.engine.snapshot;
+
+import java.io.BufferedOutputStream;
+import java.io.File;
+import java.io.FileNotFoundException;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+
+public class SnapshotLogger implements AutoCloseable {
+  public static final String SNAPSHOT_LOG_NAME = "snapshot.log";
+  public static final String SPLIT_CHAR = "#";
+
+  private File logFile;
+  private BufferedOutputStream os;
+
+  public SnapshotLogger(File logFile) throws FileNotFoundException {
+    this.logFile = logFile;
+    os = new BufferedOutputStream(new FileOutputStream(logFile));
+  }
+
+  @Override
+  public void close() throws Exception {
+    os.close();
+  }
+
+  public void logFile(String sourceFile, String linkFile) throws IOException {
+    os.write(sourceFile.getBytes(StandardCharsets.UTF_8));
+    os.write(SPLIT_CHAR.getBytes(StandardCharsets.UTF_8));
+    os.write(linkFile.getBytes(StandardCharsets.UTF_8));
+    os.write("\n".getBytes(StandardCharsets.UTF_8));
+    os.flush();
+  }
+
+  public void logSnapshotType(SnapshotType type) throws IOException {
+    os.write(String.valueOf(type).getBytes(StandardCharsets.UTF_8));
+    os.write("\n".getBytes(StandardCharsets.UTF_8));
+    os.flush();
+  }
+
+  public void cleanUpWhenFailed() throws IOException {
+    os.close();
+    Files.delete(logFile.toPath());
+  }
+
+  public static enum SnapshotType {
+    LOCAL_DISK,
+    REMOTE_DISK

Review Comment:
   The `REMOTE_DISK` is not used, whether it can be deleted？



##########
server/src/main/java/org/apache/iotdb/db/engine/snapshot/SnapshotTaker.java:
##########
@@ -117,83 +208,150 @@ public boolean takeFullSnapshot(String snapshotDirPath, boolean flushBeforeSnaps
     } finally {
       manager.readUnlock();
     }
+    return true;
+  }
+
+  private boolean createSnapshotInAnotherDisk(File snapshotDir) {
+    for (String dataDir : IoTDBDescriptor.getInstance().getConfig().getDataDirs()) {
+      File seqDir =
+          new File(
+              dataDir,
+              "sequence"
+                  + File.separator
+                  + dataRegion.getStorageGroupName()
+                  + File.separator
+                  + dataRegion.getDataRegionId());

Review Comment:
   Can this function be combined with `createSnapshotInLocalDisk`？
   actually, I think the only difference between the two functions is the target path of the snapshot dir.



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

[GitHub] [iotdb] THUMarkLau commented on a diff in pull request #6782: [IOTDB-3771] Fix cannot take snapshot when the data dir and snapshot dir is on different disk

Posted by GitBox <gi...@apache.org>.

THUMarkLau commented on code in PR #6782:
URL: https://github.com/apache/iotdb/pull/6782#discussion_r930040442


##########
server/src/main/java/org/apache/iotdb/db/engine/snapshot/SnapshotTaker.java:
##########
@@ -117,83 +208,152 @@ public boolean takeFullSnapshot(String snapshotDirPath, boolean flushBeforeSnaps
     } finally {
       manager.readUnlock();
     }
+    return true;
+  }
+
+  private boolean createSnapshotInAnotherDisk(File snapshotDir) {
+    for (String dataDir : IoTDBDescriptor.getInstance().getConfig().getDataDirs()) {
+      File seqDir =
+          new File(
+              dataDir,
+              "sequence"
+                  + File.separator
+                  + dataRegion.getLogicalStorageGroupName()
+                  + File.separator
+                  + dataRegion.getDataRegionId());
+      File unseqDir =
+          new File(
+              dataDir,
+              "unsequence"
+                  + File.separator
+                  + dataRegion.getLogicalStorageGroupName()
+                  + File.separator
+                  + dataRegion.getDataRegionId());
+      File localSeqSnapshotDir =
+          new File(
+              dataDir,
+              "snapshot"
+                  + File.separator
+                  + "sequence"
+                  + File.separator
+                  + snapshotDir.getName()
+                  + File.separator
+                  + dataRegion.getLogicalStorageGroupName()
+                  + File.separator
+                  + dataRegion.getDataRegionId());
+      File localUnseqSnapshotDir =
+          new File(
+              dataDir,
+              "snapshot"
+                  + File.separator
+                  + "unsequence"
+                  + File.separator
+                  + snapshotDir.getName()
+                  + File.separator
+                  + dataRegion.getLogicalStorageGroupName()
+                  + File.separator
+                  + dataRegion.getDataRegionId());
+      if (!localSeqSnapshotDir.mkdirs()) {
+        LOGGER.warn("Failed to create local snapshot dir {}", localSeqSnapshotDir);
+        return false;
+      }
+      List<Long> timePartitions = dataRegion.getTimePartitions();
+      TsFileManager manager = dataRegion.getTsFileManager();
+      manager.readLock();
+      for (long timePartition : timePartitions) {
+        File seqTimePartitionDir = new File(seqDir, String.valueOf(timePartition));
+        File seqTimePartitionSnapshotDir =
+            new File(localSeqSnapshotDir, String.valueOf(timePartition));
 
-    LOGGER.info(
-        "Successfully take snapshot for {}-{}, snapshot directory is {}",
-        dataRegion.getLogicalStorageGroupName(),
-        dataRegion.getDataRegionId(),
-        snapshotDirPath);
+        try {
+          createFileSnapshotToTargetOne(seqTimePartitionDir, seqTimePartitionSnapshotDir);
+        } catch (IOException e) {
+          LOGGER.error(
+              "Failed to create snapshot for {}-{}",
+              dataRegion.getLogicalStorageGroupName(),
+              dataRegion.getDataRegionId(),
+              e);
+        } finally {
+          cleanUpWhenFail(snapshotDir);

Review Comment:
   This finally is unnecessary, so I've deleted it.



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

[GitHub] [iotdb] sonarcloud[bot] commented on pull request #6782: [IOTDB-3771] Fix cannot take snapshot when the data dir and snapshot dir is on different disk

Posted by GitBox <gi...@apache.org>.

sonarcloud[bot] commented on PR #6782:
URL: https://github.com/apache/iotdb/pull/6782#issuecomment-1208927239

   SonarCloud Quality Gate failed.&nbsp; &nbsp; [![Quality Gate failed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/failed-16px.png 'Quality Gate failed')](https://sonarcloud.io/dashboard?id=apache_incubator-iotdb&pullRequest=6782)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=BUG) [![B](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/B-16px.png 'B')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=BUG) [1 Bug](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=CODE_SMELL) [33 Code Smells](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=CODE_SMELL)
   
   [![57.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/50-16px.png '57.0%')](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=6782&metric=new_coverage&view=list) [57.0% Coverage](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=6782&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=6782&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=6782&metric=new_duplicated_lines_density&view=list)
   
   


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

[GitHub] [iotdb] qiaojialin commented on a diff in pull request #6782: [IOTDB-3771] Fix cannot take snapshot when the data dir and snapshot dir is on different disk

Posted by GitBox <gi...@apache.org>.

qiaojialin commented on code in PR #6782:
URL: https://github.com/apache/iotdb/pull/6782#discussion_r947536822


##########
server/src/main/java/org/apache/iotdb/db/engine/storagegroup/TsFileResource.java:
##########
@@ -1005,4 +1005,8 @@ private void generatePathToTimeSeriesMetadataMap() throws IOException {
   public boolean isFileInList() {
     return prev != null || next != null;
   }
+
+  public boolean createHardLink(String path) {

Review Comment:
   remove



##########
server/src/main/java/org/apache/iotdb/db/engine/snapshot/SnapshotTaker.java:
##########
@@ -89,111 +70,131 @@ public boolean takeFullSnapshot(String snapshotDirPath, boolean flushBeforeSnaps
       dataRegion.syncCloseAllWorkingTsFileProcessors();
     }
 
-    List<Long> timePartitions = dataRegion.getTimePartitions();
+    File snapshotLog = new File(snapshotDir, SnapshotLogger.SNAPSHOT_LOG_NAME);
+    try {
+      snapshotLogger = new SnapshotLogger(snapshotLog);
+      boolean success = false;
+
+      readLockTheFile();
+      try {
+        success = createSnapshot(seqFiles, snapshotDir.getName()) || success;
+        success = createSnapshot(unseqFiles, snapshotDir.getName()) || success;
+      } finally {
+        readUnlockTheFile();
+      }
+
+      LOGGER.info(
+          "Successfully take snapshot for {}-{}, snapshot directory is {}",
+          dataRegion.getStorageGroupName(),
+          dataRegion.getDataRegionId(),
+          snapshotDirPath);
+
+      return success;

Review Comment:
   could move to finally



##########
server/src/main/java/org/apache/iotdb/db/engine/snapshot/SnapshotTaker.java:
##########
@@ -89,111 +70,131 @@ public boolean takeFullSnapshot(String snapshotDirPath, boolean flushBeforeSnaps
       dataRegion.syncCloseAllWorkingTsFileProcessors();
     }
 
-    List<Long> timePartitions = dataRegion.getTimePartitions();
+    File snapshotLog = new File(snapshotDir, SnapshotLogger.SNAPSHOT_LOG_NAME);
+    try {
+      snapshotLogger = new SnapshotLogger(snapshotLog);
+      boolean success = false;
+
+      readLockTheFile();
+      try {
+        success = createSnapshot(seqFiles, snapshotDir.getName()) || success;
+        success = createSnapshot(unseqFiles, snapshotDir.getName()) || success;
+      } finally {
+        readUnlockTheFile();
+      }
+
+      LOGGER.info(
+          "Successfully take snapshot for {}-{}, snapshot directory is {}",
+          dataRegion.getStorageGroupName(),
+          dataRegion.getDataRegionId(),
+          snapshotDirPath);
+
+      return success;
+    } catch (Exception e) {
+      LOGGER.error(
+          "Exception occurs when taking snapshot for {}-{}",
+          dataRegion.getStorageGroupName(),
+          dataRegion.getDataRegionId(),
+          e);
+      return false;
+    } finally {
+      try {
+        snapshotLogger.close();
+      } catch (Exception e) {
+        LOGGER.error("Failed to close snapshot logger", e);
+      }
+    }
+  }
+
+  private void readLockTheFile() {
     TsFileManager manager = dataRegion.getTsFileManager();
     manager.readLock();
     try {
-      for (Long timePartition : timePartitions) {
-        List<String> seqDataDirs = getAllDataDirOfOnePartition(true, timePartition);
-
-        try {
-          createFileSnapshot(seqDataDirs, true, timePartition);
-        } catch (IOException e) {
-          LOGGER.error("Fail to create snapshot", e);
-          cleanUpWhenFail(snapshotDir);
-          return false;
-        }
-
-        List<String> unseqDataDirs = getAllDataDirOfOnePartition(false, timePartition);
-
-        try {
-          createFileSnapshot(unseqDataDirs, false, timePartition);
-        } catch (IOException e) {
-          LOGGER.error("Fail to create snapshot", e);
-          cleanUpWhenFail(snapshotDir);
-          return false;
-        }
+      seqFiles = manager.getTsFileList(true);
+      unseqFiles = manager.getTsFileList(false);
+      for (TsFileResource resource : seqFiles) {
+        resource.readLock();
+      }
+      for (TsFileResource resource : unseqFiles) {
+        resource.readLock();
       }
     } finally {
       manager.readUnlock();
     }
-
-    LOGGER.info(
-        "Successfully take snapshot for {}-{}, snapshot directory is {}",
-        dataRegion.getLogicalStorageGroupName(),
-        dataRegion.getDataRegionId(),
-        snapshotDirPath);
-
-    return true;
   }
 
-  private List<String> getAllDataDirOfOnePartition(boolean sequence, long timePartition) {
-    String[] dataDirs = IoTDBDescriptor.getInstance().getConfig().getDataDirs();
-    List<String> resultDirs = new LinkedList<>();
-
-    for (String dataDir : dataDirs) {
-      resultDirs.add(
-          dataDir
-              + File.separator
-              + (sequence
-                  ? IoTDBConstant.SEQUENCE_FLODER_NAME
-                  : IoTDBConstant.UNSEQUENCE_FLODER_NAME)
-              + File.separator
-              + dataRegion.getLogicalStorageGroupName()
-              + File.separator
-              + dataRegion.getDataRegionId()
-              + File.separator
-              + timePartition
-              + File.separator);
+  private void readUnlockTheFile() {
+    for (TsFileResource resource : seqFiles) {
+      resource.readUnlock();
+    }
+    for (TsFileResource resource : unseqFiles) {
+      resource.readUnlock();
     }
-    return resultDirs;
   }
 
-  private void createFileSnapshot(List<String> sourceDirPaths, boolean sequence, long timePartition)
+  private boolean createSnapshot(List<TsFileResource> resources, String snapshotId)
       throws IOException {
-    File timePartitionDir =
-        new File(sequence ? seqBaseDir : unseqBaseDir, String.valueOf(timePartition));
-    if (!timePartitionDir.exists() && !timePartitionDir.mkdirs()) {
-      throw new IOException(
-          String.format("%s not exists and cannot create it", timePartitionDir.getAbsolutePath()));
-    }
-
-    for (String sourceDirPath : sourceDirPaths) {
-      File sourceDir = new File(sourceDirPath);
-      if (!sourceDir.exists()) {
-        continue;
-      }
-      // Collect TsFile, TsFileResource, Mods, CompactionMods
-      File[] files =
-          sourceDir.listFiles(
-              (dir, name) ->
-                  name.endsWith(".tsfile")
-                      || name.endsWith(TsFileResource.RESOURCE_SUFFIX)
-                      || name.endsWith(ModificationFile.FILE_SUFFIX)
-                      || name.endsWith(ModificationFile.COMPACTION_FILE_SUFFIX)
-                      || name.endsWith(CompactionLogger.INNER_COMPACTION_LOG_NAME_SUFFIX)
-                      || name.endsWith(CompactionLogger.CROSS_COMPACTION_LOG_NAME_SUFFIX)
-                      || name.endsWith(IoTDBConstant.INNER_COMPACTION_TMP_FILE_SUFFIX)
-                      || name.endsWith(IoTDBConstant.CROSS_COMPACTION_TMP_FILE_SUFFIX));
-      if (files == null || files.length == 0) {
-        continue;
-      }
-
-      for (File file : files) {
-        File linkFile = new File(timePartitionDir, file.getName());
-        Files.createLink(linkFile.toPath(), file.toPath());
+    for (TsFileResource resource : resources) {
+      File tsFile = resource.getTsFile();
+      File snapshotTsFile = getSnapshotFilePathForResource(tsFile, snapshotId);
+      // create hard link for tsfile, resource, mods, compaction mods
+      createHardLink(snapshotTsFile, tsFile);
+      createHardLink(
+          new File(snapshotTsFile.getAbsolutePath() + TsFileResource.RESOURCE_SUFFIX),
+          new File(tsFile.getAbsolutePath() + TsFileResource.RESOURCE_SUFFIX));
+      if (resource.getModFile().exists()) {
+        createHardLink(
+            new File(snapshotTsFile.getAbsolutePath() + ModificationFile.FILE_SUFFIX),
+            new File(tsFile.getAbsolutePath() + ModificationFile.FILE_SUFFIX));
       }
     }
+    return true;
   }
 
-  private void cleanUpWhenFail(File snapshotDir) {
-    File[] files = snapshotDir.listFiles();
-    if (files != null) {
-      for (File file : files) {
-        if (!file.delete()) {
-          LOGGER.error("Failed to delete link file {} after failing to create snapshot", file);
-        }
-      }
+  private void createHardLink(File target, File source) throws IOException {
+    Files.createLink(target.toPath(), source.toPath());
+    snapshotLogger.logFile(source.getAbsolutePath(), target.getAbsolutePath());
+  }
+
+  /**
+   * Construct the snapshot file path for a given tsfile, and will create the dir. Eg, given a
+   * tsfile in /data/iotdb/data/sequence/root.testsg/1/0/1-1-0-0.tsfile, with snapshotId "sm123",
+   * the snapshot location will be /data/iotdb/data/snapshot/sm123/root.testsg/1/0/1-1-0-0.tsfile
+   *
+   * @param tsFile tsfile to be taken a snapshot
+   * @param snapshotId the id for current snapshot
+   * @return the File object of the snapshot file, and its parent directory will be created
+   * @throws IOException
+   */
+  public File getSnapshotFilePathForResource(File tsFile, String snapshotId) throws IOException {

Review Comment:
   getSnapshotFilePathForTsFile



##########
server/src/main/java/org/apache/iotdb/db/engine/snapshot/SnapshotTaker.java:
##########
@@ -89,111 +70,114 @@ public boolean takeFullSnapshot(String snapshotDirPath, boolean flushBeforeSnaps
       dataRegion.syncCloseAllWorkingTsFileProcessors();
     }
 
-    List<Long> timePartitions = dataRegion.getTimePartitions();
+    File snapshotLog = new File(snapshotDir, SnapshotLogger.SNAPSHOT_LOG_NAME);
+    try {
+      snapshotLogger = new SnapshotLogger(snapshotLog);
+      boolean success = false;
+
+      readLockTheFile();
+      try {
+        success = createSnapshot(seqFiles, snapshotDir.getName()) || success;
+        success = createSnapshot(unseqFiles, snapshotDir.getName()) || success;
+      } finally {
+        readUnlockTheFile();
+      }
+
+      LOGGER.info(
+          "Successfully take snapshot for {}-{}, snapshot directory is {}",
+          dataRegion.getStorageGroupName(),
+          dataRegion.getDataRegionId(),
+          snapshotDirPath);
+
+      return success;
+    } catch (Exception e) {
+      LOGGER.error(
+          "Exception occurs when taking snapshot for {}-{}",
+          dataRegion.getStorageGroupName(),
+          dataRegion.getDataRegionId(),
+          e);
+      return false;
+    } finally {
+      try {
+        snapshotLogger.close();
+      } catch (Exception e) {
+        LOGGER.error("Failed to close snapshot logger", e);
+      }
+    }
+  }
+
+  private void readLockTheFile() {
     TsFileManager manager = dataRegion.getTsFileManager();
     manager.readLock();
     try {
-      for (Long timePartition : timePartitions) {
-        List<String> seqDataDirs = getAllDataDirOfOnePartition(true, timePartition);
-
-        try {
-          createFileSnapshot(seqDataDirs, true, timePartition);
-        } catch (IOException e) {
-          LOGGER.error("Fail to create snapshot", e);
-          cleanUpWhenFail(snapshotDir);
-          return false;
-        }
-
-        List<String> unseqDataDirs = getAllDataDirOfOnePartition(false, timePartition);
-
-        try {
-          createFileSnapshot(unseqDataDirs, false, timePartition);
-        } catch (IOException e) {
-          LOGGER.error("Fail to create snapshot", e);
-          cleanUpWhenFail(snapshotDir);
-          return false;
-        }
+      seqFiles = manager.getTsFileList(true);
+      unseqFiles = manager.getTsFileList(false);
+      for (TsFileResource resource : seqFiles) {
+        resource.readLock();
+      }
+      for (TsFileResource resource : unseqFiles) {
+        resource.readLock();
       }
     } finally {
       manager.readUnlock();
     }
-
-    LOGGER.info(
-        "Successfully take snapshot for {}-{}, snapshot directory is {}",
-        dataRegion.getLogicalStorageGroupName(),
-        dataRegion.getDataRegionId(),
-        snapshotDirPath);
-
-    return true;
   }
 
-  private List<String> getAllDataDirOfOnePartition(boolean sequence, long timePartition) {
-    String[] dataDirs = IoTDBDescriptor.getInstance().getConfig().getDataDirs();
-    List<String> resultDirs = new LinkedList<>();
-
-    for (String dataDir : dataDirs) {
-      resultDirs.add(
-          dataDir
-              + File.separator
-              + (sequence
-                  ? IoTDBConstant.SEQUENCE_FLODER_NAME
-                  : IoTDBConstant.UNSEQUENCE_FLODER_NAME)
-              + File.separator
-              + dataRegion.getLogicalStorageGroupName()
-              + File.separator
-              + dataRegion.getDataRegionId()
-              + File.separator
-              + timePartition
-              + File.separator);
+  private void readUnlockTheFile() {
+    for (TsFileResource resource : seqFiles) {
+      resource.readUnlock();
+    }
+    for (TsFileResource resource : unseqFiles) {
+      resource.readUnlock();
     }
-    return resultDirs;
   }
 
-  private void createFileSnapshot(List<String> sourceDirPaths, boolean sequence, long timePartition)
+  private boolean createSnapshot(List<TsFileResource> resources, String snapshotId)
       throws IOException {
-    File timePartitionDir =
-        new File(sequence ? seqBaseDir : unseqBaseDir, String.valueOf(timePartition));
-    if (!timePartitionDir.exists() && !timePartitionDir.mkdirs()) {
-      throw new IOException(
-          String.format("%s not exists and cannot create it", timePartitionDir.getAbsolutePath()));
-    }
-
-    for (String sourceDirPath : sourceDirPaths) {
-      File sourceDir = new File(sourceDirPath);
-      if (!sourceDir.exists()) {
-        continue;
-      }
-      // Collect TsFile, TsFileResource, Mods, CompactionMods
-      File[] files =
-          sourceDir.listFiles(
-              (dir, name) ->
-                  name.endsWith(".tsfile")
-                      || name.endsWith(TsFileResource.RESOURCE_SUFFIX)
-                      || name.endsWith(ModificationFile.FILE_SUFFIX)
-                      || name.endsWith(ModificationFile.COMPACTION_FILE_SUFFIX)
-                      || name.endsWith(CompactionLogger.INNER_COMPACTION_LOG_NAME_SUFFIX)
-                      || name.endsWith(CompactionLogger.CROSS_COMPACTION_LOG_NAME_SUFFIX)
-                      || name.endsWith(IoTDBConstant.INNER_COMPACTION_TMP_FILE_SUFFIX)
-                      || name.endsWith(IoTDBConstant.CROSS_COMPACTION_TMP_FILE_SUFFIX));
-      if (files == null || files.length == 0) {
-        continue;
-      }
-
-      for (File file : files) {
-        File linkFile = new File(timePartitionDir, file.getName());
-        Files.createLink(linkFile.toPath(), file.toPath());
+    for (TsFileResource resource : resources) {
+      File tsFile = resource.getTsFile();
+      File dir = getSnapshotDirDirForResource(tsFile, snapshotId);
+      // create hard link for tsfile, resource, mods, compaction mods
+      createHardLink(new File(dir, tsFile.getName()), tsFile);
+      createHardLink(
+          new File(dir, tsFile.getName() + TsFileResource.RESOURCE_SUFFIX),
+          new File(tsFile.getAbsolutePath() + TsFileResource.RESOURCE_SUFFIX));
+      if (resource.getModFile().exists()) {
+        createHardLink(
+            new File(dir, tsFile.getName() + ModificationFile.FILE_SUFFIX),
+            new File(tsFile.getAbsolutePath() + ModificationFile.FILE_SUFFIX));
       }
     }
+    return true;
   }
 
-  private void cleanUpWhenFail(File snapshotDir) {
-    File[] files = snapshotDir.listFiles();
-    if (files != null) {
-      for (File file : files) {
-        if (!file.delete()) {
-          LOGGER.error("Failed to delete link file {} after failing to create snapshot", file);
-        }
-      }
+  private void createHardLink(File target, File source) throws IOException {
+    Files.createLink(target.toPath(), source.toPath());
+    snapshotLogger.logFile(source.getAbsolutePath(), target.getAbsolutePath());
+  }
+
+  private File getSnapshotDirDirForResource(File tsFile, String snapshotId) throws IOException {

Review Comment:
   dirdir？



##########
server/src/main/java/org/apache/iotdb/db/engine/DataFileConstant.java:
##########
@@ -0,0 +1,52 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iotdb.db.engine;
+
+import org.apache.iotdb.commons.conf.IoTDBConstant;
+import org.apache.iotdb.db.engine.compaction.log.CompactionLogger;
+import org.apache.iotdb.db.engine.modification.ModificationFile;
+import org.apache.iotdb.db.engine.storagegroup.TsFileResource;
+import org.apache.iotdb.tsfile.common.constant.TsFileConstant;
+
+import java.io.File;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Set;
+
+public class DataFileConstant {
+  public static final String[] DATA_FILE_SUFFIX =

Review Comment:
   Only contain tsfile, resource, mods



##########
server/src/main/java/org/apache/iotdb/db/engine/snapshot/SnapshotTaker.java:
##########
@@ -89,111 +70,131 @@ public boolean takeFullSnapshot(String snapshotDirPath, boolean flushBeforeSnaps
       dataRegion.syncCloseAllWorkingTsFileProcessors();
     }
 
-    List<Long> timePartitions = dataRegion.getTimePartitions();
+    File snapshotLog = new File(snapshotDir, SnapshotLogger.SNAPSHOT_LOG_NAME);
+    try {
+      snapshotLogger = new SnapshotLogger(snapshotLog);
+      boolean success = false;
+
+      readLockTheFile();
+      try {
+        success = createSnapshot(seqFiles, snapshotDir.getName()) || success;
+        success = createSnapshot(unseqFiles, snapshotDir.getName()) || success;
+      } finally {
+        readUnlockTheFile();
+      }
+
+      LOGGER.info(
+          "Successfully take snapshot for {}-{}, snapshot directory is {}",
+          dataRegion.getStorageGroupName(),
+          dataRegion.getDataRegionId(),
+          snapshotDirPath);
+
+      return success;
+    } catch (Exception e) {
+      LOGGER.error(
+          "Exception occurs when taking snapshot for {}-{}",
+          dataRegion.getStorageGroupName(),
+          dataRegion.getDataRegionId(),
+          e);
+      return false;
+    } finally {
+      try {
+        snapshotLogger.close();
+      } catch (Exception e) {
+        LOGGER.error("Failed to close snapshot logger", e);
+      }
+    }
+  }
+
+  private void readLockTheFile() {
     TsFileManager manager = dataRegion.getTsFileManager();
     manager.readLock();
     try {
-      for (Long timePartition : timePartitions) {
-        List<String> seqDataDirs = getAllDataDirOfOnePartition(true, timePartition);
-
-        try {
-          createFileSnapshot(seqDataDirs, true, timePartition);
-        } catch (IOException e) {
-          LOGGER.error("Fail to create snapshot", e);
-          cleanUpWhenFail(snapshotDir);
-          return false;
-        }
-
-        List<String> unseqDataDirs = getAllDataDirOfOnePartition(false, timePartition);
-
-        try {
-          createFileSnapshot(unseqDataDirs, false, timePartition);
-        } catch (IOException e) {
-          LOGGER.error("Fail to create snapshot", e);
-          cleanUpWhenFail(snapshotDir);
-          return false;
-        }
+      seqFiles = manager.getTsFileList(true);
+      unseqFiles = manager.getTsFileList(false);
+      for (TsFileResource resource : seqFiles) {
+        resource.readLock();
+      }
+      for (TsFileResource resource : unseqFiles) {
+        resource.readLock();
       }
     } finally {
       manager.readUnlock();
     }
-
-    LOGGER.info(
-        "Successfully take snapshot for {}-{}, snapshot directory is {}",
-        dataRegion.getLogicalStorageGroupName(),
-        dataRegion.getDataRegionId(),
-        snapshotDirPath);
-
-    return true;
   }
 
-  private List<String> getAllDataDirOfOnePartition(boolean sequence, long timePartition) {
-    String[] dataDirs = IoTDBDescriptor.getInstance().getConfig().getDataDirs();
-    List<String> resultDirs = new LinkedList<>();
-
-    for (String dataDir : dataDirs) {
-      resultDirs.add(
-          dataDir
-              + File.separator
-              + (sequence
-                  ? IoTDBConstant.SEQUENCE_FLODER_NAME
-                  : IoTDBConstant.UNSEQUENCE_FLODER_NAME)
-              + File.separator
-              + dataRegion.getLogicalStorageGroupName()
-              + File.separator
-              + dataRegion.getDataRegionId()
-              + File.separator
-              + timePartition
-              + File.separator);
+  private void readUnlockTheFile() {
+    for (TsFileResource resource : seqFiles) {
+      resource.readUnlock();
+    }
+    for (TsFileResource resource : unseqFiles) {
+      resource.readUnlock();
     }
-    return resultDirs;
   }
 
-  private void createFileSnapshot(List<String> sourceDirPaths, boolean sequence, long timePartition)
+  private boolean createSnapshot(List<TsFileResource> resources, String snapshotId)
       throws IOException {
-    File timePartitionDir =
-        new File(sequence ? seqBaseDir : unseqBaseDir, String.valueOf(timePartition));
-    if (!timePartitionDir.exists() && !timePartitionDir.mkdirs()) {
-      throw new IOException(
-          String.format("%s not exists and cannot create it", timePartitionDir.getAbsolutePath()));
-    }
-
-    for (String sourceDirPath : sourceDirPaths) {
-      File sourceDir = new File(sourceDirPath);
-      if (!sourceDir.exists()) {
-        continue;
-      }
-      // Collect TsFile, TsFileResource, Mods, CompactionMods
-      File[] files =
-          sourceDir.listFiles(
-              (dir, name) ->
-                  name.endsWith(".tsfile")
-                      || name.endsWith(TsFileResource.RESOURCE_SUFFIX)
-                      || name.endsWith(ModificationFile.FILE_SUFFIX)
-                      || name.endsWith(ModificationFile.COMPACTION_FILE_SUFFIX)
-                      || name.endsWith(CompactionLogger.INNER_COMPACTION_LOG_NAME_SUFFIX)
-                      || name.endsWith(CompactionLogger.CROSS_COMPACTION_LOG_NAME_SUFFIX)
-                      || name.endsWith(IoTDBConstant.INNER_COMPACTION_TMP_FILE_SUFFIX)
-                      || name.endsWith(IoTDBConstant.CROSS_COMPACTION_TMP_FILE_SUFFIX));
-      if (files == null || files.length == 0) {
-        continue;
-      }
-
-      for (File file : files) {
-        File linkFile = new File(timePartitionDir, file.getName());
-        Files.createLink(linkFile.toPath(), file.toPath());
+    for (TsFileResource resource : resources) {
+      File tsFile = resource.getTsFile();
+      File snapshotTsFile = getSnapshotFilePathForResource(tsFile, snapshotId);
+      // create hard link for tsfile, resource, mods, compaction mods

Review Comment:
   ```suggestion
         // create hard link for tsfile, resource, mods
   ```



##########
server/src/main/java/org/apache/iotdb/db/engine/snapshot/SnapshotTaker.java:
##########
@@ -89,111 +70,131 @@ public boolean takeFullSnapshot(String snapshotDirPath, boolean flushBeforeSnaps
       dataRegion.syncCloseAllWorkingTsFileProcessors();
     }
 
-    List<Long> timePartitions = dataRegion.getTimePartitions();
+    File snapshotLog = new File(snapshotDir, SnapshotLogger.SNAPSHOT_LOG_NAME);
+    try {
+      snapshotLogger = new SnapshotLogger(snapshotLog);
+      boolean success = false;
+
+      readLockTheFile();
+      try {
+        success = createSnapshot(seqFiles, snapshotDir.getName()) || success;
+        success = createSnapshot(unseqFiles, snapshotDir.getName()) || success;
+      } finally {
+        readUnlockTheFile();
+      }
+
+      LOGGER.info(
+          "Successfully take snapshot for {}-{}, snapshot directory is {}",
+          dataRegion.getStorageGroupName(),
+          dataRegion.getDataRegionId(),
+          snapshotDirPath);
+
+      return success;
+    } catch (Exception e) {
+      LOGGER.error(
+          "Exception occurs when taking snapshot for {}-{}",
+          dataRegion.getStorageGroupName(),
+          dataRegion.getDataRegionId(),
+          e);
+      return false;

Review Comment:
   could move to finally



##########
server/src/main/java/org/apache/iotdb/db/engine/snapshot/SnapshotTaker.java:
##########
@@ -89,111 +70,131 @@ public boolean takeFullSnapshot(String snapshotDirPath, boolean flushBeforeSnaps
       dataRegion.syncCloseAllWorkingTsFileProcessors();
     }
 
-    List<Long> timePartitions = dataRegion.getTimePartitions();
+    File snapshotLog = new File(snapshotDir, SnapshotLogger.SNAPSHOT_LOG_NAME);
+    try {
+      snapshotLogger = new SnapshotLogger(snapshotLog);
+      boolean success = false;
+
+      readLockTheFile();
+      try {
+        success = createSnapshot(seqFiles, snapshotDir.getName()) || success;
+        success = createSnapshot(unseqFiles, snapshotDir.getName()) || success;
+      } finally {
+        readUnlockTheFile();
+      }
+
+      LOGGER.info(
+          "Successfully take snapshot for {}-{}, snapshot directory is {}",

Review Comment:
   What if the createSnapshot fail?  You need to check the success then log



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

[GitHub] [iotdb] SzyWilliam commented on a diff in pull request #6782: [IOTDB-3771] Fix cannot take snapshot when the data dir and snapshot dir is on different disk

Posted by GitBox <gi...@apache.org>.

SzyWilliam commented on code in PR #6782:
URL: https://github.com/apache/iotdb/pull/6782#discussion_r930017615


##########
server/src/main/java/org/apache/iotdb/db/engine/snapshot/SnapshotTaker.java:
##########
@@ -117,83 +208,152 @@ public boolean takeFullSnapshot(String snapshotDirPath, boolean flushBeforeSnaps
     } finally {
       manager.readUnlock();
     }
+    return true;
+  }
+
+  private boolean createSnapshotInAnotherDisk(File snapshotDir) {
+    for (String dataDir : IoTDBDescriptor.getInstance().getConfig().getDataDirs()) {
+      File seqDir =
+          new File(
+              dataDir,
+              "sequence"
+                  + File.separator
+                  + dataRegion.getLogicalStorageGroupName()
+                  + File.separator
+                  + dataRegion.getDataRegionId());
+      File unseqDir =
+          new File(
+              dataDir,
+              "unsequence"
+                  + File.separator
+                  + dataRegion.getLogicalStorageGroupName()
+                  + File.separator
+                  + dataRegion.getDataRegionId());
+      File localSeqSnapshotDir =
+          new File(
+              dataDir,
+              "snapshot"
+                  + File.separator
+                  + "sequence"
+                  + File.separator
+                  + snapshotDir.getName()
+                  + File.separator
+                  + dataRegion.getLogicalStorageGroupName()
+                  + File.separator
+                  + dataRegion.getDataRegionId());
+      File localUnseqSnapshotDir =
+          new File(
+              dataDir,
+              "snapshot"
+                  + File.separator
+                  + "unsequence"
+                  + File.separator
+                  + snapshotDir.getName()
+                  + File.separator
+                  + dataRegion.getLogicalStorageGroupName()
+                  + File.separator
+                  + dataRegion.getDataRegionId());
+      if (!localSeqSnapshotDir.mkdirs()) {
+        LOGGER.warn("Failed to create local snapshot dir {}", localSeqSnapshotDir);
+        return false;
+      }
+      List<Long> timePartitions = dataRegion.getTimePartitions();
+      TsFileManager manager = dataRegion.getTsFileManager();
+      manager.readLock();
+      for (long timePartition : timePartitions) {
+        File seqTimePartitionDir = new File(seqDir, String.valueOf(timePartition));
+        File seqTimePartitionSnapshotDir =
+            new File(localSeqSnapshotDir, String.valueOf(timePartition));
 
-    LOGGER.info(
-        "Successfully take snapshot for {}-{}, snapshot directory is {}",
-        dataRegion.getLogicalStorageGroupName(),
-        dataRegion.getDataRegionId(),
-        snapshotDirPath);
+        try {
+          createFileSnapshotToTargetOne(seqTimePartitionDir, seqTimePartitionSnapshotDir);
+        } catch (IOException e) {
+          LOGGER.error(
+              "Failed to create snapshot for {}-{}",
+              dataRegion.getLogicalStorageGroupName(),
+              dataRegion.getDataRegionId(),
+              e);
+        } finally {
+          cleanUpWhenFail(snapshotDir);

Review Comment:
   cleanUpWhenFail should be called in catch clause instead of final clause?



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

[GitHub] [iotdb] coveralls commented on pull request #6782: [IOTDB-3771] Fix cannot take snapshot when the data dir and snapshot dir is on different disk

Posted by GitBox <gi...@apache.org>.

coveralls commented on PR #6782:
URL: https://github.com/apache/iotdb/pull/6782#issuecomment-1207970781

   
   [![Coverage Status](https://coveralls.io/builds/51487178/badge)](https://coveralls.io/builds/51487178)
   
   Coverage increased (+0.06%) to 42.68% when pulling **894104b29fe253947886ad6b3b699426aa26ad96 on IOTDB-3771** into **39db994138c878b9e4f055d4c62e56f9ceb86693 on master**.
   


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

[GitHub] [iotdb] qiaojialin commented on a diff in pull request #6782: [IOTDB-3771] Fix cannot take snapshot when the data dir and snapshot dir is on different disk

Posted by GitBox <gi...@apache.org>.

qiaojialin commented on code in PR #6782:
URL: https://github.com/apache/iotdb/pull/6782#discussion_r944328796


##########
server/src/main/java/org/apache/iotdb/db/engine/snapshot/SnapshotTaker.java:
##########
@@ -117,83 +210,154 @@ public boolean takeFullSnapshot(String snapshotDirPath, boolean flushBeforeSnaps
     } finally {
       manager.readUnlock();
     }
+    return true;
+  }
 
-    LOGGER.info(
-        "Successfully take snapshot for {}-{}, snapshot directory is {}",
-        dataRegion.getLogicalStorageGroupName(),
-        dataRegion.getDataRegionId(),
-        snapshotDirPath);
+  private boolean createSnapshotInAnotherFs(File snapshotDir) {
+    try {
+      snapshotLogger.logSnapshotType(SnapshotLogger.SnapshotType.REMOTE_FS);
+    } catch (IOException e) {
+      LOGGER.error("Fail to create snapshot", e);
+      cleanUpWhenFail(snapshotDir);
+      return false;
+    }
+    for (String dataDir : IoTDBDescriptor.getInstance().getConfig().getDataDirs()) {
+      File seqDir =
+          new File(
+              dataDir,
+              "sequence"
+                  + File.separator
+                  + dataRegion.getStorageGroupName()
+                  + File.separator
+                  + dataRegion.getDataRegionId());
+      File unseqDir =
+          new File(
+              dataDir,
+              "unsequence"
+                  + File.separator
+                  + dataRegion.getStorageGroupName()
+                  + File.separator
+                  + dataRegion.getDataRegionId());
+      File localSeqSnapshotDir =
+          new File(
+              dataDir,
+              "snapshot"
+                  + File.separator
+                  + "sequence"
+                  + File.separator
+                  + snapshotDir.getName()
+                  + File.separator
+                  + dataRegion.getStorageGroupName()
+                  + File.separator
+                  + dataRegion.getDataRegionId());
+      File localUnseqSnapshotDir =
+          new File(
+              dataDir,
+              "snapshot"
+                  + File.separator
+                  + "unsequence"
+                  + File.separator
+                  + snapshotDir.getName()
+                  + File.separator
+                  + dataRegion.getStorageGroupName()
+                  + File.separator
+                  + dataRegion.getDataRegionId());
+      if (!localSeqSnapshotDir.mkdirs()) {
+        LOGGER.warn("Failed to create local snapshot dir {}", localSeqSnapshotDir);
+        return false;
+      }
+      List<Long> timePartitions = dataRegion.getTimePartitions();
+      TsFileManager manager = dataRegion.getTsFileManager();
+      manager.readLock();
+      for (long timePartition : timePartitions) {
+        createSnapshotForTimePartition(snapshotDir, seqDir, localSeqSnapshotDir, timePartition);
+        createSnapshotForTimePartition(snapshotDir, unseqDir, localUnseqSnapshotDir, timePartition);
+      }
+    }
 
     return true;
   }
 
-  private List<String> getAllDataDirOfOnePartition(boolean sequence, long timePartition) {
-    String[] dataDirs = IoTDBDescriptor.getInstance().getConfig().getDataDirs();
-    List<String> resultDirs = new LinkedList<>();
+  private void createSnapshotForTimePartition(
+      File snapshotDir, File dataDir, File localSeqSnapshotDir, long timePartition) {
+    File seqTimePartitionDir = new File(dataDir, String.valueOf(timePartition));
+    File seqTimePartitionSnapshotDir = new File(localSeqSnapshotDir, String.valueOf(timePartition));
 
-    for (String dataDir : dataDirs) {
-      resultDirs.add(
-          dataDir
-              + File.separator
-              + (sequence
-                  ? IoTDBConstant.SEQUENCE_FLODER_NAME
-                  : IoTDBConstant.UNSEQUENCE_FLODER_NAME)
-              + File.separator
-              + dataRegion.getLogicalStorageGroupName()
-              + File.separator
-              + dataRegion.getDataRegionId()
-              + File.separator
-              + timePartition
-              + File.separator);
+    try {
+      createFileSnapshotToTargetOne(seqTimePartitionDir, seqTimePartitionSnapshotDir);
+    } catch (IOException e) {
+      LOGGER.error(
+          "Failed to create snapshot for {}-{}",
+          dataRegion.getStorageGroupName(),
+          dataRegion.getDataRegionId(),
+          e);
+      cleanUpWhenFail(snapshotDir);
     }
-    return resultDirs;
   }
 
-  private void createFileSnapshot(List<String> sourceDirPaths, boolean sequence, long timePartition)
-      throws IOException {
-    File timePartitionDir =
-        new File(sequence ? seqBaseDir : unseqBaseDir, String.valueOf(timePartition));
-    if (!timePartitionDir.exists() && !timePartitionDir.mkdirs()) {
-      throw new IOException(
-          String.format("%s not exists and cannot create it", timePartitionDir.getAbsolutePath()));
+  private void createFileSnapshotToTargetOne(File sourceDir, File targetDir) throws IOException {
+    File[] files =
+        sourceDir.listFiles(
+            (dir, name) ->
+                name.endsWith(TsFileConstant.TSFILE_SUFFIX)
+                    || name.endsWith(TsFileResource.RESOURCE_SUFFIX)
+                    || name.endsWith(ModificationFile.FILE_SUFFIX)
+                    || name.endsWith(ModificationFile.COMPACTION_FILE_SUFFIX)
+                    || name.endsWith(CompactionLogger.INNER_COMPACTION_LOG_NAME_SUFFIX)
+                    || name.endsWith(CompactionLogger.CROSS_COMPACTION_LOG_NAME_SUFFIX)
+                    || name.endsWith(IoTDBConstant.INNER_COMPACTION_TMP_FILE_SUFFIX)
+                    || name.endsWith(IoTDBConstant.CROSS_COMPACTION_TMP_FILE_SUFFIX));

Review Comment:
   move to a unified class: StorageEngineFile



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

[GitHub] [iotdb] sonarcloud[bot] commented on pull request #6782: [IOTDB-3771] Fix cannot take snapshot when the data dir and snapshot dir is on different disk

Posted by GitBox <gi...@apache.org>.

sonarcloud[bot] commented on PR #6782:
URL: https://github.com/apache/iotdb/pull/6782#issuecomment-1215127509

   SonarCloud Quality Gate failed.&nbsp; &nbsp; [![Quality Gate failed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/failed-16px.png 'Quality Gate failed')](https://sonarcloud.io/dashboard?id=apache_incubator-iotdb&pullRequest=6782)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=BUG) [![B](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/B-16px.png 'B')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=BUG) [1 Bug](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=CODE_SMELL) [39 Code Smells](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=CODE_SMELL)
   
   [![52.4%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/50-16px.png '52.4%')](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=6782&metric=new_coverage&view=list) [52.4% Coverage](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=6782&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=6782&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=6782&metric=new_duplicated_lines_density&view=list)
   
   


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

[GitHub] [iotdb] THUMarkLau commented on a diff in pull request #6782: [IOTDB-3771] Fix cannot take snapshot when the data dir and snapshot dir is on different disk

Posted by GitBox <gi...@apache.org>.

THUMarkLau commented on code in PR #6782:
URL: https://github.com/apache/iotdb/pull/6782#discussion_r947673159


##########
server/src/main/java/org/apache/iotdb/db/engine/snapshot/SnapshotTaker.java:
##########
@@ -89,111 +70,131 @@ public boolean takeFullSnapshot(String snapshotDirPath, boolean flushBeforeSnaps
       dataRegion.syncCloseAllWorkingTsFileProcessors();
     }
 
-    List<Long> timePartitions = dataRegion.getTimePartitions();
+    File snapshotLog = new File(snapshotDir, SnapshotLogger.SNAPSHOT_LOG_NAME);
+    try {
+      snapshotLogger = new SnapshotLogger(snapshotLog);
+      boolean success = false;
+
+      readLockTheFile();
+      try {
+        success = createSnapshot(seqFiles, snapshotDir.getName()) || success;
+        success = createSnapshot(unseqFiles, snapshotDir.getName()) || success;
+      } finally {
+        readUnlockTheFile();
+      }
+
+      LOGGER.info(
+          "Successfully take snapshot for {}-{}, snapshot directory is {}",

Review Comment:
   resolve



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

[GitHub] [iotdb] qiaojialin merged pull request #6782: [IOTDB-3771] Fix cannot take snapshot when the data dir and snapshot dir is on different disk

Posted by GitBox <gi...@apache.org>.

qiaojialin merged PR #6782:
URL: https://github.com/apache/iotdb/pull/6782


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

[GitHub] [iotdb] THUMarkLau commented on a diff in pull request #6782: [IOTDB-3771] Fix cannot take snapshot when the data dir and snapshot dir is on different disk

Posted by GitBox <gi...@apache.org>.

THUMarkLau commented on code in PR #6782:
URL: https://github.com/apache/iotdb/pull/6782#discussion_r939766582


##########
server/src/main/java/org/apache/iotdb/db/engine/snapshot/SnapshotTaker.java:
##########
@@ -117,83 +208,150 @@ public boolean takeFullSnapshot(String snapshotDirPath, boolean flushBeforeSnaps
     } finally {
       manager.readUnlock();
     }
+    return true;
+  }
+
+  private boolean createSnapshotInAnotherDisk(File snapshotDir) {
+    for (String dataDir : IoTDBDescriptor.getInstance().getConfig().getDataDirs()) {
+      File seqDir =
+          new File(
+              dataDir,
+              "sequence"
+                  + File.separator
+                  + dataRegion.getStorageGroupName()
+                  + File.separator
+                  + dataRegion.getDataRegionId());

Review Comment:
   The core of these two functions to create hard links steps are the same, and this step has been extracted as function `createFileSnapshotToTargetOne`. The difference of `createSnapshotInLocalDisk` and `createSnapshotInRemoteDisk` is about how to traverse the data directory and create snapshot dir in local data dirs.



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

[GitHub] [iotdb] sonarcloud[bot] commented on pull request #6782: [IOTDB-3771] Fix cannot take snapshot when the data dir and snapshot dir is on different disk

Posted by GitBox <gi...@apache.org>.

sonarcloud[bot] commented on PR #6782:
URL: https://github.com/apache/iotdb/pull/6782#issuecomment-1210955825

   SonarCloud Quality Gate failed.&nbsp; &nbsp; [![Quality Gate failed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/failed-16px.png 'Quality Gate failed')](https://sonarcloud.io/dashboard?id=apache_incubator-iotdb&pullRequest=6782)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=BUG) [![B](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/B-16px.png 'B')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=BUG) [1 Bug](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=CODE_SMELL) [37 Code Smells](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=CODE_SMELL)
   
   [![50.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/50-16px.png '50.0%')](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=6782&metric=new_coverage&view=list) [50.0% Coverage](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=6782&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=6782&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=6782&metric=new_duplicated_lines_density&view=list)
   
   


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

[GitHub] [iotdb] THUMarkLau commented on a diff in pull request #6782: [IOTDB-3771] Fix cannot take snapshot when the data dir and snapshot dir is on different disk

Posted by GitBox <gi...@apache.org>.

THUMarkLau commented on code in PR #6782:
URL: https://github.com/apache/iotdb/pull/6782#discussion_r939766140


##########
server/src/main/java/org/apache/iotdb/db/engine/snapshot/SnapshotLogger.java:
##########
@@ -0,0 +1,69 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iotdb.db.engine.snapshot;
+
+import java.io.BufferedOutputStream;
+import java.io.File;
+import java.io.FileNotFoundException;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+
+public class SnapshotLogger implements AutoCloseable {
+  public static final String SNAPSHOT_LOG_NAME = "snapshot.log";
+  public static final String SPLIT_CHAR = "#";
+
+  private File logFile;
+  private BufferedOutputStream os;
+
+  public SnapshotLogger(File logFile) throws FileNotFoundException {
+    this.logFile = logFile;
+    os = new BufferedOutputStream(new FileOutputStream(logFile));
+  }
+
+  @Override
+  public void close() throws Exception {
+    os.close();
+  }
+
+  public void logFile(String sourceFile, String linkFile) throws IOException {
+    os.write(sourceFile.getBytes(StandardCharsets.UTF_8));
+    os.write(SPLIT_CHAR.getBytes(StandardCharsets.UTF_8));
+    os.write(linkFile.getBytes(StandardCharsets.UTF_8));
+    os.write("\n".getBytes(StandardCharsets.UTF_8));
+    os.flush();
+  }
+
+  public void logSnapshotType(SnapshotType type) throws IOException {
+    os.write(String.valueOf(type).getBytes(StandardCharsets.UTF_8));
+    os.write("\n".getBytes(StandardCharsets.UTF_8));
+    os.flush();
+  }
+
+  public void cleanUpWhenFailed() throws IOException {
+    os.close();
+    Files.delete(logFile.toPath());
+  }
+
+  public static enum SnapshotType {
+    LOCAL_DISK,
+    REMOTE_DISK

Review Comment:
   It is logged when taking snapshot in different disk.



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

[GitHub] [iotdb] sonarcloud[bot] commented on pull request #6782: [IOTDB-3771] Fix cannot take snapshot when the data dir and snapshot dir is on different disk

Posted by GitBox <gi...@apache.org>.

sonarcloud[bot] commented on PR #6782:
URL: https://github.com/apache/iotdb/pull/6782#issuecomment-1208239257

   SonarCloud Quality Gate failed.&nbsp; &nbsp; [![Quality Gate failed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/failed-16px.png 'Quality Gate failed')](https://sonarcloud.io/dashboard?id=apache_incubator-iotdb&pullRequest=6782)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=BUG) [![B](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/B-16px.png 'B')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=BUG) [1 Bug](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=CODE_SMELL) [33 Code Smells](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=CODE_SMELL)
   
   [![56.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/50-16px.png '56.0%')](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=6782&metric=new_coverage&view=list) [56.0% Coverage](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=6782&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=6782&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=6782&metric=new_duplicated_lines_density&view=list)
   
   


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

[GitHub] [iotdb] sonarcloud[bot] commented on pull request #6782: [IOTDB-3771] Fix cannot take snapshot when the data dir and snapshot dir is on different disk

Posted by GitBox <gi...@apache.org>.

sonarcloud[bot] commented on PR #6782:
URL: https://github.com/apache/iotdb/pull/6782#issuecomment-1207975866

   SonarCloud Quality Gate failed.&nbsp; &nbsp; [![Quality Gate failed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/failed-16px.png 'Quality Gate failed')](https://sonarcloud.io/dashboard?id=apache_incubator-iotdb&pullRequest=6782)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=BUG) [![B](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/B-16px.png 'B')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=BUG) [1 Bug](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=CODE_SMELL) [32 Code Smells](https://sonarcloud.io/project/issues?id=apache_incubator-iotdb&pullRequest=6782&resolved=false&types=CODE_SMELL)
   
   [![56.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/50-16px.png '56.0%')](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=6782&metric=new_coverage&view=list) [56.0% Coverage](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=6782&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=6782&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_incubator-iotdb&pullRequest=6782&metric=new_duplicated_lines_density&view=list)
   
   


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

[GitHub] [iotdb] THUMarkLau commented on a diff in pull request #6782: [IOTDB-3771] Fix cannot take snapshot when the data dir and snapshot dir is on different disk

Posted by GitBox <gi...@apache.org>.

THUMarkLau commented on code in PR #6782:
URL: https://github.com/apache/iotdb/pull/6782#discussion_r947658850


##########
server/src/main/java/org/apache/iotdb/db/engine/storagegroup/TsFileResource.java:
##########
@@ -1005,4 +1005,8 @@ private void generatePathToTimeSeriesMetadataMap() throws IOException {
   public boolean isFileInList() {
     return prev != null || next != null;
   }
+
+  public boolean createHardLink(String path) {

Review Comment:
   resolve



##########
server/src/main/java/org/apache/iotdb/db/engine/DataFileConstant.java:
##########
@@ -0,0 +1,52 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iotdb.db.engine;
+
+import org.apache.iotdb.commons.conf.IoTDBConstant;
+import org.apache.iotdb.db.engine.compaction.log.CompactionLogger;
+import org.apache.iotdb.db.engine.modification.ModificationFile;
+import org.apache.iotdb.db.engine.storagegroup.TsFileResource;
+import org.apache.iotdb.tsfile.common.constant.TsFileConstant;
+
+import java.io.File;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Set;
+
+public class DataFileConstant {
+  public static final String[] DATA_FILE_SUFFIX =

Review Comment:
   resolve



##########
server/src/main/java/org/apache/iotdb/db/engine/snapshot/SnapshotTaker.java:
##########
@@ -89,111 +70,114 @@ public boolean takeFullSnapshot(String snapshotDirPath, boolean flushBeforeSnaps
       dataRegion.syncCloseAllWorkingTsFileProcessors();
     }
 
-    List<Long> timePartitions = dataRegion.getTimePartitions();
+    File snapshotLog = new File(snapshotDir, SnapshotLogger.SNAPSHOT_LOG_NAME);
+    try {
+      snapshotLogger = new SnapshotLogger(snapshotLog);
+      boolean success = false;
+
+      readLockTheFile();
+      try {
+        success = createSnapshot(seqFiles, snapshotDir.getName()) || success;
+        success = createSnapshot(unseqFiles, snapshotDir.getName()) || success;
+      } finally {
+        readUnlockTheFile();
+      }
+
+      LOGGER.info(
+          "Successfully take snapshot for {}-{}, snapshot directory is {}",
+          dataRegion.getStorageGroupName(),
+          dataRegion.getDataRegionId(),
+          snapshotDirPath);
+
+      return success;
+    } catch (Exception e) {
+      LOGGER.error(
+          "Exception occurs when taking snapshot for {}-{}",
+          dataRegion.getStorageGroupName(),
+          dataRegion.getDataRegionId(),
+          e);
+      return false;
+    } finally {
+      try {
+        snapshotLogger.close();
+      } catch (Exception e) {
+        LOGGER.error("Failed to close snapshot logger", e);
+      }
+    }
+  }
+
+  private void readLockTheFile() {
     TsFileManager manager = dataRegion.getTsFileManager();
     manager.readLock();
     try {
-      for (Long timePartition : timePartitions) {
-        List<String> seqDataDirs = getAllDataDirOfOnePartition(true, timePartition);
-
-        try {
-          createFileSnapshot(seqDataDirs, true, timePartition);
-        } catch (IOException e) {
-          LOGGER.error("Fail to create snapshot", e);
-          cleanUpWhenFail(snapshotDir);
-          return false;
-        }
-
-        List<String> unseqDataDirs = getAllDataDirOfOnePartition(false, timePartition);
-
-        try {
-          createFileSnapshot(unseqDataDirs, false, timePartition);
-        } catch (IOException e) {
-          LOGGER.error("Fail to create snapshot", e);
-          cleanUpWhenFail(snapshotDir);
-          return false;
-        }
+      seqFiles = manager.getTsFileList(true);
+      unseqFiles = manager.getTsFileList(false);
+      for (TsFileResource resource : seqFiles) {
+        resource.readLock();
+      }
+      for (TsFileResource resource : unseqFiles) {
+        resource.readLock();
       }
     } finally {
       manager.readUnlock();
     }
-
-    LOGGER.info(
-        "Successfully take snapshot for {}-{}, snapshot directory is {}",
-        dataRegion.getLogicalStorageGroupName(),
-        dataRegion.getDataRegionId(),
-        snapshotDirPath);
-
-    return true;
   }
 
-  private List<String> getAllDataDirOfOnePartition(boolean sequence, long timePartition) {
-    String[] dataDirs = IoTDBDescriptor.getInstance().getConfig().getDataDirs();
-    List<String> resultDirs = new LinkedList<>();
-
-    for (String dataDir : dataDirs) {
-      resultDirs.add(
-          dataDir
-              + File.separator
-              + (sequence
-                  ? IoTDBConstant.SEQUENCE_FLODER_NAME
-                  : IoTDBConstant.UNSEQUENCE_FLODER_NAME)
-              + File.separator
-              + dataRegion.getLogicalStorageGroupName()
-              + File.separator
-              + dataRegion.getDataRegionId()
-              + File.separator
-              + timePartition
-              + File.separator);
+  private void readUnlockTheFile() {
+    for (TsFileResource resource : seqFiles) {
+      resource.readUnlock();
+    }
+    for (TsFileResource resource : unseqFiles) {
+      resource.readUnlock();
     }
-    return resultDirs;
   }
 
-  private void createFileSnapshot(List<String> sourceDirPaths, boolean sequence, long timePartition)
+  private boolean createSnapshot(List<TsFileResource> resources, String snapshotId)
       throws IOException {
-    File timePartitionDir =
-        new File(sequence ? seqBaseDir : unseqBaseDir, String.valueOf(timePartition));
-    if (!timePartitionDir.exists() && !timePartitionDir.mkdirs()) {
-      throw new IOException(
-          String.format("%s not exists and cannot create it", timePartitionDir.getAbsolutePath()));
-    }
-
-    for (String sourceDirPath : sourceDirPaths) {
-      File sourceDir = new File(sourceDirPath);
-      if (!sourceDir.exists()) {
-        continue;
-      }
-      // Collect TsFile, TsFileResource, Mods, CompactionMods
-      File[] files =
-          sourceDir.listFiles(
-              (dir, name) ->
-                  name.endsWith(".tsfile")
-                      || name.endsWith(TsFileResource.RESOURCE_SUFFIX)
-                      || name.endsWith(ModificationFile.FILE_SUFFIX)
-                      || name.endsWith(ModificationFile.COMPACTION_FILE_SUFFIX)
-                      || name.endsWith(CompactionLogger.INNER_COMPACTION_LOG_NAME_SUFFIX)
-                      || name.endsWith(CompactionLogger.CROSS_COMPACTION_LOG_NAME_SUFFIX)
-                      || name.endsWith(IoTDBConstant.INNER_COMPACTION_TMP_FILE_SUFFIX)
-                      || name.endsWith(IoTDBConstant.CROSS_COMPACTION_TMP_FILE_SUFFIX));
-      if (files == null || files.length == 0) {
-        continue;
-      }
-
-      for (File file : files) {
-        File linkFile = new File(timePartitionDir, file.getName());
-        Files.createLink(linkFile.toPath(), file.toPath());
+    for (TsFileResource resource : resources) {
+      File tsFile = resource.getTsFile();
+      File dir = getSnapshotDirDirForResource(tsFile, snapshotId);
+      // create hard link for tsfile, resource, mods, compaction mods
+      createHardLink(new File(dir, tsFile.getName()), tsFile);
+      createHardLink(
+          new File(dir, tsFile.getName() + TsFileResource.RESOURCE_SUFFIX),
+          new File(tsFile.getAbsolutePath() + TsFileResource.RESOURCE_SUFFIX));
+      if (resource.getModFile().exists()) {
+        createHardLink(
+            new File(dir, tsFile.getName() + ModificationFile.FILE_SUFFIX),
+            new File(tsFile.getAbsolutePath() + ModificationFile.FILE_SUFFIX));
       }
     }
+    return true;
   }
 
-  private void cleanUpWhenFail(File snapshotDir) {
-    File[] files = snapshotDir.listFiles();
-    if (files != null) {
-      for (File file : files) {
-        if (!file.delete()) {
-          LOGGER.error("Failed to delete link file {} after failing to create snapshot", file);
-        }
-      }
+  private void createHardLink(File target, File source) throws IOException {
+    Files.createLink(target.toPath(), source.toPath());
+    snapshotLogger.logFile(source.getAbsolutePath(), target.getAbsolutePath());
+  }
+
+  private File getSnapshotDirDirForResource(File tsFile, String snapshotId) throws IOException {

Review Comment:
   resolve



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

[GitHub] [iotdb] THUMarkLau commented on a diff in pull request #6782: [IOTDB-3771] Fix cannot take snapshot when the data dir and snapshot dir is on different disk

Posted by GitBox <gi...@apache.org>.

THUMarkLau commented on code in PR #6782:
URL: https://github.com/apache/iotdb/pull/6782#discussion_r947838165


##########
server/src/main/java/org/apache/iotdb/db/engine/snapshot/SnapshotTaker.java:
##########
@@ -89,111 +70,131 @@ public boolean takeFullSnapshot(String snapshotDirPath, boolean flushBeforeSnaps
       dataRegion.syncCloseAllWorkingTsFileProcessors();
     }
 
-    List<Long> timePartitions = dataRegion.getTimePartitions();
+    File snapshotLog = new File(snapshotDir, SnapshotLogger.SNAPSHOT_LOG_NAME);
+    try {
+      snapshotLogger = new SnapshotLogger(snapshotLog);
+      boolean success = false;
+
+      readLockTheFile();
+      try {
+        success = createSnapshot(seqFiles, snapshotDir.getName()) || success;
+        success = createSnapshot(unseqFiles, snapshotDir.getName()) || success;
+      } finally {
+        readUnlockTheFile();
+      }
+
+      LOGGER.info(
+          "Successfully take snapshot for {}-{}, snapshot directory is {}",
+          dataRegion.getStorageGroupName(),
+          dataRegion.getDataRegionId(),
+          snapshotDirPath);
+
+      return success;

Review Comment:
   IDEA hints that it is bad to move a return into a finally block.



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