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/10/07 07:09:47 UTC

[GitHub] [iotdb] THUMarkLau opened a new pull request, #7535: [IOTDB-4205] Use logical path to take place of absolute path in snapshot log

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

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


-- 
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 #7535: [IOTDB-4205] Use logical path to take place of absolute path in snapshot log

Posted by GitBox <gi...@apache.org>.
THUMarkLau commented on code in PR #7535:
URL: https://github.com/apache/iotdb/pull/7535#discussion_r992265044


##########
server/src/main/java/org/apache/iotdb/db/engine/snapshot/SnapshotLogAnalyzer.java:
##########
@@ -55,25 +55,47 @@ public boolean hasNext() {
     }
   }
 
+  public String getSnapshotId() {
+    return snapshotId;
+  }
+
   /**
-   * @return The next pair of files recorded in the log. The left one is the path of source file,
-   *     the right one is the path of target file
+   * Return the total num of file in this snapshot.
+   *
+   * @return
    */
-  public Pair<String, String> getNextPairs() {
-    if (reader == null) {
-      return null;
+  public int getTotalFileCountInSnapshot() throws IOException {
+    reader.reset();
+    String currLine;
+    int cnt = 0;
+    while ((currLine = reader.readLine()) != null && !currLine.equals(SnapshotLogger.END_FLAG)) {
+      cnt++;
     }
-    try {
-      String fileInfo = reader.readLine();
-      String[] filesPath = fileInfo.split(SnapshotLogger.SPLIT_CHAR);
-      if (filesPath.length != 2) {
-        LOGGER.warn("Illegal file info: {} in snapshot log", fileInfo);
-        return null;
-      }
-      return new Pair<>(filesPath[0], filesPath[1]);
-    } catch (IOException e) {
-      LOGGER.error("Exception occurs when analyzing snapshot log", e);
-      return null;
+    return cnt;
+  }
+
+  /**
+   * Read the tail of the log file to see if the snapshot is complete.
+   *
+   * @return
+   */
+  public boolean isSnapshotComplete() throws IOException {
+    char[] endFlagInChar = new char[SnapshotLogger.END_FLAG.length()];
+    long fileLength = snapshotLogFile.length();
+    int endFlagLength = SnapshotLogger.END_FLAG.getBytes(StandardCharsets.UTF_8).length;
+    if (fileLength < endFlagLength) {
+      // this snapshot cannot be complete
+      return false;
     }
+    reader.mark((int) fileLength);
+    reader.skip(
+        (int)
+            (fileLength - endFlagLength - snapshotId.getBytes(StandardCharsets.UTF_8).length - 1));

Review Comment:
   1 means the '\n' in the first line



-- 
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 #7535: [IOTDB-4205] Use logical path to take place of absolute path in snapshot log

Posted by GitBox <gi...@apache.org>.
SzyWilliam commented on code in PR #7535:
URL: https://github.com/apache/iotdb/pull/7535#discussion_r991428088


##########
server/src/main/java/org/apache/iotdb/db/engine/snapshot/SnapshotLoader.java:
##########
@@ -361,24 +429,44 @@ public List<File> getSnapshotFileInfo() throws IOException {
   private List<File> getSnapshotFileWithLog(File logFile) throws IOException {
     SnapshotLogAnalyzer analyzer = new SnapshotLogAnalyzer(logFile);
     try {
-
+      String snapshotId = analyzer.getSnapshotId();
+      String[] dataDirs = IoTDBDescriptor.getInstance().getConfig().getDataDirs();
       List<File> fileList = new LinkedList<>();
-
-      while (analyzer.hasNext()) {
-        fileList.add(new File(analyzer.getNextPairs().right));
+      for (String dataDir : dataDirs) {
+        String snapshotDir =
+            dataDir
+                + File.separator
+                + IoTDBConstant.SNAPSHOT_FOLDER_NAME
+                + File.separator
+                + snapshotId;
+        fileList.addAll(searchDataFilesRecursively(snapshotDir));
       }
-
       return fileList;
     } finally {
       analyzer.close();
     }
   }
 
-  private List<File> getSnapshotFileWithoutLog() {
-    return new LinkedList<>(
-        Arrays.asList(
-            Objects.requireNonNull(
-                new File(snapshotPath)
-                    .listFiles((dir, name) -> SnapshotFileSet.isDataFile(new File(dir, name))))));
+  /**
+   * Search all data files in one directory recursively.
+   *
+   * @return
+   */
+  private List<File> searchDataFilesRecursively(String dir) {

Review Comment:
   We can use [Files.walkFileTree](https://docs.oracle.com/javase/7/docs/api/java/nio/file/Files.html#walkFileTree(java.nio.file.Path,%20java.nio.file.FileVisitor)) here.



-- 
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] xingtanzjr commented on a diff in pull request #7535: [IOTDB-4205] Use logical path to take place of absolute path in snapshot log

Posted by GitBox <gi...@apache.org>.
xingtanzjr commented on code in PR #7535:
URL: https://github.com/apache/iotdb/pull/7535#discussion_r991317015


##########
server/src/main/java/org/apache/iotdb/db/engine/snapshot/SnapshotLogAnalyzer.java:
##########
@@ -55,25 +55,47 @@ public boolean hasNext() {
     }
   }
 
+  public String getSnapshotId() {
+    return snapshotId;
+  }
+
   /**
-   * @return The next pair of files recorded in the log. The left one is the path of source file,
-   *     the right one is the path of target file
+   * Return the total num of file in this snapshot.
+   *
+   * @return
    */
-  public Pair<String, String> getNextPairs() {
-    if (reader == null) {
-      return null;
+  public int getTotalFileCountInSnapshot() throws IOException {
+    reader.reset();
+    String currLine;
+    int cnt = 0;
+    while ((currLine = reader.readLine()) != null && !currLine.equals(SnapshotLogger.END_FLAG)) {
+      cnt++;
     }
-    try {
-      String fileInfo = reader.readLine();
-      String[] filesPath = fileInfo.split(SnapshotLogger.SPLIT_CHAR);
-      if (filesPath.length != 2) {
-        LOGGER.warn("Illegal file info: {} in snapshot log", fileInfo);
-        return null;
-      }
-      return new Pair<>(filesPath[0], filesPath[1]);
-    } catch (IOException e) {
-      LOGGER.error("Exception occurs when analyzing snapshot log", e);
-      return null;
+    return cnt;
+  }
+
+  /**
+   * Read the tail of the log file to see if the snapshot is complete.
+   *
+   * @return
+   */
+  public boolean isSnapshotComplete() throws IOException {
+    char[] endFlagInChar = new char[SnapshotLogger.END_FLAG.length()];

Review Comment:
   And you can use some symbol inside the analyzer to mark the result of validation



-- 
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] xingtanzjr merged pull request #7535: [IOTDB-4205] Use logical path to take place of absolute path in snapshot log

Posted by GitBox <gi...@apache.org>.
xingtanzjr merged PR #7535:
URL: https://github.com/apache/iotdb/pull/7535


-- 
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] xingtanzjr commented on a diff in pull request #7535: [IOTDB-4205] Use logical path to take place of absolute path in snapshot log

Posted by GitBox <gi...@apache.org>.
xingtanzjr commented on code in PR #7535:
URL: https://github.com/apache/iotdb/pull/7535#discussion_r991329314


##########
server/src/main/java/org/apache/iotdb/db/engine/snapshot/SnapshotLoader.java:
##########
@@ -217,142 +224,203 @@ private void deleteAllFilesInDataDirs() throws IOException {
 
   private void createLinksFromSnapshotDirToDataDirWithoutLog(File sourceDir)
       throws IOException, DiskSpaceInsufficientException {
-    File seqFileDir = new File(sourceDir, "sequence" + File.separator + storageGroupName);
-    File unseqFileDir = new File(sourceDir, "unsequence" + File.separator + storageGroupName);
+    File seqFileDir =
+        new File(
+            sourceDir,
+            IoTDBConstant.SEQUENCE_FLODER_NAME
+                + File.separator
+                + storageGroupName
+                + File.separator
+                + dataRegionId);
+    File unseqFileDir =
+        new File(
+            sourceDir,
+            IoTDBConstant.UNSEQUENCE_FLODER_NAME
+                + File.separator
+                + storageGroupName
+                + File.separator
+                + dataRegionId);
     if (!seqFileDir.exists() && !unseqFileDir.exists()) {
       throw new IOException(
           String.format(
               "Cannot find %s or %s",
               seqFileDir.getAbsolutePath(), unseqFileDir.getAbsolutePath()));
     }
-
-    File[] seqRegionDirs = seqFileDir.listFiles();
-    if (seqRegionDirs != null && seqRegionDirs.length > 0) {
-      for (File seqRegionDir : seqRegionDirs) {
-        if (!seqRegionDir.isDirectory()) {
-          LOGGER.info("Skip {}, because it is not a directory", seqRegionDir);
+    folderManager =
+        new FolderManager(
+            Arrays.asList(IoTDBDescriptor.getInstance().getConfig().getDataDirs()),
+            DirectoryStrategyType.SEQUENCE_STRATEGY);
+    File[] timePartitionFolders = seqFileDir.listFiles();
+    if (timePartitionFolders != null) {
+      for (File timePartitionFolder : timePartitionFolders) {
+        File[] files = timePartitionFolder.listFiles();
+        if (files == null || files.length == 0) {
           continue;
         }
-        File[] seqPartitionDirs = seqRegionDir.listFiles();
-        if (seqPartitionDirs != null && seqPartitionDirs.length > 0) {
-          for (File seqPartitionDir : seqPartitionDirs) {
-            String[] splitPath =
-                seqPartitionDir
-                    .getAbsolutePath()
-                    .split(File.separator.equals("\\") ? "\\\\" : File.separator);
-            long timePartition = Long.parseLong(splitPath[splitPath.length - 1]);
-            File[] files = seqPartitionDir.listFiles();
-            if (files != null && files.length > 0) {
-              Arrays.sort(files, Comparator.comparing(File::getName));
-              String currDir = DirectoryManager.getInstance().getNextFolderForSequenceFile();
-              for (File file : files) {
-                if (file.getName().endsWith(".tsfile")) {
-                  currDir = DirectoryManager.getInstance().getNextFolderForSequenceFile();
-                }
-                File targetFile =
-                    new File(
-                        currDir,
-                        storageGroupName
-                            + File.separator
-                            + dataRegionId
-                            + File.separator
-                            + timePartition
-                            + File.separator
-                            + file.getName());
-                if (!targetFile.getParentFile().exists() && !targetFile.getParentFile().mkdirs()) {
-                  throw new IOException(
-                      String.format("Failed to create dir %s", targetFile.getParent()));
-                }
-                try {
-                  Files.createLink(targetFile.toPath(), file.toPath());
-                } catch (FileSystemException e) {
-                  // cannot create link between two directories in different fs
-                  // copy it
-                  Files.copy(file.toPath(), targetFile.toPath());
-                }
-              }
-            }
-          }
-        }
+        String targetSuffix =
+            IoTDBConstant.SEQUENCE_FLODER_NAME
+                + File.separator
+                + storageGroupName
+                + File.separator
+                + dataRegionId
+                + File.separator
+                + timePartitionFolder.getName();
+        createLinksFromSnapshotToSourceDir(targetSuffix, files);
       }
     }
 
-    File[] unseqRegionDirs = unseqFileDir.listFiles();
-    if (unseqRegionDirs != null && unseqRegionDirs.length > 0) {
-      for (File unseqRegionDir : unseqRegionDirs) {
-        if (!unseqRegionDir.isDirectory()) {
-          LOGGER.info("Skip {}, because it is not a directory", unseqRegionDir);
+    timePartitionFolders = unseqFileDir.listFiles();
+    if (timePartitionFolders != null) {
+      for (File timePartitionFolder : timePartitionFolders) {
+        File[] files = timePartitionFolder.listFiles();
+        if (files == null || files.length == 0) {
           continue;
         }
-        File[] unseqPartitionDirs = unseqRegionDir.listFiles();
-        if (unseqPartitionDirs != null && unseqPartitionDirs.length > 0) {
-          for (File unseqPartitionDir : unseqPartitionDirs) {
-            String[] splitPath =
-                unseqPartitionDir
-                    .getAbsolutePath()
-                    .split(File.separator.equals("\\") ? "\\\\" : File.separator);
-            long timePartition = Long.parseLong(splitPath[splitPath.length - 1]);
-            File[] files = unseqPartitionDir.listFiles();
-            if (files != null && files.length > 0) {
-              Arrays.sort(files, Comparator.comparing(File::getName));
-              String currDir = DirectoryManager.getInstance().getNextFolderForUnSequenceFile();
-              for (File file : files) {
-                if (file.getName().endsWith(".tsfile")) {
-                  currDir = DirectoryManager.getInstance().getNextFolderForUnSequenceFile();
-                }
-                File targetFile =
-                    new File(
-                        currDir,
-                        storageGroupName
-                            + File.separator
-                            + dataRegionId
-                            + File.separator
-                            + timePartition
-                            + File.separator
-                            + file.getName());
-                if (!targetFile.getParentFile().exists() && !targetFile.getParentFile().mkdirs()) {
-                  throw new IOException(
-                      String.format("Failed to create dir %s", targetFile.getParent()));
-                }
-                try {
-                  Files.createLink(targetFile.toPath(), file.toPath());
-                } catch (FileSystemException e) {
-                  Files.copy(file.toPath(), targetFile.toPath());
-                }
-              }
-            }
-          }
-        }
+        String targetSuffix =
+            IoTDBConstant.UNSEQUENCE_FLODER_NAME
+                + File.separator
+                + storageGroupName
+                + File.separator
+                + dataRegionId
+                + File.separator
+                + timePartitionFolder.getName();
+        createLinksFromSnapshotToSourceDir(targetSuffix, files);
       }
     }
   }
 
-  private void createLinksFromSnapshotDirToDataDirWithLog() {
-    while (logAnalyzer.hasNext()) {
-      Pair<String, String> filesPath = logAnalyzer.getNextPairs();
-      File sourceFile = new File(filesPath.left);
-      File linkedFile = new File(filesPath.right);
-      if (!linkedFile.exists()) {
-        LOGGER.warn("Snapshot file {} does not exist, skip it", linkedFile);
-        continue;
-      }
-      if (!sourceFile.getParentFile().exists() && !sourceFile.getParentFile().mkdirs()) {
-        LOGGER.error("Failed to create folder {}", sourceFile.getParentFile());
-        continue;
+  private void createLinksFromSnapshotToSourceDir(String targetSuffix, File[] files)
+      throws DiskSpaceInsufficientException, IOException {
+    for (File file : files) {
+      String dataDir = folderManager.getNextFolder();
+      File targetFile =
+          new File(dataDir + File.separator + targetSuffix + File.separator + file.getName());
+      if (!targetFile.getParentFile().exists() && !targetFile.getParentFile().mkdirs()) {
+        throw new IOException(
+            String.format(
+                "Cannot create directory %s", targetFile.getParentFile().getAbsolutePath()));
       }
       try {
-        Files.createLink(sourceFile.toPath(), linkedFile.toPath());
+        Files.createLink(targetFile.toPath(), file.toPath());
+        continue;
       } catch (IOException e) {
-        LOGGER.error("Failed to create link from {} to {}", linkedFile, sourceFile, e);
+        LOGGER.info("Cannot create link from {} to {}, try to copy it", file, targetFile);
       }
+
+      Files.copy(file.toPath(), targetFile.toPath());
+    }
+  }
+
+  private void createLinksFromSnapshotDirToDataDirWithLog() throws IOException {
+    String snapshotId = logAnalyzer.getSnapshotId();
+    int fileNum = logAnalyzer.getTotalFileCountInSnapshot();

Review Comment:
   Use more accurate variable name here such as `expectedFileCount` 



-- 
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] xingtanzjr commented on a diff in pull request #7535: [IOTDB-4205] Use logical path to take place of absolute path in snapshot log

Posted by GitBox <gi...@apache.org>.
xingtanzjr commented on code in PR #7535:
URL: https://github.com/apache/iotdb/pull/7535#discussion_r995302558


##########
server/src/main/java/org/apache/iotdb/db/engine/snapshot/SnapshotLogAnalyzer.java:
##########
@@ -55,25 +55,47 @@ public boolean hasNext() {
     }
   }
 
+  public String getSnapshotId() {
+    return snapshotId;
+  }
+
   /**
-   * @return The next pair of files recorded in the log. The left one is the path of source file,
-   *     the right one is the path of target file
+   * Return the total num of file in this snapshot.
+   *
+   * @return
    */
-  public Pair<String, String> getNextPairs() {
-    if (reader == null) {
-      return null;
+  public int getTotalFileCountInSnapshot() throws IOException {
+    reader.reset();
+    String currLine;
+    int cnt = 0;
+    while ((currLine = reader.readLine()) != null && !currLine.equals(SnapshotLogger.END_FLAG)) {
+      cnt++;
     }
-    try {
-      String fileInfo = reader.readLine();
-      String[] filesPath = fileInfo.split(SnapshotLogger.SPLIT_CHAR);
-      if (filesPath.length != 2) {
-        LOGGER.warn("Illegal file info: {} in snapshot log", fileInfo);
-        return null;
-      }
-      return new Pair<>(filesPath[0], filesPath[1]);
-    } catch (IOException e) {
-      LOGGER.error("Exception occurs when analyzing snapshot log", e);
-      return null;
+    return cnt;
+  }
+
+  /**
+   * Read the tail of the log file to see if the snapshot is complete.
+   *
+   * @return
+   */
+  public boolean isSnapshotComplete() throws IOException {
+    char[] endFlagInChar = new char[SnapshotLogger.END_FLAG.length()];

Review Comment:
   Done



-- 
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] xingtanzjr commented on a diff in pull request #7535: [IOTDB-4205] Use logical path to take place of absolute path in snapshot log

Posted by GitBox <gi...@apache.org>.
xingtanzjr commented on code in PR #7535:
URL: https://github.com/apache/iotdb/pull/7535#discussion_r991316532


##########
server/src/main/java/org/apache/iotdb/db/engine/snapshot/SnapshotLogAnalyzer.java:
##########
@@ -55,25 +55,47 @@ public boolean hasNext() {
     }
   }
 
+  public String getSnapshotId() {
+    return snapshotId;
+  }
+
   /**
-   * @return The next pair of files recorded in the log. The left one is the path of source file,
-   *     the right one is the path of target file
+   * Return the total num of file in this snapshot.
+   *
+   * @return
    */
-  public Pair<String, String> getNextPairs() {
-    if (reader == null) {
-      return null;
+  public int getTotalFileCountInSnapshot() throws IOException {
+    reader.reset();
+    String currLine;
+    int cnt = 0;
+    while ((currLine = reader.readLine()) != null && !currLine.equals(SnapshotLogger.END_FLAG)) {
+      cnt++;
     }
-    try {
-      String fileInfo = reader.readLine();
-      String[] filesPath = fileInfo.split(SnapshotLogger.SPLIT_CHAR);
-      if (filesPath.length != 2) {
-        LOGGER.warn("Illegal file info: {} in snapshot log", fileInfo);
-        return null;
-      }
-      return new Pair<>(filesPath[0], filesPath[1]);
-    } catch (IOException e) {
-      LOGGER.error("Exception occurs when analyzing snapshot log", e);
-      return null;
+    return cnt;
+  }
+
+  /**
+   * Read the tail of the log file to see if the snapshot is complete.
+   *
+   * @return
+   */
+  public boolean isSnapshotComplete() throws IOException {
+    char[] endFlagInChar = new char[SnapshotLogger.END_FLAG.length()];

Review Comment:
   Do the validation logic when constructing Analyzer to avoid multiple invoking to this method by others



-- 
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 #7535: [IOTDB-4205] Use logical path to take place of absolute path in snapshot log

Posted by GitBox <gi...@apache.org>.
THUMarkLau commented on code in PR #7535:
URL: https://github.com/apache/iotdb/pull/7535#discussion_r992265827


##########
server/src/main/java/org/apache/iotdb/db/engine/snapshot/SnapshotLoader.java:
##########
@@ -217,142 +224,203 @@ private void deleteAllFilesInDataDirs() throws IOException {
 
   private void createLinksFromSnapshotDirToDataDirWithoutLog(File sourceDir)
       throws IOException, DiskSpaceInsufficientException {
-    File seqFileDir = new File(sourceDir, "sequence" + File.separator + storageGroupName);
-    File unseqFileDir = new File(sourceDir, "unsequence" + File.separator + storageGroupName);
+    File seqFileDir =
+        new File(
+            sourceDir,
+            IoTDBConstant.SEQUENCE_FLODER_NAME
+                + File.separator
+                + storageGroupName
+                + File.separator
+                + dataRegionId);
+    File unseqFileDir =
+        new File(
+            sourceDir,
+            IoTDBConstant.UNSEQUENCE_FLODER_NAME
+                + File.separator
+                + storageGroupName
+                + File.separator
+                + dataRegionId);
     if (!seqFileDir.exists() && !unseqFileDir.exists()) {
       throw new IOException(
           String.format(
               "Cannot find %s or %s",
               seqFileDir.getAbsolutePath(), unseqFileDir.getAbsolutePath()));
     }
-
-    File[] seqRegionDirs = seqFileDir.listFiles();
-    if (seqRegionDirs != null && seqRegionDirs.length > 0) {
-      for (File seqRegionDir : seqRegionDirs) {
-        if (!seqRegionDir.isDirectory()) {
-          LOGGER.info("Skip {}, because it is not a directory", seqRegionDir);
+    folderManager =
+        new FolderManager(
+            Arrays.asList(IoTDBDescriptor.getInstance().getConfig().getDataDirs()),
+            DirectoryStrategyType.SEQUENCE_STRATEGY);
+    File[] timePartitionFolders = seqFileDir.listFiles();
+    if (timePartitionFolders != null) {
+      for (File timePartitionFolder : timePartitionFolders) {
+        File[] files = timePartitionFolder.listFiles();
+        if (files == null || files.length == 0) {
           continue;
         }
-        File[] seqPartitionDirs = seqRegionDir.listFiles();
-        if (seqPartitionDirs != null && seqPartitionDirs.length > 0) {
-          for (File seqPartitionDir : seqPartitionDirs) {
-            String[] splitPath =
-                seqPartitionDir
-                    .getAbsolutePath()
-                    .split(File.separator.equals("\\") ? "\\\\" : File.separator);
-            long timePartition = Long.parseLong(splitPath[splitPath.length - 1]);
-            File[] files = seqPartitionDir.listFiles();
-            if (files != null && files.length > 0) {
-              Arrays.sort(files, Comparator.comparing(File::getName));
-              String currDir = DirectoryManager.getInstance().getNextFolderForSequenceFile();
-              for (File file : files) {
-                if (file.getName().endsWith(".tsfile")) {
-                  currDir = DirectoryManager.getInstance().getNextFolderForSequenceFile();
-                }
-                File targetFile =
-                    new File(
-                        currDir,
-                        storageGroupName
-                            + File.separator
-                            + dataRegionId
-                            + File.separator
-                            + timePartition
-                            + File.separator
-                            + file.getName());
-                if (!targetFile.getParentFile().exists() && !targetFile.getParentFile().mkdirs()) {
-                  throw new IOException(
-                      String.format("Failed to create dir %s", targetFile.getParent()));
-                }
-                try {
-                  Files.createLink(targetFile.toPath(), file.toPath());
-                } catch (FileSystemException e) {
-                  // cannot create link between two directories in different fs
-                  // copy it
-                  Files.copy(file.toPath(), targetFile.toPath());
-                }
-              }
-            }
-          }
-        }
+        String targetSuffix =
+            IoTDBConstant.SEQUENCE_FLODER_NAME
+                + File.separator
+                + storageGroupName
+                + File.separator
+                + dataRegionId
+                + File.separator
+                + timePartitionFolder.getName();
+        createLinksFromSnapshotToSourceDir(targetSuffix, files);
       }
     }
 
-    File[] unseqRegionDirs = unseqFileDir.listFiles();
-    if (unseqRegionDirs != null && unseqRegionDirs.length > 0) {
-      for (File unseqRegionDir : unseqRegionDirs) {
-        if (!unseqRegionDir.isDirectory()) {
-          LOGGER.info("Skip {}, because it is not a directory", unseqRegionDir);
+    timePartitionFolders = unseqFileDir.listFiles();
+    if (timePartitionFolders != null) {
+      for (File timePartitionFolder : timePartitionFolders) {
+        File[] files = timePartitionFolder.listFiles();
+        if (files == null || files.length == 0) {
           continue;
         }
-        File[] unseqPartitionDirs = unseqRegionDir.listFiles();
-        if (unseqPartitionDirs != null && unseqPartitionDirs.length > 0) {
-          for (File unseqPartitionDir : unseqPartitionDirs) {
-            String[] splitPath =
-                unseqPartitionDir
-                    .getAbsolutePath()
-                    .split(File.separator.equals("\\") ? "\\\\" : File.separator);
-            long timePartition = Long.parseLong(splitPath[splitPath.length - 1]);
-            File[] files = unseqPartitionDir.listFiles();
-            if (files != null && files.length > 0) {
-              Arrays.sort(files, Comparator.comparing(File::getName));
-              String currDir = DirectoryManager.getInstance().getNextFolderForUnSequenceFile();
-              for (File file : files) {
-                if (file.getName().endsWith(".tsfile")) {
-                  currDir = DirectoryManager.getInstance().getNextFolderForUnSequenceFile();
-                }
-                File targetFile =
-                    new File(
-                        currDir,
-                        storageGroupName
-                            + File.separator
-                            + dataRegionId
-                            + File.separator
-                            + timePartition
-                            + File.separator
-                            + file.getName());
-                if (!targetFile.getParentFile().exists() && !targetFile.getParentFile().mkdirs()) {
-                  throw new IOException(
-                      String.format("Failed to create dir %s", targetFile.getParent()));
-                }
-                try {
-                  Files.createLink(targetFile.toPath(), file.toPath());
-                } catch (FileSystemException e) {
-                  Files.copy(file.toPath(), targetFile.toPath());
-                }
-              }
-            }
-          }
-        }
+        String targetSuffix =
+            IoTDBConstant.UNSEQUENCE_FLODER_NAME
+                + File.separator
+                + storageGroupName
+                + File.separator
+                + dataRegionId
+                + File.separator
+                + timePartitionFolder.getName();
+        createLinksFromSnapshotToSourceDir(targetSuffix, files);
       }
     }
   }
 
-  private void createLinksFromSnapshotDirToDataDirWithLog() {
-    while (logAnalyzer.hasNext()) {
-      Pair<String, String> filesPath = logAnalyzer.getNextPairs();
-      File sourceFile = new File(filesPath.left);
-      File linkedFile = new File(filesPath.right);
-      if (!linkedFile.exists()) {
-        LOGGER.warn("Snapshot file {} does not exist, skip it", linkedFile);
-        continue;
-      }
-      if (!sourceFile.getParentFile().exists() && !sourceFile.getParentFile().mkdirs()) {
-        LOGGER.error("Failed to create folder {}", sourceFile.getParentFile());
-        continue;
+  private void createLinksFromSnapshotToSourceDir(String targetSuffix, File[] files)
+      throws DiskSpaceInsufficientException, IOException {
+    for (File file : files) {
+      String dataDir = folderManager.getNextFolder();

Review Comment:
   yes



-- 
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] xingtanzjr commented on a diff in pull request #7535: [IOTDB-4205] Use logical path to take place of absolute path in snapshot log

Posted by GitBox <gi...@apache.org>.
xingtanzjr commented on code in PR #7535:
URL: https://github.com/apache/iotdb/pull/7535#discussion_r991404758


##########
server/src/main/java/org/apache/iotdb/db/engine/snapshot/SnapshotLoader.java:
##########
@@ -217,142 +224,203 @@ private void deleteAllFilesInDataDirs() throws IOException {
 
   private void createLinksFromSnapshotDirToDataDirWithoutLog(File sourceDir)
       throws IOException, DiskSpaceInsufficientException {
-    File seqFileDir = new File(sourceDir, "sequence" + File.separator + storageGroupName);
-    File unseqFileDir = new File(sourceDir, "unsequence" + File.separator + storageGroupName);
+    File seqFileDir =
+        new File(
+            sourceDir,
+            IoTDBConstant.SEQUENCE_FLODER_NAME
+                + File.separator
+                + storageGroupName
+                + File.separator
+                + dataRegionId);
+    File unseqFileDir =
+        new File(
+            sourceDir,
+            IoTDBConstant.UNSEQUENCE_FLODER_NAME
+                + File.separator
+                + storageGroupName
+                + File.separator
+                + dataRegionId);
     if (!seqFileDir.exists() && !unseqFileDir.exists()) {
       throw new IOException(
           String.format(
               "Cannot find %s or %s",
               seqFileDir.getAbsolutePath(), unseqFileDir.getAbsolutePath()));
     }
-
-    File[] seqRegionDirs = seqFileDir.listFiles();
-    if (seqRegionDirs != null && seqRegionDirs.length > 0) {
-      for (File seqRegionDir : seqRegionDirs) {
-        if (!seqRegionDir.isDirectory()) {
-          LOGGER.info("Skip {}, because it is not a directory", seqRegionDir);
+    folderManager =

Review Comment:
   Initializing the `folderManager` outside but using it inside several methods is wired.



-- 
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] xingtanzjr commented on a diff in pull request #7535: [IOTDB-4205] Use logical path to take place of absolute path in snapshot log

Posted by GitBox <gi...@apache.org>.
xingtanzjr commented on code in PR #7535:
URL: https://github.com/apache/iotdb/pull/7535#discussion_r991399470


##########
server/src/main/java/org/apache/iotdb/db/engine/snapshot/SnapshotLoader.java:
##########
@@ -217,142 +224,203 @@ private void deleteAllFilesInDataDirs() throws IOException {
 
   private void createLinksFromSnapshotDirToDataDirWithoutLog(File sourceDir)
       throws IOException, DiskSpaceInsufficientException {
-    File seqFileDir = new File(sourceDir, "sequence" + File.separator + storageGroupName);
-    File unseqFileDir = new File(sourceDir, "unsequence" + File.separator + storageGroupName);
+    File seqFileDir =
+        new File(
+            sourceDir,
+            IoTDBConstant.SEQUENCE_FLODER_NAME
+                + File.separator
+                + storageGroupName
+                + File.separator
+                + dataRegionId);
+    File unseqFileDir =
+        new File(
+            sourceDir,
+            IoTDBConstant.UNSEQUENCE_FLODER_NAME
+                + File.separator
+                + storageGroupName
+                + File.separator
+                + dataRegionId);
     if (!seqFileDir.exists() && !unseqFileDir.exists()) {
       throw new IOException(
           String.format(
               "Cannot find %s or %s",
               seqFileDir.getAbsolutePath(), unseqFileDir.getAbsolutePath()));
     }
-
-    File[] seqRegionDirs = seqFileDir.listFiles();
-    if (seqRegionDirs != null && seqRegionDirs.length > 0) {
-      for (File seqRegionDir : seqRegionDirs) {
-        if (!seqRegionDir.isDirectory()) {
-          LOGGER.info("Skip {}, because it is not a directory", seqRegionDir);
+    folderManager =
+        new FolderManager(
+            Arrays.asList(IoTDBDescriptor.getInstance().getConfig().getDataDirs()),
+            DirectoryStrategyType.SEQUENCE_STRATEGY);
+    File[] timePartitionFolders = seqFileDir.listFiles();
+    if (timePartitionFolders != null) {
+      for (File timePartitionFolder : timePartitionFolders) {
+        File[] files = timePartitionFolder.listFiles();
+        if (files == null || files.length == 0) {
           continue;
         }
-        File[] seqPartitionDirs = seqRegionDir.listFiles();
-        if (seqPartitionDirs != null && seqPartitionDirs.length > 0) {
-          for (File seqPartitionDir : seqPartitionDirs) {
-            String[] splitPath =
-                seqPartitionDir
-                    .getAbsolutePath()
-                    .split(File.separator.equals("\\") ? "\\\\" : File.separator);
-            long timePartition = Long.parseLong(splitPath[splitPath.length - 1]);
-            File[] files = seqPartitionDir.listFiles();
-            if (files != null && files.length > 0) {
-              Arrays.sort(files, Comparator.comparing(File::getName));
-              String currDir = DirectoryManager.getInstance().getNextFolderForSequenceFile();
-              for (File file : files) {
-                if (file.getName().endsWith(".tsfile")) {
-                  currDir = DirectoryManager.getInstance().getNextFolderForSequenceFile();
-                }
-                File targetFile =
-                    new File(
-                        currDir,
-                        storageGroupName
-                            + File.separator
-                            + dataRegionId
-                            + File.separator
-                            + timePartition
-                            + File.separator
-                            + file.getName());
-                if (!targetFile.getParentFile().exists() && !targetFile.getParentFile().mkdirs()) {
-                  throw new IOException(
-                      String.format("Failed to create dir %s", targetFile.getParent()));
-                }
-                try {
-                  Files.createLink(targetFile.toPath(), file.toPath());
-                } catch (FileSystemException e) {
-                  // cannot create link between two directories in different fs
-                  // copy it
-                  Files.copy(file.toPath(), targetFile.toPath());
-                }
-              }
-            }
-          }
-        }
+        String targetSuffix =
+            IoTDBConstant.SEQUENCE_FLODER_NAME
+                + File.separator
+                + storageGroupName
+                + File.separator
+                + dataRegionId
+                + File.separator
+                + timePartitionFolder.getName();
+        createLinksFromSnapshotToSourceDir(targetSuffix, files);
       }
     }
 
-    File[] unseqRegionDirs = unseqFileDir.listFiles();
-    if (unseqRegionDirs != null && unseqRegionDirs.length > 0) {
-      for (File unseqRegionDir : unseqRegionDirs) {
-        if (!unseqRegionDir.isDirectory()) {
-          LOGGER.info("Skip {}, because it is not a directory", unseqRegionDir);
+    timePartitionFolders = unseqFileDir.listFiles();
+    if (timePartitionFolders != null) {
+      for (File timePartitionFolder : timePartitionFolders) {
+        File[] files = timePartitionFolder.listFiles();
+        if (files == null || files.length == 0) {
           continue;
         }
-        File[] unseqPartitionDirs = unseqRegionDir.listFiles();
-        if (unseqPartitionDirs != null && unseqPartitionDirs.length > 0) {
-          for (File unseqPartitionDir : unseqPartitionDirs) {
-            String[] splitPath =
-                unseqPartitionDir
-                    .getAbsolutePath()
-                    .split(File.separator.equals("\\") ? "\\\\" : File.separator);
-            long timePartition = Long.parseLong(splitPath[splitPath.length - 1]);
-            File[] files = unseqPartitionDir.listFiles();
-            if (files != null && files.length > 0) {
-              Arrays.sort(files, Comparator.comparing(File::getName));
-              String currDir = DirectoryManager.getInstance().getNextFolderForUnSequenceFile();
-              for (File file : files) {
-                if (file.getName().endsWith(".tsfile")) {
-                  currDir = DirectoryManager.getInstance().getNextFolderForUnSequenceFile();
-                }
-                File targetFile =
-                    new File(
-                        currDir,
-                        storageGroupName
-                            + File.separator
-                            + dataRegionId
-                            + File.separator
-                            + timePartition
-                            + File.separator
-                            + file.getName());
-                if (!targetFile.getParentFile().exists() && !targetFile.getParentFile().mkdirs()) {
-                  throw new IOException(
-                      String.format("Failed to create dir %s", targetFile.getParent()));
-                }
-                try {
-                  Files.createLink(targetFile.toPath(), file.toPath());
-                } catch (FileSystemException e) {
-                  Files.copy(file.toPath(), targetFile.toPath());
-                }
-              }
-            }
-          }
-        }
+        String targetSuffix =
+            IoTDBConstant.UNSEQUENCE_FLODER_NAME
+                + File.separator
+                + storageGroupName
+                + File.separator
+                + dataRegionId
+                + File.separator
+                + timePartitionFolder.getName();
+        createLinksFromSnapshotToSourceDir(targetSuffix, files);
       }
     }
   }
 
-  private void createLinksFromSnapshotDirToDataDirWithLog() {
-    while (logAnalyzer.hasNext()) {
-      Pair<String, String> filesPath = logAnalyzer.getNextPairs();
-      File sourceFile = new File(filesPath.left);
-      File linkedFile = new File(filesPath.right);
-      if (!linkedFile.exists()) {
-        LOGGER.warn("Snapshot file {} does not exist, skip it", linkedFile);
-        continue;
-      }
-      if (!sourceFile.getParentFile().exists() && !sourceFile.getParentFile().mkdirs()) {
-        LOGGER.error("Failed to create folder {}", sourceFile.getParentFile());
-        continue;
+  private void createLinksFromSnapshotToSourceDir(String targetSuffix, File[] files)
+      throws DiskSpaceInsufficientException, IOException {
+    for (File file : files) {
+      String dataDir = folderManager.getNextFolder();
+      File targetFile =
+          new File(dataDir + File.separator + targetSuffix + File.separator + file.getName());
+      if (!targetFile.getParentFile().exists() && !targetFile.getParentFile().mkdirs()) {
+        throw new IOException(
+            String.format(
+                "Cannot create directory %s", targetFile.getParentFile().getAbsolutePath()));
       }
       try {
-        Files.createLink(sourceFile.toPath(), linkedFile.toPath());
+        Files.createLink(targetFile.toPath(), file.toPath());
+        continue;
       } catch (IOException e) {
-        LOGGER.error("Failed to create link from {} to {}", linkedFile, sourceFile, e);
+        LOGGER.info("Cannot create link from {} to {}, try to copy it", file, targetFile);
       }
+
+      Files.copy(file.toPath(), targetFile.toPath());
+    }
+  }
+
+  private void createLinksFromSnapshotDirToDataDirWithLog() throws IOException {
+    String snapshotId = logAnalyzer.getSnapshotId();
+    int fileNum = logAnalyzer.getTotalFileCountInSnapshot();
+    String[] dataDirs = IoTDBDescriptor.getInstance().getConfig().getDataDirs();
+    int fileCnt = 0;
+    for (String dataDir : dataDirs) {

Review Comment:
   How can we ensure each DataDir has corresponding snapshot ?



-- 
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] xingtanzjr commented on a diff in pull request #7535: [IOTDB-4205] Use logical path to take place of absolute path in snapshot log

Posted by GitBox <gi...@apache.org>.
xingtanzjr commented on code in PR #7535:
URL: https://github.com/apache/iotdb/pull/7535#discussion_r991301138


##########
server/src/main/java/org/apache/iotdb/db/engine/snapshot/SnapshotTaker.java:
##########
@@ -183,8 +186,8 @@ private void createHardLink(File target, File source) throws IOException {
       LOGGER.error("Hard link source file {} doesn't exist", source);
     }
     Files.deleteIfExists(target.toPath());
-    Files.createLink(target.getAbsoluteFile().toPath(), source.getAbsoluteFile().toPath());
-    snapshotLogger.logFile(source.getAbsolutePath(), target.getAbsolutePath());

Review Comment:
   Is this change equivalent with previous style ?



-- 
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] xingtanzjr commented on a diff in pull request #7535: [IOTDB-4205] Use logical path to take place of absolute path in snapshot log

Posted by GitBox <gi...@apache.org>.
xingtanzjr commented on code in PR #7535:
URL: https://github.com/apache/iotdb/pull/7535#discussion_r991304338


##########
server/src/main/java/org/apache/iotdb/db/engine/snapshot/SnapshotTaker.java:
##########
@@ -71,7 +72,8 @@ public boolean takeFullSnapshot(String snapshotDirPath, boolean flushBeforeSnaps
     File snapshotLog = new File(snapshotDir, SnapshotLogger.SNAPSHOT_LOG_NAME);
     try {
       snapshotLogger = new SnapshotLogger(snapshotLog);
-      boolean success;
+      boolean success = true;

Review Comment:
   initializing value is unnecessary



-- 
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] xingtanzjr commented on a diff in pull request #7535: [IOTDB-4205] Use logical path to take place of absolute path in snapshot log

Posted by GitBox <gi...@apache.org>.
xingtanzjr commented on code in PR #7535:
URL: https://github.com/apache/iotdb/pull/7535#discussion_r991327460


##########
server/src/test/java/org/apache/iotdb/db/engine/snapshot/IoTDBSnapshotTest.java:
##########
@@ -108,11 +108,9 @@ public void testCreateSnapshot()
             snapshotDir.listFiles((dir, name) -> name.equals(SnapshotLogger.SNAPSHOT_LOG_NAME));
         Assert.assertEquals(1, files.length);
         SnapshotLogAnalyzer analyzer = new SnapshotLogAnalyzer(files[0]);
-        int cnt = 0;
-        while (analyzer.hasNext()) {
-          analyzer.getNextPairs();
-          cnt++;
-        }
+        analyzer.getSnapshotId();

Review Comment:
   What is this line used for ? 



##########
server/src/test/java/org/apache/iotdb/db/engine/snapshot/IoTDBSnapshotTest.java:
##########
@@ -147,10 +145,9 @@ public void testCreateSnapshotWithUnclosedTsFile()
         Assert.assertEquals(1, files.length);
         SnapshotLogAnalyzer analyzer = new SnapshotLogAnalyzer(files[0]);
         int cnt = 0;
-        while (analyzer.hasNext()) {
-          analyzer.getNextPairs();
-          cnt++;
-        }
+        analyzer.getSnapshotId();

Review Comment:
   What is this line used for ?



-- 
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] xingtanzjr commented on a diff in pull request #7535: [IOTDB-4205] Use logical path to take place of absolute path in snapshot log

Posted by GitBox <gi...@apache.org>.
xingtanzjr commented on code in PR #7535:
URL: https://github.com/apache/iotdb/pull/7535#discussion_r991317489


##########
server/src/main/java/org/apache/iotdb/db/engine/snapshot/SnapshotLogAnalyzer.java:
##########
@@ -55,25 +55,47 @@ public boolean hasNext() {
     }
   }
 
+  public String getSnapshotId() {
+    return snapshotId;
+  }
+
   /**
-   * @return The next pair of files recorded in the log. The left one is the path of source file,
-   *     the right one is the path of target file
+   * Return the total num of file in this snapshot.
+   *
+   * @return
    */
-  public Pair<String, String> getNextPairs() {
-    if (reader == null) {
-      return null;
+  public int getTotalFileCountInSnapshot() throws IOException {
+    reader.reset();
+    String currLine;
+    int cnt = 0;
+    while ((currLine = reader.readLine()) != null && !currLine.equals(SnapshotLogger.END_FLAG)) {
+      cnt++;
     }
-    try {
-      String fileInfo = reader.readLine();
-      String[] filesPath = fileInfo.split(SnapshotLogger.SPLIT_CHAR);
-      if (filesPath.length != 2) {
-        LOGGER.warn("Illegal file info: {} in snapshot log", fileInfo);
-        return null;
-      }
-      return new Pair<>(filesPath[0], filesPath[1]);
-    } catch (IOException e) {
-      LOGGER.error("Exception occurs when analyzing snapshot log", e);
-      return null;
+    return cnt;
+  }
+
+  /**
+   * Read the tail of the log file to see if the snapshot is complete.
+   *
+   * @return
+   */
+  public boolean isSnapshotComplete() throws IOException {
+    char[] endFlagInChar = new char[SnapshotLogger.END_FLAG.length()];
+    long fileLength = snapshotLogFile.length();
+    int endFlagLength = SnapshotLogger.END_FLAG.getBytes(StandardCharsets.UTF_8).length;
+    if (fileLength < endFlagLength) {
+      // this snapshot cannot be complete
+      return false;
     }
+    reader.mark((int) fileLength);
+    reader.skip(
+        (int)
+            (fileLength - endFlagLength - snapshotId.getBytes(StandardCharsets.UTF_8).length - 1));

Review Comment:
   What does `1` means here ? Maybe we need to use a constant to indicating its meaning



-- 
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] xingtanzjr commented on a diff in pull request #7535: [IOTDB-4205] Use logical path to take place of absolute path in snapshot log

Posted by GitBox <gi...@apache.org>.
xingtanzjr commented on code in PR #7535:
URL: https://github.com/apache/iotdb/pull/7535#discussion_r991409757


##########
server/src/main/java/org/apache/iotdb/db/engine/snapshot/SnapshotLoader.java:
##########
@@ -217,142 +224,203 @@ private void deleteAllFilesInDataDirs() throws IOException {
 
   private void createLinksFromSnapshotDirToDataDirWithoutLog(File sourceDir)
       throws IOException, DiskSpaceInsufficientException {
-    File seqFileDir = new File(sourceDir, "sequence" + File.separator + storageGroupName);
-    File unseqFileDir = new File(sourceDir, "unsequence" + File.separator + storageGroupName);
+    File seqFileDir =
+        new File(
+            sourceDir,
+            IoTDBConstant.SEQUENCE_FLODER_NAME
+                + File.separator
+                + storageGroupName
+                + File.separator
+                + dataRegionId);
+    File unseqFileDir =
+        new File(
+            sourceDir,
+            IoTDBConstant.UNSEQUENCE_FLODER_NAME
+                + File.separator
+                + storageGroupName
+                + File.separator
+                + dataRegionId);
     if (!seqFileDir.exists() && !unseqFileDir.exists()) {
       throw new IOException(
           String.format(
               "Cannot find %s or %s",
               seqFileDir.getAbsolutePath(), unseqFileDir.getAbsolutePath()));
     }
-
-    File[] seqRegionDirs = seqFileDir.listFiles();
-    if (seqRegionDirs != null && seqRegionDirs.length > 0) {
-      for (File seqRegionDir : seqRegionDirs) {
-        if (!seqRegionDir.isDirectory()) {
-          LOGGER.info("Skip {}, because it is not a directory", seqRegionDir);
+    folderManager =
+        new FolderManager(
+            Arrays.asList(IoTDBDescriptor.getInstance().getConfig().getDataDirs()),
+            DirectoryStrategyType.SEQUENCE_STRATEGY);
+    File[] timePartitionFolders = seqFileDir.listFiles();
+    if (timePartitionFolders != null) {
+      for (File timePartitionFolder : timePartitionFolders) {
+        File[] files = timePartitionFolder.listFiles();
+        if (files == null || files.length == 0) {
           continue;
         }
-        File[] seqPartitionDirs = seqRegionDir.listFiles();
-        if (seqPartitionDirs != null && seqPartitionDirs.length > 0) {
-          for (File seqPartitionDir : seqPartitionDirs) {
-            String[] splitPath =
-                seqPartitionDir
-                    .getAbsolutePath()
-                    .split(File.separator.equals("\\") ? "\\\\" : File.separator);
-            long timePartition = Long.parseLong(splitPath[splitPath.length - 1]);
-            File[] files = seqPartitionDir.listFiles();
-            if (files != null && files.length > 0) {
-              Arrays.sort(files, Comparator.comparing(File::getName));
-              String currDir = DirectoryManager.getInstance().getNextFolderForSequenceFile();
-              for (File file : files) {
-                if (file.getName().endsWith(".tsfile")) {
-                  currDir = DirectoryManager.getInstance().getNextFolderForSequenceFile();
-                }
-                File targetFile =
-                    new File(
-                        currDir,
-                        storageGroupName
-                            + File.separator
-                            + dataRegionId
-                            + File.separator
-                            + timePartition
-                            + File.separator
-                            + file.getName());
-                if (!targetFile.getParentFile().exists() && !targetFile.getParentFile().mkdirs()) {
-                  throw new IOException(
-                      String.format("Failed to create dir %s", targetFile.getParent()));
-                }
-                try {
-                  Files.createLink(targetFile.toPath(), file.toPath());
-                } catch (FileSystemException e) {
-                  // cannot create link between two directories in different fs
-                  // copy it
-                  Files.copy(file.toPath(), targetFile.toPath());
-                }
-              }
-            }
-          }
-        }
+        String targetSuffix =
+            IoTDBConstant.SEQUENCE_FLODER_NAME
+                + File.separator
+                + storageGroupName
+                + File.separator
+                + dataRegionId
+                + File.separator
+                + timePartitionFolder.getName();
+        createLinksFromSnapshotToSourceDir(targetSuffix, files);
       }
     }
 
-    File[] unseqRegionDirs = unseqFileDir.listFiles();
-    if (unseqRegionDirs != null && unseqRegionDirs.length > 0) {
-      for (File unseqRegionDir : unseqRegionDirs) {
-        if (!unseqRegionDir.isDirectory()) {
-          LOGGER.info("Skip {}, because it is not a directory", unseqRegionDir);
+    timePartitionFolders = unseqFileDir.listFiles();
+    if (timePartitionFolders != null) {
+      for (File timePartitionFolder : timePartitionFolders) {
+        File[] files = timePartitionFolder.listFiles();
+        if (files == null || files.length == 0) {
           continue;
         }
-        File[] unseqPartitionDirs = unseqRegionDir.listFiles();
-        if (unseqPartitionDirs != null && unseqPartitionDirs.length > 0) {
-          for (File unseqPartitionDir : unseqPartitionDirs) {
-            String[] splitPath =
-                unseqPartitionDir
-                    .getAbsolutePath()
-                    .split(File.separator.equals("\\") ? "\\\\" : File.separator);
-            long timePartition = Long.parseLong(splitPath[splitPath.length - 1]);
-            File[] files = unseqPartitionDir.listFiles();
-            if (files != null && files.length > 0) {
-              Arrays.sort(files, Comparator.comparing(File::getName));
-              String currDir = DirectoryManager.getInstance().getNextFolderForUnSequenceFile();
-              for (File file : files) {
-                if (file.getName().endsWith(".tsfile")) {
-                  currDir = DirectoryManager.getInstance().getNextFolderForUnSequenceFile();
-                }
-                File targetFile =
-                    new File(
-                        currDir,
-                        storageGroupName
-                            + File.separator
-                            + dataRegionId
-                            + File.separator
-                            + timePartition
-                            + File.separator
-                            + file.getName());
-                if (!targetFile.getParentFile().exists() && !targetFile.getParentFile().mkdirs()) {
-                  throw new IOException(
-                      String.format("Failed to create dir %s", targetFile.getParent()));
-                }
-                try {
-                  Files.createLink(targetFile.toPath(), file.toPath());
-                } catch (FileSystemException e) {
-                  Files.copy(file.toPath(), targetFile.toPath());
-                }
-              }
-            }
-          }
-        }
+        String targetSuffix =
+            IoTDBConstant.UNSEQUENCE_FLODER_NAME
+                + File.separator
+                + storageGroupName
+                + File.separator
+                + dataRegionId
+                + File.separator
+                + timePartitionFolder.getName();
+        createLinksFromSnapshotToSourceDir(targetSuffix, files);
       }
     }
   }
 
-  private void createLinksFromSnapshotDirToDataDirWithLog() {
-    while (logAnalyzer.hasNext()) {
-      Pair<String, String> filesPath = logAnalyzer.getNextPairs();
-      File sourceFile = new File(filesPath.left);
-      File linkedFile = new File(filesPath.right);
-      if (!linkedFile.exists()) {
-        LOGGER.warn("Snapshot file {} does not exist, skip it", linkedFile);
-        continue;
-      }
-      if (!sourceFile.getParentFile().exists() && !sourceFile.getParentFile().mkdirs()) {
-        LOGGER.error("Failed to create folder {}", sourceFile.getParentFile());
-        continue;
+  private void createLinksFromSnapshotToSourceDir(String targetSuffix, File[] files)
+      throws DiskSpaceInsufficientException, IOException {
+    for (File file : files) {
+      String dataDir = folderManager.getNextFolder();

Review Comment:
   What is this operation used for ? Distribute the snapshot files to different 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] THUMarkLau commented on a diff in pull request #7535: [IOTDB-4205] Use logical path to take place of absolute path in snapshot log

Posted by GitBox <gi...@apache.org>.
THUMarkLau commented on code in PR #7535:
URL: https://github.com/apache/iotdb/pull/7535#discussion_r992264120


##########
server/src/main/java/org/apache/iotdb/db/engine/snapshot/SnapshotTaker.java:
##########
@@ -183,8 +186,8 @@ private void createHardLink(File target, File source) throws IOException {
       LOGGER.error("Hard link source file {} doesn't exist", source);
     }
     Files.deleteIfExists(target.toPath());
-    Files.createLink(target.getAbsoluteFile().toPath(), source.getAbsoluteFile().toPath());
-    snapshotLogger.logFile(source.getAbsolutePath(), target.getAbsolutePath());

Review Comment:
   yes



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