(iotdb) branch compaction_review updated: fix comments

[ji...@apache.org]

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

jiangtian pushed a commit to branch compaction_review
in repository https://gitbox.apache.org/repos/asf/iotdb.git


The following commit(s) were added to refs/heads/compaction_review by this push:
     new 35d47387bfe fix comments
35d47387bfe is described below

commit 35d47387bfe0646d29943715d20e8c9616cf3889
Author: jt2594838 <jt...@163.com>
AuthorDate: Tue Jun 11 09:40:41 2024 +0800

    fix comments
---
 .../task/InsertionCrossSpaceCompactionTask.java    |  3 ++-
 .../utils/CrossSpaceCompactionCandidate.java       | 30 +++++++++++++++-------
 2 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/storageengine/dataregion/compaction/execute/task/InsertionCrossSpaceCompactionTask.java b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/storageengine/dataregion/compaction/execute/task/InsertionCrossSpaceCompactionTask.java
index 32c92678eef..41ac8ea7111 100644
--- a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/storageengine/dataregion/compaction/execute/task/InsertionCrossSpaceCompactionTask.java
+++ b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/storageengine/dataregion/compaction/execute/task/InsertionCrossSpaceCompactionTask.java
@@ -266,9 +266,10 @@ public class InsertionCrossSpaceCompactionTask extends AbstractCompactionTask {
         throw new CompactionRecoverException("Can not recover InsertionCrossSpaceCompactionTask");
       }
       if (shouldRollback()) {
+        // the target file is not fully generated, remove the partially generated result
         rollback();
       } else {
-        // That finishTask() is revoked means
+        //  the target file is fully generated, remove the source file
         finishTask();
       }
     } catch (Exception e) {
diff --git a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/storageengine/dataregion/compaction/selector/utils/CrossSpaceCompactionCandidate.java b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/storageengine/dataregion/compaction/selector/utils/CrossSpaceCompactionCandidate.java
index de75a3d9d21..8cf29534bba 100644
--- a/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/storageengine/dataregion/compaction/selector/utils/CrossSpaceCompactionCandidate.java
+++ b/iotdb-core/datanode/src/main/java/org/apache/iotdb/db/storageengine/dataregion/compaction/selector/utils/CrossSpaceCompactionCandidate.java
@@ -63,6 +63,8 @@ public class CrossSpaceCompactionCandidate {
     if (nextUnseqFileIndex >= unseqFiles.size()) {
       return false;
     }
+    // unseq files should be compacted in order to avoid old data overwriting new data
+    // if any unseq file fails to be selected, the remaining ones cannot be selected either
     return prepareNextSplit();
   }
 
@@ -78,7 +80,8 @@ public class CrossSpaceCompactionCandidate {
     // check one by one. And we cannot skip any device in the unseq file because it may lead to
     // omission of target seq file
     if (!unseqFile.hasDetailedDeviceInfo()) {
-      // unseq file resource has been deleted due to TTL and cannot upgrade to DEVICE_TIME_INDEX
+      // unseq file resource is unsealed or has been deleted due to TTL and cannot upgrade to
+      // DEVICE_TIME_INDEX
       return false;
     }
     for (DeviceInfo unseqDeviceInfo : unseqFile.getDevices()) {
@@ -87,6 +90,12 @@ public class CrossSpaceCompactionCandidate {
       // The `previousSeqFile` means the seqFile which contains the device and its endTime is just
       // be smaller than startTime of the device in unseqFile
       TsFileResourceCandidate previousSeqFile = null;
+
+      // if a seq file actually overlaps or effectively overlaps with the unseq file, the seq
+      // file should be merged with the unseq file
+      // e.g.: two seq files seq1 and seq2. seq1 ranges [0, 100] and seq2 ranges [200, 300]. One
+      // unseq file unseq1 ranges [80, 150]. Then the [80, 100] part from unseq1 actually
+      // overlaps with seq1, while the [101, 150] part effectively overlaps with seq2.
       for (TsFileResourceCandidate seqFile : seqFiles) {
         // If the seqFile may need to be selected but its invalid, the selection should be
         // terminated.
@@ -122,14 +131,17 @@ public class CrossSpaceCompactionCandidate {
           }
         }
       }
-      // Most of cases, one unsetFile should have at least one conresponding seqFile whose startTime
-      // is larger than unseqFiles's endTime for each device in unseqFile. But some scenario will
-      // break this rule such as:
-      // 1. Delete or TTL operation deletes the seqFile
-      // 2. the unseqFile is created by load operation
-      // In these scenario, the data for soem device in unseqFile will be written to wrong seqFile,
-      // which lead to failed overlap check after compaction. The following changes ensure the
-      // correct seqFile won't be lost in this selection.
+
+      // If: 1.some seq files are removed by deletion or TTL; 2. the unseq file is loaded; the
+      // unseq file may have larger timestamps and does not overlap with any seqFile.
+      //
+      // For example, previously we have two seqFiles, ranging [0, 100] [200, 300] respectively.
+      // Then the seqFile of [200, 300] is deleted and an unseqFile of [150, 180] is written/loaded.
+      // The unseqFile does not overlap with any seqFile and not seqFile has larger timestamp
+      // than it, thus the unseqFile cannot select any candidate seqFiles throughput the loop above.
+      //
+      // In this case, the unseqFile should be merged with the last seqFile if possible.
+      // TODO: let insertionCompaction handle this case
 
       // That this judgement is true indicates `previousSeqFile` is unnecessary.
       if (atLeastOneSeqFileSelected || previousSeqFile == null) {