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/02/24 07:16:06 UTC
[GitHub] [iotdb] choubenson opened a new pull request #5113: [IOTDB-2603]Fix compaction recover
choubenson opened a new pull request #5113:
URL: https://github.com/apache/iotdb/pull/5113
1. There may be more data after iotdb restart. The reason is that cross space compaction recovery did not consisder one case, which is when some source files lost.
2. If there is deletion during cross space compaction process, if the iOTDB is stopped then and all the source files are not deleted, then the deleted data will be queried during the next restart.
--
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] jt2594838 commented on a change in pull request #5113: [To rel/0.12][IOTDB-2603]Fix compaction recover
Posted by GitBox <gi...@apache.org>.
jt2594838 commented on a change in pull request #5113:
URL: https://github.com/apache/iotdb/pull/5113#discussion_r813840365
##########
File path: server/src/main/java/org/apache/iotdb/db/engine/merge/task/RecoverMergeTask.java
##########
@@ -80,7 +95,39 @@ public void recoverMerge() throws IOException, MetadataException {
}
}
- private void resumeAfterFilesLogged() throws IOException {
+ private void handleWhenAllSourceFilesExist() throws IOException {
cleanUp(false);
+ tsFileManagement.removeMergingModification();
+ }
+
+ private void handleWhenSomeSourceFilesLost() throws IOException {
+ for (TsFileResource sourceSeqResource : resource.getSeqFiles()) {
+ File targetFile = modifyTsFileNameUnseqMergCnt(sourceSeqResource.getTsFile());
+ File targetFileResource = new File(targetFile.getPath() + TsFileResource.RESOURCE_SUFFIX);
+ File tmpTargetFile = new File(sourceSeqResource.getTsFilePath() + MERGE_SUFFIX);
+ // move to target file and target resource file
+ if (!targetFile.exists()) {
+ // move target file
+ FSFactoryProducer.getFSFactory().moveFile(tmpTargetFile, targetFile);
+ }
+ // serialize target resource file
+ TsFileResource targetTsFileResouce = new TsFileResource(targetFile);
+ try (TsFileSequenceReader reader = new TsFileSequenceReader(targetFile.getAbsolutePath())) {
+ FileLoaderUtils.updateTsFileResource(reader, targetTsFileResouce);
+ }
+ targetTsFileResouce.serialize();
Review comment:
Is this newly generated resource managed by TsFileManagement?
##########
File path: server/src/main/java/org/apache/iotdb/db/engine/merge/task/RecoverMergeTask.java
##########
@@ -80,7 +95,39 @@ public void recoverMerge() throws IOException, MetadataException {
}
}
- private void resumeAfterFilesLogged() throws IOException {
+ private void handleWhenAllSourceFilesExist() throws IOException {
cleanUp(false);
+ tsFileManagement.removeMergingModification();
+ }
+
+ private void handleWhenSomeSourceFilesLost() throws IOException {
+ for (TsFileResource sourceSeqResource : resource.getSeqFiles()) {
+ File targetFile = modifyTsFileNameUnseqMergCnt(sourceSeqResource.getTsFile());
+ File targetFileResource = new File(targetFile.getPath() + TsFileResource.RESOURCE_SUFFIX);
+ File tmpTargetFile = new File(sourceSeqResource.getTsFilePath() + MERGE_SUFFIX);
+ // move to target file and target resource file
+ if (!targetFile.exists()) {
+ // move target file
+ FSFactoryProducer.getFSFactory().moveFile(tmpTargetFile, targetFile);
+ }
+ // serialize target resource file
+ TsFileResource targetTsFileResouce = new TsFileResource(targetFile);
+ try (TsFileSequenceReader reader = new TsFileSequenceReader(targetFile.getAbsolutePath())) {
+ FileLoaderUtils.updateTsFileResource(reader, targetTsFileResouce);
+ }
+ targetTsFileResouce.serialize();
+
+ // delete source seq file
+ sourceSeqResource.remove();
+
+ // delete merge file
+ File mergeFile = new File(sourceSeqResource.getTsFilePath() + MERGE_SUFFIX);
+ if (mergeFile.exists()) {
+ mergeFile.delete();
+ }
+
+ sourceSeqResource.setFile(targetFile);
Review comment:
What is the point of `sourceSeqResource.setFile(targetFile);`?
--
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] choubenson commented on a change in pull request #5113: [To rel/0.12][IOTDB-2603]Fix compaction recover
Posted by GitBox <gi...@apache.org>.
choubenson commented on a change in pull request #5113:
URL: https://github.com/apache/iotdb/pull/5113#discussion_r814038302
##########
File path: server/src/main/java/org/apache/iotdb/db/engine/merge/task/RecoverMergeTask.java
##########
@@ -80,7 +95,39 @@ public void recoverMerge() throws IOException, MetadataException {
}
}
- private void resumeAfterFilesLogged() throws IOException {
+ private void handleWhenAllSourceFilesExist() throws IOException {
cleanUp(false);
+ tsFileManagement.removeMergingModification();
+ }
+
+ private void handleWhenSomeSourceFilesLost() throws IOException {
+ for (TsFileResource sourceSeqResource : resource.getSeqFiles()) {
+ File targetFile = modifyTsFileNameUnseqMergCnt(sourceSeqResource.getTsFile());
+ File targetFileResource = new File(targetFile.getPath() + TsFileResource.RESOURCE_SUFFIX);
+ File tmpTargetFile = new File(sourceSeqResource.getTsFilePath() + MERGE_SUFFIX);
+ // move to target file and target resource file
+ if (!targetFile.exists()) {
+ // move target file
+ FSFactoryProducer.getFSFactory().moveFile(tmpTargetFile, targetFile);
+ }
+ // serialize target resource file
+ TsFileResource targetTsFileResouce = new TsFileResource(targetFile);
+ try (TsFileSequenceReader reader = new TsFileSequenceReader(targetFile.getAbsolutePath())) {
+ FileLoaderUtils.updateTsFileResource(reader, targetTsFileResouce);
+ }
+ targetTsFileResouce.serialize();
Review comment:
Resolved. Here has changed to update resource memory in TsFileManagement.
--
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 change in pull request #5113: [To rel/0.12][IOTDB-2603]Fix compaction recover
Posted by GitBox <gi...@apache.org>.
THUMarkLau commented on a change in pull request #5113:
URL: https://github.com/apache/iotdb/pull/5113#discussion_r813643218
##########
File path: server/src/main/java/org/apache/iotdb/db/engine/merge/task/RecoverMergeTask.java
##########
@@ -80,7 +95,57 @@ public void recoverMerge() throws IOException, MetadataException {
}
}
- private void resumeAfterFilesLogged() throws IOException {
+ private void handleWhenAllSourceFilesExist() throws IOException {
cleanUp(false);
+ appendNewModificationToOldModsFile(resource.getSeqFiles());
+ appendNewModificationToOldModsFile(resource.getUnseqFiles());
+ tsFileManagement.removeMergingModification();
+ }
+
+ private void handleWhenSomeSourceFilesLost() throws IOException {
+ for (TsFileResource sourceSeqResource : resource.getSeqFiles()) {
+ File targetFile = modifyTsFileNameUnseqMergCnt(sourceSeqResource.getTsFile());
+ File targetFileResource = new File(targetFile.getPath() + TsFileResource.RESOURCE_SUFFIX);
+ File tmpTargetFile = new File(sourceSeqResource.getTsFilePath() + MERGE_SUFFIX);
+ // move to target file and target resource file
+ if (!targetFileResource.exists()) {
+ if (!targetFile.exists()) {
+ // move target file
+ FSFactoryProducer.getFSFactory().moveFile(tmpTargetFile, targetFile);
+ }
+ // move target resource file
+ FSFactoryProducer.getFSFactory()
Review comment:
If the target file already exists, its tsfileResource must exist, because SGP recovery generates a tsfileResource for a tsfile that does not have a tsfileResource (see `TsFileRecoverPerformer::recover`). If the target file does not exist and the corresponding temporary file does, the tsfileResource should be regenerated by reading from the temporary file rather than moving the tsfileResource of source file directly.
--
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 change in pull request #5113: [To rel/0.12][IOTDB-2603]Fix compaction recover
Posted by GitBox <gi...@apache.org>.
THUMarkLau commented on a change in pull request #5113:
URL: https://github.com/apache/iotdb/pull/5113#discussion_r813621228
##########
File path: server/src/assembly/resources/conf/iotdb-engine.properties
##########
@@ -451,11 +451,6 @@ timestamp_precision=ms
# If you are feeling the rebooting is too slow, set this to false, false by default
# continue_merge_after_reboot=false
-# When set to true, all unseq merges becomes full merge (the whole SeqFiles are re-written despite how
-# much they are overflowed). This may increase merge overhead depending on how much the SeqFiles
-# are overflowed.
-# force_full_merge=true
-
Review comment:
remove the argument in `IoTDBConfig` and `IoTDBDescriptor`
--
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] choubenson commented on a change in pull request #5113: [To rel/0.12][IOTDB-2603]Fix compaction recover
Posted by GitBox <gi...@apache.org>.
choubenson commented on a change in pull request #5113:
URL: https://github.com/apache/iotdb/pull/5113#discussion_r813732026
##########
File path: server/src/main/java/org/apache/iotdb/db/engine/merge/task/RecoverMergeTask.java
##########
@@ -80,7 +95,57 @@ public void recoverMerge() throws IOException, MetadataException {
}
}
- private void resumeAfterFilesLogged() throws IOException {
+ private void handleWhenAllSourceFilesExist() throws IOException {
cleanUp(false);
+ appendNewModificationToOldModsFile(resource.getSeqFiles());
+ appendNewModificationToOldModsFile(resource.getUnseqFiles());
+ tsFileManagement.removeMergingModification();
+ }
+
+ private void handleWhenSomeSourceFilesLost() throws IOException {
+ for (TsFileResource sourceSeqResource : resource.getSeqFiles()) {
+ File targetFile = modifyTsFileNameUnseqMergCnt(sourceSeqResource.getTsFile());
+ File targetFileResource = new File(targetFile.getPath() + TsFileResource.RESOURCE_SUFFIX);
+ File tmpTargetFile = new File(sourceSeqResource.getTsFilePath() + MERGE_SUFFIX);
+ // move to target file and target resource file
+ if (!targetFileResource.exists()) {
+ if (!targetFile.exists()) {
+ // move target file
+ FSFactoryProducer.getFSFactory().moveFile(tmpTargetFile, targetFile);
+ }
+ // move target resource file
+ FSFactoryProducer.getFSFactory()
Review comment:
resolved
##########
File path: server/src/assembly/resources/conf/iotdb-engine.properties
##########
@@ -451,11 +451,6 @@ timestamp_precision=ms
# If you are feeling the rebooting is too slow, set this to false, false by default
# continue_merge_after_reboot=false
-# When set to true, all unseq merges becomes full merge (the whole SeqFiles are re-written despite how
-# much they are overflowed). This may increase merge overhead depending on how much the SeqFiles
-# are overflowed.
-# force_full_merge=true
-
Review comment:
resolved
--
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] choubenson commented on a change in pull request #5113: [To rel/0.12][IOTDB-2603]Fix compaction recover
Posted by GitBox <gi...@apache.org>.
choubenson commented on a change in pull request #5113:
URL: https://github.com/apache/iotdb/pull/5113#discussion_r814050539
##########
File path: server/src/main/java/org/apache/iotdb/db/engine/merge/task/RecoverMergeTask.java
##########
@@ -80,7 +95,39 @@ public void recoverMerge() throws IOException, MetadataException {
}
}
- private void resumeAfterFilesLogged() throws IOException {
+ private void handleWhenAllSourceFilesExist() throws IOException {
cleanUp(false);
+ tsFileManagement.removeMergingModification();
+ }
+
+ private void handleWhenSomeSourceFilesLost() throws IOException {
+ for (TsFileResource sourceSeqResource : resource.getSeqFiles()) {
+ File targetFile = modifyTsFileNameUnseqMergCnt(sourceSeqResource.getTsFile());
+ File targetFileResource = new File(targetFile.getPath() + TsFileResource.RESOURCE_SUFFIX);
+ File tmpTargetFile = new File(sourceSeqResource.getTsFilePath() + MERGE_SUFFIX);
+ // move to target file and target resource file
+ if (!targetFile.exists()) {
+ // move target file
+ FSFactoryProducer.getFSFactory().moveFile(tmpTargetFile, targetFile);
+ }
+ // serialize target resource file
+ TsFileResource targetTsFileResouce = new TsFileResource(targetFile);
+ try (TsFileSequenceReader reader = new TsFileSequenceReader(targetFile.getAbsolutePath())) {
+ FileLoaderUtils.updateTsFileResource(reader, targetTsFileResouce);
+ }
+ targetTsFileResouce.serialize();
+
+ // delete source seq file
+ sourceSeqResource.remove();
+
+ // delete merge file
+ File mergeFile = new File(sourceSeqResource.getTsFilePath() + MERGE_SUFFIX);
+ if (mergeFile.exists()) {
+ mergeFile.delete();
+ }
+
+ sourceSeqResource.setFile(targetFile);
Review comment:
In order to update the mods of the target file in the cleanUp method, I did not notice that 0.12 is recoverTsFile before merge recovery, so we need to update the memory. It's a mistake and has been fixed.
--
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] jt2594838 merged pull request #5113: [To rel/0.12][IOTDB-2603]Fix compaction recover
Posted by GitBox <gi...@apache.org>.
jt2594838 merged pull request #5113:
URL: https://github.com/apache/iotdb/pull/5113
--
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