[HBASE-12454] Setting didPerformCompaction early in HRegion#compact

• Type: Bug
• Status: Closed
• Priority: Trivial
• Resolution: Fixed
• Affects Version/s: 0.98.8
• Fix Version/s: 1.0.0, 0.98.9
• Hadoop Flags: Reviewed
• Assignee: Andrew Kyle Purtell
• Reporter: Andrew Kyle Purtell
• Created: 10/Nov/14 20:36
• Updated: 06/Apr/18 17:55
• Resolved: 10/Dec/14 01:29

Description

It appears we are setting 'didPerformCompaction' to "true" before attempting the compaction in HRegion#compact. If Store#compact throws an exception or is interrupted, we won't call Store#cancelRequestedCompaction in the last finally block of the method as it looks like we should.

--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
@@ -1540,58 +1540,58 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver {
         return false;
       }
 
       status = TaskMonitor.get().createStatus("Compacting " + store + " in " + this);
       if (this.closed.get()) {
         String msg = "Skipping compaction on " + this + " because closed";
         LOG.debug(msg);
         status.abort(msg);
         return false;
       }
       boolean wasStateSet = false;
       try {
         synchronized (writestate) {
           if (writestate.writesEnabled) {
             wasStateSet = true;
             ++writestate.compacting;
           } else {
             String msg = "NOT compacting region " + this + ". Writes disabled.";
             LOG.info(msg);
             status.abort(msg);
             return false;
           }
         }
         LOG.info("Starting compaction on " + store + " in region " + this
             + (compaction.getRequest().isOffPeak()?" as an off-peak compaction":""));
         doRegionCompactionPrep();
         try {
           status.setStatus("Compacting store " + store);
-          didPerformCompaction = true;
           store.compact(compaction);
+          didPerformCompaction = true;
         } catch (InterruptedIOException iioe) {
           String msg = "compaction interrupted";
           LOG.info(msg, iioe);
           status.abort(msg);
           return false;
         }
       } finally {
         if (wasStateSet) {
           synchronized (writestate) {
             --writestate.compacting;
             if (writestate.compacting <= 0) {
               writestate.notifyAll();
             }
           }
         }
       }
       status.markComplete("Compaction complete");
       return true;
     } finally {
       try {
         if (!didPerformCompaction) store.cancelRequestedCompaction(compaction);   <-----
         if (status != null) status.cleanup();
       } finally {
         lock.readLock().unlock();
       }
     }
   }