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 2021/02/26 05:58:39 UTC

[GitHub] [iotdb] zhanglingzhe0820 opened a new pull request #2739: Fix unseq merge data overlap bug

zhanglingzhe0820 opened a new pull request #2739:
URL: https://github.com/apache/iotdb/pull/2739


   The unseq merge selector try to filter unseq files which overlapped with some open file. However, the open file's endTime is always Long.MIN_VALUE which will be filtered in pervious logic. So unseq files which overlapped with some open file cannot be excluded.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

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



[GitHub] [iotdb] jixuan1989 edited a comment on pull request #2739: Fix unseq merge data overlap bug

Posted by GitBox <gi...@apache.org>.
jixuan1989 edited a comment on pull request #2739:
URL: https://github.com/apache/iotdb/pull/2739#issuecomment-786602948


   > The unseq merge selector try to filter unseq files which overlapped with some open file. However, the open file's endTime is always Long.MIN_VALUE which will be filtered in pervious logic. So unseq files which overlapped with some open file cannot be excluded.
   
   ## Behavior
   
   ...
   
   ## Reasom
   
   The unseq merge selector tries to find unseq files which overlap with some unsealed files. 
   
   However, the unsealed file's endTime is always Long.MIN_VALUE which will be ignored because [Long.MIN_VALUE, Long.MIN_VALUE) never overlap with other files . 
   
   As a result,  the unseq files which overlapped with some unsealed file cannot be excluded. (what is this meaning????)
   
   ## Solution
   
   .....
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

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



[GitHub] [iotdb] jixuan1989 edited a comment on pull request #2739: Fix unseq merge data overlap bug

Posted by GitBox <gi...@apache.org>.
jixuan1989 edited a comment on pull request #2739:
URL: https://github.com/apache/iotdb/pull/2739#issuecomment-786602948


   > The unseq merge selector try to filter unseq files which overlapped with some open file. However, the open file's endTime is always Long.MIN_VALUE which will be filtered in pervious logic. So unseq files which overlapped with some open file cannot be excluded.
   
   ## Behavior
   
   ...
   
   ## Reason
   
   The unseq merge selector tries to find unseq files which overlap with some unsealed files. 
   
   However, the unsealed file's endTime is always Long.MIN_VALUE which will be ignored because [Long.MIN_VALUE, Long.MIN_VALUE) never overlap with other files . 
   
   As a result,  the unseq files which overlapped with some unsealed file cannot be excluded. (what is this meaning????)
   
   ## Solution
   
   .....
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

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



[GitHub] [iotdb] jixuan1989 commented on pull request #2739: Fix unseq merge data overlap bug

Posted by GitBox <gi...@apache.org>.
jixuan1989 commented on pull request #2739:
URL: https://github.com/apache/iotdb/pull/2739#issuecomment-786602948


   > The unseq merge selector try to filter unseq files which overlapped with some open file. However, the open file's endTime is always Long.MIN_VALUE which will be filtered in pervious logic. So unseq files which overlapped with some open file cannot be excluded.
   
   ## Behavior
   
   ## Reasom
   
   The unseq merge selector tries to find unseq files which overlap with some unsealed files. 
   
   However, the unsealed file's endTime is always Long.MIN_VALUE which will be ignored because [Long.MIN_VALUE, Long.MIN_VALUE) never overlap with other files . 
   
   As a result,  the unseq files which overlapped with some unsealed file cannot be excluded. (what is this meaning????)
   
   ## Solution
   
   .....
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

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



[GitHub] [iotdb] zhanglingzhe0820 commented on pull request #2739: [ISSUE-2746] Fix data overlapped bug after unseq compaction

Posted by GitBox <gi...@apache.org>.
zhanglingzhe0820 commented on pull request #2739:
URL: https://github.com/apache/iotdb/pull/2739#issuecomment-787778986


   cherry-pick to 0.11 [PR](https://github.com/apache/iotdb/pull/2750)


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

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



[GitHub] [iotdb] qiaojialin merged pull request #2739: [ISSUE-2746] Fix data overlapped bug after unseq compaction

Posted by GitBox <gi...@apache.org>.
qiaojialin merged pull request #2739:
URL: https://github.com/apache/iotdb/pull/2739


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

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



[GitHub] [iotdb] wangchao316 commented on a change in pull request #2739: [ISSUE-2746] Fix data overlapped bug after unseq compaction

Posted by GitBox <gi...@apache.org>.
wangchao316 commented on a change in pull request #2739:
URL: https://github.com/apache/iotdb/pull/2739#discussion_r584429634



##########
File path: server/src/main/java/org/apache/iotdb/db/engine/merge/selector/MaxFileMergeFileSelector.java
##########
@@ -228,7 +227,7 @@ private void selectOverlappedSeqFiles(TsFileResource unseqFile) {
         if (seqSelected[i] || !seqFile.getDevices().contains(deviceId)) {
           continue;
         }
-        long seqEndTime = seqFile.getEndTime(deviceId);
+        long seqEndTime = seqFile.isClosed() ? seqFile.getEndTime(deviceId) : Long.MAX_VALUE;

Review comment:
       that is ok... 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

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



[GitHub] [iotdb] zhanglingzhe0820 commented on a change in pull request #2739: [ISSUE-2746] Fix data overlapped bug after unseq compaction

Posted by GitBox <gi...@apache.org>.
zhanglingzhe0820 commented on a change in pull request #2739:
URL: https://github.com/apache/iotdb/pull/2739#discussion_r584408981



##########
File path: server/src/main/java/org/apache/iotdb/db/engine/merge/selector/MaxFileMergeFileSelector.java
##########
@@ -228,7 +227,7 @@ private void selectOverlappedSeqFiles(TsFileResource unseqFile) {
         if (seqSelected[i] || !seqFile.getDevices().contains(deviceId)) {
           continue;
         }
-        long seqEndTime = seqFile.getEndTime(deviceId);
+        long seqEndTime = seqFile.isClosed() ? seqFile.getEndTime(deviceId) : Long.MAX_VALUE;

Review comment:
       > hi @zhanglingzhe0820 @jixuan1989 @qiaojialin ,I have a question.
   > if a seqfile do not closed. and then is there a conflict? flushing and merging...
   > 
   > 1. merging finished, need delete old tsfile , if this seqfile do not closed,  can this delete ?
   > 2. if flushing data  when merging...  , maybe have a chunk data do not write tsfile, but this tsfile merged..
   hi
   1. unsealed seq file will not be selected to a merge task so it will not be deleted.
   2. a file which is flushing is also an unsealed file.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

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



[GitHub] [iotdb] wangchao316 commented on a change in pull request #2739: Fix unseq merge data overlap bug

Posted by GitBox <gi...@apache.org>.
wangchao316 commented on a change in pull request #2739:
URL: https://github.com/apache/iotdb/pull/2739#discussion_r584023843



##########
File path: server/src/main/java/org/apache/iotdb/db/engine/merge/selector/MaxFileMergeFileSelector.java
##########
@@ -228,7 +227,7 @@ private void selectOverlappedSeqFiles(TsFileResource unseqFile) {
         if (seqSelected[i] || !seqFile.getDevices().contains(deviceId)) {
           continue;
         }
-        long seqEndTime = seqFile.getEndTime(deviceId);
+        long seqEndTime = seqFile.isClosed() ? seqFile.getEndTime(deviceId) : Long.MAX_VALUE;

Review comment:
       hi ,I have a question.
   if a seqfile do not closed. and then  is there a conflict?  flushing and merging...




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

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



[GitHub] [iotdb] qiaojialin commented on a change in pull request #2739: Fix unseq merge data overlap bug

Posted by GitBox <gi...@apache.org>.
qiaojialin commented on a change in pull request #2739:
URL: https://github.com/apache/iotdb/pull/2739#discussion_r583455132



##########
File path: server/src/main/java/org/apache/iotdb/db/engine/merge/selector/MaxFileMergeFileSelector.java
##########
@@ -228,7 +227,7 @@ private void selectOverlappedSeqFiles(TsFileResource unseqFile) {
         if (seqSelected[i] || !seqFile.getDevices().contains(deviceId)) {
           continue;
         }
-        long seqEndTime = seqFile.getEndTime(deviceId);
+        long seqEndTime = seqFile.isClosed() ? seqFile.getEndTime(deviceId) : Long.MAX_VALUE;

Review comment:
       add a javadoc for TsFileResource.getEndTime()




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

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



[GitHub] [iotdb] wangchao316 commented on pull request #2739: Fix unseq merge data overlap bug

Posted by GitBox <gi...@apache.org>.
wangchao316 commented on pull request #2739:
URL: https://github.com/apache/iotdb/pull/2739#issuecomment-786997862


   hi  @zhanglingzhe0820 @jixuan1989 @qiaojialin ,I have a question.
   if a seqfile do not closed. and then is there a conflict? flushing and merging...
   1. merging finished, need delete old tsfile , if this seqfile do not closed,  can this delete ?
   2. if flushing data  when merging...  , maybe have a chunk data do not write tsfile, but this tsfile merged..


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

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