You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by GitBox <gi...@apache.org> on 2022/08/25 14:58:50 UTC

[GitHub] [hbase] Apache9 commented on a diff in pull request #4725: HBASE-27152 Under compaction mark may leak

Apache9 commented on code in PR #4725:
URL: https://github.com/apache/hbase/pull/4725#discussion_r955079022


##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CompactSplit.java:
##########
@@ -382,15 +381,20 @@ protected void requestCompactionInternal(HRegion region, HStore store, String wh
       // pool; we will do selection there, and move to large pool if necessary.
       pool = shortCompactions;
     }
-    pool.execute(
-      new CompactionRunner(store, region, compaction, tracker, completeTracker, pool, user));
+
+    // A simple implementation for under compaction marks.
+    // Since this method is always called in the synchronized methods, we do not need to use the
+    // boolean result to make sure that exactly the one that added here will be removed
+    // in the next steps.
+    underCompactionStores.add(getStoreNameForUnderCompaction(store));

Review Comment:
   So this is the actual fix here?  We should add it first before submit the compaction runner, so in the finally block it can finally remove the marker? In the old implementation, it is posible that after the compaction finishes and removes the marker from the underCompactionStores, we then added the marker to underCompactionStores?



##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CompactSplit.java:
##########
@@ -145,15 +144,15 @@ private void createCompactionExecutors() {
     final String n = Thread.currentThread().getName();
 
     StealJobQueue<Runnable> stealJobQueue = new StealJobQueue<Runnable>(COMPARATOR);
+    // Since the StealJobQueue is unbounded, we need not to set the RejectedExecutionHandler for

Review Comment:
   This could be done as a separated issue? And what is the disadvantage to set the rejection handler, seems no harm?



-- 
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: issues-unsubscribe@hbase.apache.org

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