[GitHub] [hbase] saintstack commented on a change in pull request #3261: HBASE-25880 remove files from filesCompacting when clear compaction queue

[GitBox <gi...@apache.org>]

saintstack commented on a change in pull request #3261:
URL: https://github.com/apache/hbase/pull/3261#discussion_r633285717



##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CompactSplit.java
##########
@@ -794,13 +795,30 @@ void shutdownLongCompactions(){
   }
 
   public void clearLongCompactionsQueue() {
+    removeFromFilesCompacting(longCompactions);

Review comment:
       Was this fixed then?

##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CompactSplit.java
##########
@@ -794,13 +795,30 @@ void shutdownLongCompactions(){
   }
 
   public void clearLongCompactionsQueue() {
+    removeFromFilesCompacting(longCompactions);
     longCompactions.getQueue().clear();
   }
 
   public void clearShortCompactionsQueue() {
+    removeFromFilesCompacting(shortCompactions);
     shortCompactions.getQueue().clear();
   }
 
+  private void removeFromFilesCompacting(ThreadPoolExecutor compactor) {
+    for (Runnable runnable : compactor.getQueue()) {
+      if (!(runnable instanceof CompactionRunner)) {
+        continue;
+      }
+      CompactionRunner runner = (CompactionRunner) runnable;
+      // for system compaction, files selection will be delayed until the compaction task
+      // actually runs, so compaction context is null for system compaction
+      if (runner.compaction != null) {
+        Collection<HStoreFile> files = runner.compaction.getRequest().getFiles();

Review comment:
       Is this addressed?

##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
##########
@@ -1953,6 +1954,24 @@ private void addToCompactingFiles(Collection<HStoreFile> filesToAdd) {
     Collections.sort(filesCompacting, storeEngine.getStoreFileManager().getStoreFileComparator());
   }
 
+  /**
+   * Remove the files from compacting files. This usually happens when we clear compaction queues.
+   */
+  void removeFromCompactingFiles(Collection<HStoreFile> filesToRemove) {
+    synchronized (filesCompacting) {
+      filesCompacting.removeAll(filesToRemove);
+      Collections.sort(filesCompacting, storeEngine.getStoreFileManager().getStoreFileComparator());
+    }

Review comment:
       Do you want to resolve this comment? Looks like there is no sorting anymore.




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