Index: src/test/java/org/apache/hadoop/hbase/regionserver/TestCompactSelection.java =================================================================== --- src/test/java/org/apache/hadoop/hbase/regionserver/TestCompactSelection.java (revision 1237756) +++ src/test/java/org/apache/hadoop/hbase/regionserver/TestCompactSelection.java (working copy) @@ -25,6 +25,7 @@ import java.util.Calendar; import java.util.GregorianCalendar; import java.util.List; +import java.util.concurrent.Executors; import junit.framework.TestCase; @@ -273,6 +274,84 @@ compactEquals(sfCreate(999,50,12,12, 1), 12, 12, 1); } + // Verify that the inter-thread compaction count synchronization works + // and doesn't lead to deadlocks. + // Since Integers are interned, all Ints with a value of one + // are the "same" object - hence locking one will lock all of them. + // This function verifies that locking an Integer with a value of 1 + // doesn't affect the execution of the compaction selection. See HBASE-5290. + public void testLocking() throws IOException, InterruptedException { + final Object commObj = new Object(); + Runnable holdLock = new Runnable() { + public void run() { + Integer one = 1; + synchronized(one){ + try { + synchronized (commObj){ commObj.notifyAll(); } + Thread.sleep(10000); + } catch (InterruptedException e) {} + } + } + }; + java.util.concurrent.ExecutorService exec = Executors.newSingleThreadExecutor(); + + // test getCompactSelectionRatio + synchronized(commObj){ + exec.submit(holdLock); + commObj.wait(); + } + long start = System.currentTimeMillis(); + + double foo = store.compactSelection(new ArrayList()) + .getCompactSelectionRatio(); + long elapsed = System.currentTimeMillis() - start; + assertTrue("Locking handled incorrectly in getCompactSelectionRatio", elapsed < 5000); + exec.shutdownNow(); + + // The next two tests require it to be an "off peak" compaction + Calendar calendar = new GregorianCalendar(); + int hourOfDay = calendar.get(Calendar.HOUR_OF_DAY); + LOG.debug("Hour of day = " + hourOfDay); + int hourPlusOne = ((hourOfDay+1+24)%24); + int hourMinusOne = ((hourOfDay-1+24)%24); + + // set peak hour to current time + this.conf.setLong("hbase.offpeak.start.hour", hourMinusOne); + this.conf.setLong("hbase.offpeak.end.hour", hourPlusOne); + + // test finishRequest + CompactSelection cs = new CompactSelection(conf, new ArrayList()); + // Force this one to be off-peak + CompactSelection.numOutstandingOffPeakCompactions = 0; + cs.getCompactSelectionRatio(); + exec = Executors.newSingleThreadExecutor(); + synchronized(commObj){ + exec.submit(holdLock); + commObj.wait(); + } + start = System.currentTimeMillis(); + cs.finishRequest(); + elapsed = System.currentTimeMillis() - start; + assertTrue("Locking handled incorrectly in finishRequest", elapsed < 5000); + exec.shutdownNow(); + + // test emptyFileList + exec = Executors.newSingleThreadExecutor(); + cs = new CompactSelection(conf, new ArrayList()); + // Force this one to be off-peak + CompactSelection.numOutstandingOffPeakCompactions = 0; + cs.getCompactSelectionRatio(); + synchronized(commObj){ + exec.submit(holdLock); + commObj.wait(); + } + start = System.currentTimeMillis(); + cs.emptyFileList(); + elapsed = System.currentTimeMillis() - start; + assertTrue("Locking handled incorrectly in emptyFileList", elapsed < 5000); + exec.shutdownNow(); + } + @org.junit.Rule public org.apache.hadoop.hbase.ResourceCheckerJUnitRule cu = new org.apache.hadoop.hbase.ResourceCheckerJUnitRule(); Index: src/main/java/org/apache/hadoop/hbase/regionserver/compactions/CompactSelection.java =================================================================== --- src/main/java/org/apache/hadoop/hbase/regionserver/compactions/CompactSelection.java (revision 1237756) +++ src/main/java/org/apache/hadoop/hbase/regionserver/compactions/CompactSelection.java (working copy) @@ -36,9 +36,18 @@ // the actual list - this is needed to handle methods like "sublist" // correctly List filesToCompact = new ArrayList(); - // number of off peak compactions either in the compaction queue or - // happening now + + /** + * Number of off peak compactions either in the compaction queue or + * happening now. Please lock compactionCountLock before modifying. + */ public static Integer numOutstandingOffPeakCompactions = 0; + + /** + * Lock object for numOutstandingOffPeakCompactions + */ + private final Object compactionCountLock = new Object(); + // HBase conf object Configuration conf; // was this compaction promoted to an off-peak @@ -83,7 +92,7 @@ */ public double getCompactSelectionRatio() { double r = this.compactRatio; - synchronized(numOutstandingOffPeakCompactions) { + synchronized(compactionCountLock) { if (isOffPeakHour() && numOutstandingOffPeakCompactions == 0) { r = this.compactRatioOffPeak; numOutstandingOffPeakCompactions++; @@ -104,7 +113,7 @@ */ public void finishRequest() { if (isOffPeakCompaction) { - synchronized(numOutstandingOffPeakCompactions) { + synchronized(compactionCountLock) { numOutstandingOffPeakCompactions--; isOffPeakCompaction = false; } @@ -124,7 +133,7 @@ public void emptyFileList() { filesToCompact.clear(); if (isOffPeakCompaction) { - synchronized(numOutstandingOffPeakCompactions) { + synchronized(compactionCountLock) { // reset the off peak count numOutstandingOffPeakCompactions--; isOffPeakCompaction = false;