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/03/28 15:45:40 UTC

[GitHub] [iotdb] THUMarkLau opened a new pull request #5363: [IOTDB-2811] Fix compaction exception handle failure cause by deletion of storage group

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


   The detail report see [IOTDB-2811](https://issues.apache.org/jira/projects/IOTDB/issues/IOTDB-2811?filter=allopenissues)


-- 
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] qiaojialin merged pull request #5363: [IOTDB-2811] Fix compaction exception handle failure cause by deletion of storage group

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


   


-- 
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 #5363: [IOTDB-2811] Fix compaction exception handle failure cause by deletion of storage group

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



##########
File path: server/src/main/java/org/apache/iotdb/db/engine/compaction/CompactionTaskManager.java
##########
@@ -290,35 +292,34 @@ public synchronized void removeRunningTaskFromList(AbstractCompactionTask task)
    *
    * @throws RejectedExecutionException
    */
-  public synchronized void submitTask(Callable<Void> compactionMergeTask)
+  public synchronized Future<Void> submitTask(AbstractCompactionTask compactionTask)
       throws RejectedExecutionException {
     if (taskExecutionPool != null && !taskExecutionPool.isTerminated()) {
-      taskExecutionPool.submit(compactionMergeTask);
-      return;
+      Future<Void> future = taskExecutionPool.submit(compactionTask);
+      runningCompactionTaskList.add(compactionTask);
+      futureList.add(new Pair<>(compactionTask, future));
+      return future;
     }
     logger.warn(
         "A CompactionTask failed to be submitted to CompactionTaskManager because {}",
         taskExecutionPool == null
             ? "taskExecutionPool is null"
             : "taskExecutionPool is terminated");
+    return null;
   }
 
   /**
    * Abort all compactions of a storage group. The caller must acquire the write lock of the
    * corresponding storage group.
    */
-  public void abortCompaction(String fullStorageGroupName) {
-    Set<Future<Void>> subTasks =
-        storageGroupTasks.getOrDefault(fullStorageGroupName, Collections.emptySet());
-    candidateCompactionTaskQueue.clear();
-    Iterator<Future<Void>> subIterator = subTasks.iterator();
-    while (subIterator.hasNext()) {
-      Future<Void> next = subIterator.next();
-      if (!next.isDone() && !next.isCancelled()) {
-        next.cancel(true);
+  public synchronized void abortCompaction(String storageGroupName) {
+    for (Pair<AbstractCompactionTask, Future<Void>> futurePair : futureList) {
+      if (futurePair.left.getFullStorageGroupName().contains(storageGroupName)) {
+        futurePair.right.cancel(true);
       }
-      subIterator.remove();
     }
+
+    candidateCompactionTaskQueue.clear();

Review comment:
       Got it.




-- 
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 #5363: [IOTDB-2811] Fix compaction exception handle failure cause by deletion of storage group

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



##########
File path: server/src/test/java/org/apache/iotdb/db/engine/storagegroup/StorageGroupProcessorTest.java
##########
@@ -705,6 +711,53 @@ public void testMerge()
         .setEnableUnseqSpaceCompaction(originEnableUnseqSpaceCompaction);
   }
 
+  @Test
+  public void testDeleteStorageGroupWhenCompacting() throws Exception {
+    IoTDBDescriptor.getInstance().getConfig().setMaxInnerCompactionCandidateFileNum(10);
+    try {
+      for (int j = 0; j < 10; j++) {
+        TSRecord record = new TSRecord(j, deviceId);
+        record.addTuple(DataPoint.getDataPoint(TSDataType.INT32, measurementId, String.valueOf(j)));
+        processor.insert(new InsertRowPlan(record));
+        processor.asyncCloseAllWorkingTsFileProcessors();
+      }
+      processor.syncCloseAllWorkingTsFileProcessors();
+      SizeTieredCompactionTask task =
+          new SizeTieredCompactionTask(
+              storageGroup,
+              "0",
+              0,
+              processor.getTsFileManager(),
+              processor.getSequenceFileList(),
+              true,
+              new AtomicInteger(0));
+      Future<Void> future = CompactionTaskManager.getInstance().submitTask(task);
+      Thread.sleep(20);
+      StorageEngine.getInstance().deleteStorageGroup(new PartialPath(storageGroup));
+      Thread.sleep(500);
+
+      for (TsFileResource resource : processor.getSequenceFileList()) {
+        Assert.assertTrue(resource.getTsFile().exists());
+      }

Review comment:
       resolve




-- 
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 #5363: [IOTDB-2811] Fix compaction exception handle failure cause by deletion of storage group

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



##########
File path: server/src/main/java/org/apache/iotdb/db/engine/compaction/CompactionTaskManager.java
##########
@@ -290,35 +292,34 @@ public synchronized void removeRunningTaskFromList(AbstractCompactionTask task)
    *
    * @throws RejectedExecutionException
    */
-  public synchronized void submitTask(Callable<Void> compactionMergeTask)
+  public synchronized Future<Void> submitTask(AbstractCompactionTask compactionTask)
       throws RejectedExecutionException {
     if (taskExecutionPool != null && !taskExecutionPool.isTerminated()) {
-      taskExecutionPool.submit(compactionMergeTask);
-      return;
+      Future<Void> future = taskExecutionPool.submit(compactionTask);
+      runningCompactionTaskList.add(compactionTask);
+      futureList.add(new Pair<>(compactionTask, future));
+      return future;
     }
     logger.warn(
         "A CompactionTask failed to be submitted to CompactionTaskManager because {}",
         taskExecutionPool == null
             ? "taskExecutionPool is null"
             : "taskExecutionPool is terminated");
+    return null;
   }
 
   /**
    * Abort all compactions of a storage group. The caller must acquire the write lock of the
    * corresponding storage group.
    */
-  public void abortCompaction(String fullStorageGroupName) {
-    Set<Future<Void>> subTasks =
-        storageGroupTasks.getOrDefault(fullStorageGroupName, Collections.emptySet());
-    candidateCompactionTaskQueue.clear();
-    Iterator<Future<Void>> subIterator = subTasks.iterator();
-    while (subIterator.hasNext()) {
-      Future<Void> next = subIterator.next();
-      if (!next.isDone() && !next.isCancelled()) {
-        next.cancel(true);
+  public synchronized void abortCompaction(String storageGroupName) {
+    for (Pair<AbstractCompactionTask, Future<Void>> futurePair : futureList) {
+      if (futurePair.left.getFullStorageGroupName().contains(storageGroupName)) {
+        futurePair.right.cancel(true);
       }
-      subIterator.remove();
     }
+
+    candidateCompactionTaskQueue.clear();

Review comment:
       Is it necessary to remove the task from` futureList` and `runningCompactionTaskList` here?

##########
File path: server/src/test/java/org/apache/iotdb/db/engine/storagegroup/StorageGroupProcessorTest.java
##########
@@ -705,6 +711,53 @@ public void testMerge()
         .setEnableUnseqSpaceCompaction(originEnableUnseqSpaceCompaction);
   }
 
+  @Test
+  public void testDeleteStorageGroupWhenCompacting() throws Exception {
+    IoTDBDescriptor.getInstance().getConfig().setMaxInnerCompactionCandidateFileNum(10);
+    try {
+      for (int j = 0; j < 10; j++) {
+        TSRecord record = new TSRecord(j, deviceId);
+        record.addTuple(DataPoint.getDataPoint(TSDataType.INT32, measurementId, String.valueOf(j)));
+        processor.insert(new InsertRowPlan(record));
+        processor.asyncCloseAllWorkingTsFileProcessors();
+      }
+      processor.syncCloseAllWorkingTsFileProcessors();
+      SizeTieredCompactionTask task =
+          new SizeTieredCompactionTask(
+              storageGroup,
+              "0",
+              0,
+              processor.getTsFileManager(),
+              processor.getSequenceFileList(),
+              true,
+              new AtomicInteger(0));
+      Future<Void> future = CompactionTaskManager.getInstance().submitTask(task);
+      Thread.sleep(20);
+      StorageEngine.getInstance().deleteStorageGroup(new PartialPath(storageGroup));
+      Thread.sleep(500);
+
+      for (TsFileResource resource : processor.getSequenceFileList()) {
+        Assert.assertTrue(resource.getTsFile().exists());
+      }

Review comment:
       Source files should not exist after deleting sg.




-- 
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 #5363: [IOTDB-2811] Fix compaction exception handle failure cause by deletion of storage group

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



##########
File path: server/src/main/java/org/apache/iotdb/db/engine/compaction/CompactionTaskManager.java
##########
@@ -290,35 +292,34 @@ public synchronized void removeRunningTaskFromList(AbstractCompactionTask task)
    *
    * @throws RejectedExecutionException
    */
-  public synchronized void submitTask(Callable<Void> compactionMergeTask)
+  public synchronized Future<Void> submitTask(AbstractCompactionTask compactionTask)
       throws RejectedExecutionException {
     if (taskExecutionPool != null && !taskExecutionPool.isTerminated()) {
-      taskExecutionPool.submit(compactionMergeTask);
-      return;
+      Future<Void> future = taskExecutionPool.submit(compactionTask);
+      runningCompactionTaskList.add(compactionTask);
+      futureList.add(new Pair<>(compactionTask, future));
+      return future;
     }
     logger.warn(
         "A CompactionTask failed to be submitted to CompactionTaskManager because {}",
         taskExecutionPool == null
             ? "taskExecutionPool is null"
             : "taskExecutionPool is terminated");
+    return null;
   }
 
   /**
    * Abort all compactions of a storage group. The caller must acquire the write lock of the
    * corresponding storage group.
    */
-  public void abortCompaction(String fullStorageGroupName) {
-    Set<Future<Void>> subTasks =
-        storageGroupTasks.getOrDefault(fullStorageGroupName, Collections.emptySet());
-    candidateCompactionTaskQueue.clear();
-    Iterator<Future<Void>> subIterator = subTasks.iterator();
-    while (subIterator.hasNext()) {
-      Future<Void> next = subIterator.next();
-      if (!next.isDone() && !next.isCancelled()) {
-        next.cancel(true);
+  public synchronized void abortCompaction(String storageGroupName) {
+    for (Pair<AbstractCompactionTask, Future<Void>> futurePair : futureList) {
+      if (futurePair.left.getFullStorageGroupName().contains(storageGroupName)) {
+        futurePair.right.cancel(true);
       }
-      subIterator.remove();
     }
+
+    candidateCompactionTaskQueue.clear();

Review comment:
       When the compaction task finish, it will remove from the `futureList` and `runningCompactionTaskList` itself 

##########
File path: server/src/main/java/org/apache/iotdb/db/engine/compaction/CompactionTaskManager.java
##########
@@ -290,35 +292,34 @@ public synchronized void removeRunningTaskFromList(AbstractCompactionTask task)
    *
    * @throws RejectedExecutionException
    */
-  public synchronized void submitTask(Callable<Void> compactionMergeTask)
+  public synchronized Future<Void> submitTask(AbstractCompactionTask compactionTask)
       throws RejectedExecutionException {
     if (taskExecutionPool != null && !taskExecutionPool.isTerminated()) {
-      taskExecutionPool.submit(compactionMergeTask);
-      return;
+      Future<Void> future = taskExecutionPool.submit(compactionTask);
+      runningCompactionTaskList.add(compactionTask);
+      futureList.add(new Pair<>(compactionTask, future));
+      return future;
     }
     logger.warn(
         "A CompactionTask failed to be submitted to CompactionTaskManager because {}",
         taskExecutionPool == null
             ? "taskExecutionPool is null"
             : "taskExecutionPool is terminated");
+    return null;
   }
 
   /**
    * Abort all compactions of a storage group. The caller must acquire the write lock of the
    * corresponding storage group.
    */
-  public void abortCompaction(String fullStorageGroupName) {
-    Set<Future<Void>> subTasks =
-        storageGroupTasks.getOrDefault(fullStorageGroupName, Collections.emptySet());
-    candidateCompactionTaskQueue.clear();
-    Iterator<Future<Void>> subIterator = subTasks.iterator();
-    while (subIterator.hasNext()) {
-      Future<Void> next = subIterator.next();
-      if (!next.isDone() && !next.isCancelled()) {
-        next.cancel(true);
+  public synchronized void abortCompaction(String storageGroupName) {
+    for (Pair<AbstractCompactionTask, Future<Void>> futurePair : futureList) {
+      if (futurePair.left.getFullStorageGroupName().contains(storageGroupName)) {
+        futurePair.right.cancel(true);
       }
-      subIterator.remove();
     }
+
+    candidateCompactionTaskQueue.clear();

Review comment:
       When the compaction task finish, it will remove from the `futureList` and `runningCompactionTaskList` by itself 




-- 
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 #5363: [IOTDB-2811] Fix compaction exception handle failure cause by deletion of storage group

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



##########
File path: server/src/main/java/org/apache/iotdb/db/engine/compaction/CompactionTaskManager.java
##########
@@ -290,35 +292,34 @@ public synchronized void removeRunningTaskFromList(AbstractCompactionTask task)
    *
    * @throws RejectedExecutionException
    */
-  public synchronized void submitTask(Callable<Void> compactionMergeTask)
+  public synchronized Future<Void> submitTask(AbstractCompactionTask compactionTask)
       throws RejectedExecutionException {
     if (taskExecutionPool != null && !taskExecutionPool.isTerminated()) {
-      taskExecutionPool.submit(compactionMergeTask);
-      return;
+      Future<Void> future = taskExecutionPool.submit(compactionTask);
+      runningCompactionTaskList.add(compactionTask);
+      futureList.add(new Pair<>(compactionTask, future));
+      return future;
     }
     logger.warn(
         "A CompactionTask failed to be submitted to CompactionTaskManager because {}",
         taskExecutionPool == null
             ? "taskExecutionPool is null"
             : "taskExecutionPool is terminated");
+    return null;
   }
 
   /**
    * Abort all compactions of a storage group. The caller must acquire the write lock of the
    * corresponding storage group.
    */
-  public void abortCompaction(String fullStorageGroupName) {
-    Set<Future<Void>> subTasks =
-        storageGroupTasks.getOrDefault(fullStorageGroupName, Collections.emptySet());
-    candidateCompactionTaskQueue.clear();
-    Iterator<Future<Void>> subIterator = subTasks.iterator();
-    while (subIterator.hasNext()) {
-      Future<Void> next = subIterator.next();
-      if (!next.isDone() && !next.isCancelled()) {
-        next.cancel(true);
+  public synchronized void abortCompaction(String storageGroupName) {
+    for (Pair<AbstractCompactionTask, Future<Void>> futurePair : futureList) {
+      if (futurePair.left.getFullStorageGroupName().contains(storageGroupName)) {
+        futurePair.right.cancel(true);
       }
-      subIterator.remove();
     }
+
+    candidateCompactionTaskQueue.clear();

Review comment:
       Remove the task from `futureList` and `runningCompactionTaskList` 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] THUMarkLau commented on a change in pull request #5363: [IOTDB-2811] Fix compaction exception handle failure cause by deletion of storage group

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



##########
File path: server/src/main/java/org/apache/iotdb/db/engine/compaction/CompactionTaskManager.java
##########
@@ -290,35 +292,34 @@ public synchronized void removeRunningTaskFromList(AbstractCompactionTask task)
    *
    * @throws RejectedExecutionException
    */
-  public synchronized void submitTask(Callable<Void> compactionMergeTask)
+  public synchronized Future<Void> submitTask(AbstractCompactionTask compactionTask)
       throws RejectedExecutionException {
     if (taskExecutionPool != null && !taskExecutionPool.isTerminated()) {
-      taskExecutionPool.submit(compactionMergeTask);
-      return;
+      Future<Void> future = taskExecutionPool.submit(compactionTask);
+      runningCompactionTaskList.add(compactionTask);
+      futureList.add(new Pair<>(compactionTask, future));
+      return future;
     }
     logger.warn(
         "A CompactionTask failed to be submitted to CompactionTaskManager because {}",
         taskExecutionPool == null
             ? "taskExecutionPool is null"
             : "taskExecutionPool is terminated");
+    return null;
   }
 
   /**
    * Abort all compactions of a storage group. The caller must acquire the write lock of the
    * corresponding storage group.
    */
-  public void abortCompaction(String fullStorageGroupName) {
-    Set<Future<Void>> subTasks =
-        storageGroupTasks.getOrDefault(fullStorageGroupName, Collections.emptySet());
-    candidateCompactionTaskQueue.clear();
-    Iterator<Future<Void>> subIterator = subTasks.iterator();
-    while (subIterator.hasNext()) {
-      Future<Void> next = subIterator.next();
-      if (!next.isDone() && !next.isCancelled()) {
-        next.cancel(true);
+  public synchronized void abortCompaction(String storageGroupName) {
+    for (Pair<AbstractCompactionTask, Future<Void>> futurePair : futureList) {
+      if (futurePair.left.getFullStorageGroupName().contains(storageGroupName)) {
+        futurePair.right.cancel(true);
       }
-      subIterator.remove();
     }
+
+    candidateCompactionTaskQueue.clear();

Review comment:
       When the compaction task finish, it will remove from the `futureList` and `runningCompactionTaskList` by itself. We don't need to remove it 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] qiaojialin commented on a change in pull request #5363: [IOTDB-2811] Fix compaction exception handle failure cause by deletion of storage group

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



##########
File path: server/src/main/java/org/apache/iotdb/db/engine/compaction/CompactionTaskManager.java
##########
@@ -290,35 +292,34 @@ public synchronized void removeRunningTaskFromList(AbstractCompactionTask task)
    *
    * @throws RejectedExecutionException
    */
-  public synchronized void submitTask(Callable<Void> compactionMergeTask)
+  public synchronized Future<Void> submitTask(AbstractCompactionTask compactionTask)

Review comment:
       The returned Future is not used anywhere.

##########
File path: server/src/main/java/org/apache/iotdb/db/engine/compaction/CompactionTaskManager.java
##########
@@ -64,8 +60,8 @@
       new FixedPriorityBlockingQueue<>(1024, new CompactionTaskComparator());
   // <logicalStorageGroupName,futureSet>, it is used to terminate all compaction tasks under the

Review comment:
       logicalstoragegroup-virtualstoragegroup=fullsgname




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