You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@iotdb.apache.org by xi...@apache.org on 2023/05/14 14:11:45 UTC

[iotdb] branch rel/1.1 updated: [To rel/1.1][IOTDB-5859]Compaction error when using Version as first sort dimension (#9825)

This is an automated email from the ASF dual-hosted git repository.

xingtanzjr pushed a commit to branch rel/1.1
in repository https://gitbox.apache.org/repos/asf/iotdb.git


The following commit(s) were added to refs/heads/rel/1.1 by this push:
     new 45fbd7e7209 [To rel/1.1][IOTDB-5859]Compaction error when using Version as first sort dimension (#9825)
45fbd7e7209 is described below

commit 45fbd7e7209b5da58c46849a1ec8588e4b3bda4d
Author: 周沛辰 <45...@users.noreply.github.com>
AuthorDate: Sun May 14 22:11:40 2023 +0800

    [To rel/1.1][IOTDB-5859]Compaction error when using Version as first sort dimension (#9825)
---
 .../execute/utils/MultiTsFileDeviceIterator.java   | 13 +++++---
 .../db/engine/storagegroup/TsFileResource.java     | 21 ++++++------
 .../engine/compaction/AbstractCompactionTest.java  | 37 ++++++++++++++++++----
 ...eCrossSpaceCompactionWithFastPerformerTest.java |  1 -
 .../utils/MultiTsFileDeviceIteratorTest.java       | 14 ++++++++
 5 files changed, 62 insertions(+), 24 deletions(-)

diff --git a/server/src/main/java/org/apache/iotdb/db/engine/compaction/execute/utils/MultiTsFileDeviceIterator.java b/server/src/main/java/org/apache/iotdb/db/engine/compaction/execute/utils/MultiTsFileDeviceIterator.java
index 967338d1f8d..388cb48f0fc 100644
--- a/server/src/main/java/org/apache/iotdb/db/engine/compaction/execute/utils/MultiTsFileDeviceIterator.java
+++ b/server/src/main/java/org/apache/iotdb/db/engine/compaction/execute/utils/MultiTsFileDeviceIterator.java
@@ -59,14 +59,15 @@ public class MultiTsFileDeviceIterator implements AutoCloseable {
   private final Map<TsFileResource, List<Modification>> modificationCache = new HashMap<>();
   private Pair<String, Boolean> currentDevice = null;
 
-  /** Used for inner space compaction. */
+  /** Used for compaction with read chunk performer. */
   public MultiTsFileDeviceIterator(List<TsFileResource> tsFileResources) throws IOException {
     this.tsFileResourcesSortedByDesc = new ArrayList<>(tsFileResources);
     this.tsFileResourcesSortedByAsc = new ArrayList<>(tsFileResources);
     // sort the files from the oldest to the newest
     Collections.sort(this.tsFileResourcesSortedByAsc, TsFileResource::compareFileName);
     // sort the files from the newest to the oldest
-    Collections.sort(this.tsFileResourcesSortedByDesc, TsFileResource::compareFileNameByDesc);
+    Collections.sort(
+        this.tsFileResourcesSortedByDesc, TsFileResource::compareFileCreationOrderByDesc);
     try {
       for (TsFileResource tsFileResource : this.tsFileResourcesSortedByDesc) {
         TsFileSequenceReader reader = new TsFileSequenceReader(tsFileResource.getTsFilePath());
@@ -83,13 +84,14 @@ public class MultiTsFileDeviceIterator implements AutoCloseable {
     }
   }
 
-  /** Used for cross space compaction with read point performer. */
+  /** Used for compaction with read point performer. */
   public MultiTsFileDeviceIterator(
       List<TsFileResource> seqResources, List<TsFileResource> unseqResources) throws IOException {
     this.tsFileResourcesSortedByDesc = new ArrayList<>(seqResources);
     tsFileResourcesSortedByDesc.addAll(unseqResources);
     // sort the files from the newest to the oldest
-    Collections.sort(this.tsFileResourcesSortedByDesc, TsFileResource::compareFileNameByDesc);
+    Collections.sort(
+        this.tsFileResourcesSortedByDesc, TsFileResource::compareFileCreationOrderByDesc);
     for (TsFileResource tsFileResource : tsFileResourcesSortedByDesc) {
       TsFileSequenceReader reader =
           FileReaderManager.getInstance().get(tsFileResource.getTsFilePath(), true);
@@ -107,7 +109,8 @@ public class MultiTsFileDeviceIterator implements AutoCloseable {
     this.tsFileResourcesSortedByDesc = new ArrayList<>(seqResources);
     tsFileResourcesSortedByDesc.addAll(unseqResources);
     // sort tsfiles from the newest to the oldest
-    Collections.sort(this.tsFileResourcesSortedByDesc, TsFileResource::compareFileNameByDesc);
+    Collections.sort(
+        this.tsFileResourcesSortedByDesc, TsFileResource::compareFileCreationOrderByDesc);
     this.readerMap = readerMap;
     for (TsFileResource tsFileResource : tsFileResourcesSortedByDesc) {
       TsFileSequenceReader reader = new TsFileSequenceReader(tsFileResource.getTsFilePath());
diff --git a/server/src/main/java/org/apache/iotdb/db/engine/storagegroup/TsFileResource.java b/server/src/main/java/org/apache/iotdb/db/engine/storagegroup/TsFileResource.java
index 510b05e0015..90c50bea941 100644
--- a/server/src/main/java/org/apache/iotdb/db/engine/storagegroup/TsFileResource.java
+++ b/server/src/main/java/org/apache/iotdb/db/engine/storagegroup/TsFileResource.java
@@ -1023,32 +1023,31 @@ public class TsFileResource {
   }
 
   /**
-   * Compare the name of TsFiles corresponding to the two {@link TsFileResource}.This method will
-   * first check whether the two names meet the standard naming specifications, and then compare
-   * version of two names.
+   * Compare the creation order of the files and sort them according to the version number from
+   * largest to smallest.This method will first check whether the two names meet the standard naming
+   * specifications, and then compare version of two names. Notice: This method is only used to
+   * compare the creation order of files, which is sorted directly according to version. If you want
+   * to compare the order of the content of the file, you must first sort by timestamp and then by
+   * version.
    *
    * @param o1 a {@link TsFileResource}
    * @param o2 a {@link TsFileResource}
-   * @return -1, if o1 is smaller than o2, 1 if bigger, 0 means o1 equals to o2 or do not meet the
-   *     naming specifications
+   * @return -1, if o1 is smaller than o2, 1 if bigger, 0 means o1 equals to o2
    */
-  public static int compareFileNameByDesc(TsFileResource o1, TsFileResource o2) {
+  public static int compareFileCreationOrderByDesc(TsFileResource o1, TsFileResource o2) {
     try {
       TsFileNameGenerator.TsFileName n1 =
           TsFileNameGenerator.getTsFileName(o1.getTsFile().getName());
       TsFileNameGenerator.TsFileName n2 =
           TsFileNameGenerator.getTsFileName(o2.getTsFile().getName());
-      long timeDiff = n2.getTime() - n1.getTime();
-      if (timeDiff != 0) {
-        return timeDiff < 0 ? -1 : 1;
-      }
       long versionDiff = n2.getVersion() - n1.getVersion();
       if (versionDiff != 0) {
         return versionDiff < 0 ? -1 : 1;
       }
       return 0;
     } catch (IOException e) {
-      return 0;
+      LOGGER.error("File name may not meet the standard naming specifications.", e);
+      throw new RuntimeException(e.getMessage());
     }
   }
 
diff --git a/server/src/test/java/org/apache/iotdb/db/engine/compaction/AbstractCompactionTest.java b/server/src/test/java/org/apache/iotdb/db/engine/compaction/AbstractCompactionTest.java
index 70ba4fa38c3..d9fd6e71f22 100644
--- a/server/src/test/java/org/apache/iotdb/db/engine/compaction/AbstractCompactionTest.java
+++ b/server/src/test/java/org/apache/iotdb/db/engine/compaction/AbstractCompactionTest.java
@@ -32,11 +32,13 @@ import org.apache.iotdb.db.engine.compaction.execute.utils.reader.SeriesDataBloc
 import org.apache.iotdb.db.engine.compaction.schedule.CompactionTaskManager;
 import org.apache.iotdb.db.engine.compaction.utils.CompactionConfigRestorer;
 import org.apache.iotdb.db.engine.compaction.utils.CompactionFileGeneratorUtils;
+import org.apache.iotdb.db.engine.modification.ModificationFile;
 import org.apache.iotdb.db.engine.storagegroup.TsFileManager;
 import org.apache.iotdb.db.engine.storagegroup.TsFileResource;
 import org.apache.iotdb.db.engine.storagegroup.TsFileResourceStatus;
 import org.apache.iotdb.db.exception.StorageEngineException;
 import org.apache.iotdb.db.mpp.execution.fragment.FragmentInstanceContext;
+import org.apache.iotdb.db.protocol.rest.StringUtil;
 import org.apache.iotdb.db.query.control.FileReaderManager;
 import org.apache.iotdb.db.tools.validate.TsFileValidationTool;
 import org.apache.iotdb.db.utils.EnvironmentUtils;
@@ -70,6 +72,7 @@ import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 
+import static org.apache.iotdb.commons.conf.IoTDBConstant.FILE_NAME_SEPARATOR;
 import static org.apache.iotdb.tsfile.common.constant.TsFileConstant.PATH_SEPARATOR;
 import static org.junit.Assert.fail;
 
@@ -205,9 +208,9 @@ public class AbstractCompactionTest {
       boolean isSeq)
       throws IOException, WriteProcessException, MetadataException {
     for (int i = 0; i < fileNum; i++) {
-      long version = isSeq ? seqVersion[fileCount] : unseqVersion[fileCount];
+      fileVersion = isSeq ? seqVersion[fileCount] : unseqVersion[fileCount];
       String fileName =
-          timestamp[fileCount++] + FilePathUtils.FILE_NAME_SEPARATOR + version + "-0-0.tsfile";
+          timestamp[fileCount++] + FilePathUtils.FILE_NAME_SEPARATOR + fileVersion + "-0-0.tsfile";
       String filePath;
       if (isSeq) {
         filePath = SEQ_DIRS.getPath() + File.separator + fileName;
@@ -277,9 +280,9 @@ public class AbstractCompactionTest {
       throws IOException, WriteProcessException {
     String value = isSeq ? "seqTestValue" : "unseqTestValue";
     for (int i = 0; i < fileNum; i++) {
-      long version = isSeq ? seqVersion[fileCount] : unseqVersion[fileCount];
+      fileVersion = isSeq ? seqVersion[fileCount] : unseqVersion[fileCount];
       String fileName =
-          timestamp[fileCount++] + FilePathUtils.FILE_NAME_SEPARATOR + version + "-0-0.tsfile";
+          timestamp[fileCount++] + FilePathUtils.FILE_NAME_SEPARATOR + fileVersion + "-0-0.tsfile";
       String filePath;
       if (isSeq) {
         filePath = SEQ_DIRS.getPath() + File.separator + fileName;
@@ -581,9 +584,9 @@ public class AbstractCompactionTest {
   }
 
   protected TsFileResource createEmptyFileAndResource(boolean isSeq) {
-    long version = isSeq ? seqVersion[fileCount] : unseqVersion[fileCount];
+    fileVersion = isSeq ? seqVersion[fileCount] : unseqVersion[fileCount];
     String fileName =
-        timestamp[fileCount++] + FilePathUtils.FILE_NAME_SEPARATOR + version + "-0-0.tsfile";
+        timestamp[fileCount++] + FilePathUtils.FILE_NAME_SEPARATOR + fileVersion + "-0-0.tsfile";
     String filePath;
     if (isSeq) {
       filePath = SEQ_DIRS.getPath() + File.separator + fileName;
@@ -591,7 +594,7 @@ public class AbstractCompactionTest {
       filePath = UNSEQ_DIRS.getPath() + File.separator + fileName;
     }
     TsFileResource resource = new TsFileResource(new File(filePath));
-    resource.updatePlanIndexes(fileVersion++);
+    resource.updatePlanIndexes(fileVersion);
     resource.setStatus(TsFileResourceStatus.CLOSED);
     return resource;
   }
@@ -599,4 +602,24 @@ public class AbstractCompactionTest {
   protected void setDataType(TSDataType dataType) {
     this.dataType = dataType;
   }
+
+  protected void resetFileName(TsFileResource resource, int version) {
+    // rename TsFile
+    File file = resource.getTsFile();
+    String[] fileInfo = file.getPath().split(FILE_NAME_SEPARATOR);
+    fileInfo[1] = String.valueOf(version);
+    String newFileName = StringUtil.join(fileInfo, FILE_NAME_SEPARATOR);
+    file.renameTo(new File(newFileName));
+
+    resource.setVersion(version);
+    resource.setFile(new File(newFileName));
+
+    // rename resource file
+    file = new File(resource.getTsFilePath() + TsFileResource.RESOURCE_SUFFIX);
+    file.renameTo(new File(newFileName + TsFileResource.RESOURCE_SUFFIX));
+
+    // rename mods file
+    file = new File(resource.getTsFilePath() + ModificationFile.FILE_SUFFIX);
+    file.renameTo(new File(newFileName + ModificationFile.FILE_SUFFIX));
+  }
 }
diff --git a/server/src/test/java/org/apache/iotdb/db/engine/compaction/cross/RewriteCrossSpaceCompactionWithFastPerformerTest.java b/server/src/test/java/org/apache/iotdb/db/engine/compaction/cross/RewriteCrossSpaceCompactionWithFastPerformerTest.java
index 7c8fd2c19ba..cd974b1c134 100644
--- a/server/src/test/java/org/apache/iotdb/db/engine/compaction/cross/RewriteCrossSpaceCompactionWithFastPerformerTest.java
+++ b/server/src/test/java/org/apache/iotdb/db/engine/compaction/cross/RewriteCrossSpaceCompactionWithFastPerformerTest.java
@@ -153,7 +153,6 @@ public class RewriteCrossSpaceCompactionWithFastPerformerTest extends AbstractCo
         i < TsFileGeneratorUtils.getAlignDeviceOffset() + 4;
         i++) {
       for (int j = 0; j < 5; j++) {
-        System.out.println(i + "," + j);
         List<IMeasurementSchema> schemas = new ArrayList<>();
         schemas.add(new MeasurementSchema("s" + j, TSDataType.INT64));
         AlignedPath path =
diff --git a/server/src/test/java/org/apache/iotdb/db/engine/compaction/utils/MultiTsFileDeviceIteratorTest.java b/server/src/test/java/org/apache/iotdb/db/engine/compaction/utils/MultiTsFileDeviceIteratorTest.java
index 3160f038396..3d4547a7eea 100644
--- a/server/src/test/java/org/apache/iotdb/db/engine/compaction/utils/MultiTsFileDeviceIteratorTest.java
+++ b/server/src/test/java/org/apache/iotdb/db/engine/compaction/utils/MultiTsFileDeviceIteratorTest.java
@@ -292,6 +292,8 @@ public class MultiTsFileDeviceIteratorTest extends AbstractCompactionTest {
     deleteTimeseriesInMManager(seriesPaths);
 
     createFiles(2, 10, 5, 100, 2000, 2000, 50, 50, true, true);
+    resetFileName(seqResources.get(7), 100);
+    resetFileName(seqResources.get(8), 101);
 
     // sort the deviceId in lexicographical order from small to large
     List<String> deviceIds = new ArrayList<>();
@@ -347,6 +349,8 @@ public class MultiTsFileDeviceIteratorTest extends AbstractCompactionTest {
 
     // generate d0 ~ d9 with aligned property
     createFiles(2, 10, 15, 100, 2000, 2000, 50, 50, true, true);
+    resetFileName(seqResources.get(7), 100);
+    resetFileName(seqResources.get(8), 101);
     tsFileManager.addAll(seqResources, true);
 
     // sort the deviceId in lexicographical order from small to large
@@ -491,6 +495,8 @@ public class MultiTsFileDeviceIteratorTest extends AbstractCompactionTest {
 
     // generate d0 ~ d9 with nonAligned property
     createFiles(2, 10, 15, 100, 2000, 2000, 50, 50, false, true);
+    resetFileName(seqResources.get(7), 100);
+    resetFileName(seqResources.get(8), 101);
     tsFileManager.addAll(seqResources, true);
 
     // sort the deviceId in lexicographical order from small to large
@@ -634,6 +640,8 @@ public class MultiTsFileDeviceIteratorTest extends AbstractCompactionTest {
 
     // generate d0 ~ d9 with aligned property
     createFiles(2, 10, 15, 100, 2000, 2000, 50, 50, true, true);
+    resetFileName(seqResources.get(7), 100);
+    resetFileName(seqResources.get(8), 101);
     tsFileManager.addAll(seqResources, true);
 
     // sort the deviceId in lexicographical order from small to large
@@ -777,6 +785,8 @@ public class MultiTsFileDeviceIteratorTest extends AbstractCompactionTest {
 
     // generate d0 ~ d9 with nonAligned property
     createFiles(2, 10, 15, 100, 2000, 2000, 50, 50, false, true);
+    resetFileName(seqResources.get(7), 100);
+    resetFileName(seqResources.get(8), 101);
     tsFileManager.addAll(seqResources, true);
 
     // sort the deviceId in lexicographical order from small to large
@@ -930,6 +940,8 @@ public class MultiTsFileDeviceIteratorTest extends AbstractCompactionTest {
 
     // generate d0 ~ d9 with aligned property
     createFiles(2, 10, 15, 100, 2000, 2000, 50, 50, true, true);
+    resetFileName(seqResources.get(7), 100);
+    resetFileName(seqResources.get(8), 101);
     tsFileManager.addAll(seqResources, true);
 
     // sort the deviceId in lexicographical order from small to large
@@ -1075,6 +1087,8 @@ public class MultiTsFileDeviceIteratorTest extends AbstractCompactionTest {
 
     // generate d0 ~ d9 with nonAligned property
     createFiles(2, 10, 15, 100, 2000, 2000, 50, 50, false, true);
+    resetFileName(seqResources.get(7), 100);
+    resetFileName(seqResources.get(8), 101);
     tsFileManager.addAll(seqResources, true);
 
     // sort the deviceId in lexicographical order from small to large