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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]