Mailing-List: contact commits-help@hbase.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@hbase.apache.org
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: svn commit: r1419172 - in
 /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver:
 HRegion.java MetricsRegionWrapperImpl.java
Date: Sun, 09 Dec 2012 22:06:40 -0000
To: commits@hbase.apache.org
From: tedyu@apache.org
Message-Id: <20121209220641.0F8F623888CD@eris.apache.org>

Author: tedyu
Date: Sun Dec  9 22:06:39 2012
New Revision: 1419172

URL: http://svn.apache.org/viewvc?rev=1419172&view=rev
Log:
HBASE-7309 Metrics refresh-task is not canceled when regions are closed, leaking HRegion objects (Adrian Muraru)


Modified:
    hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
    hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionWrapperImpl.java

Modified: hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
URL: http://svn.apache.org/viewvc/hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java?rev=1419172&r1=1419171&r2=1419172&view=diff
==============================================================================
--- hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java (original)
+++ hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java Sun Dec  9 22:06:39 2012
@@ -143,6 +143,7 @@ import com.google.common.collect.Immutab
 import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
 import com.google.common.collect.MutableClassToInstanceMap;
+import com.google.common.io.Closeables;
 
 import static org.apache.hadoop.hbase.protobuf.generated.ClientProtos.CoprocessorServiceCall;
 
@@ -384,6 +385,7 @@ public class HRegion implements HeapSize
   private RegionSplitPolicy splitPolicy;
 
   private final MetricsRegion metricsRegion;
+  private final MetricsRegionWrapperImpl metricsRegionWrapper;
 
   /**
    * Should only be used for testing purposes
@@ -407,7 +409,8 @@ public class HRegion implements HeapSize
     this.coprocessorHost = null;
     this.scannerReadPoints = new ConcurrentHashMap<RegionScanner, Long>();
 
-    this.metricsRegion = new MetricsRegion(new MetricsRegionWrapperImpl(this));
+    this.metricsRegionWrapper = new MetricsRegionWrapperImpl(this);
+    this.metricsRegion = new MetricsRegion(this.metricsRegionWrapper);
     this.maxBusyWaitDuration = 2 * HConstants.DEFAULT_HBASE_RPC_TIMEOUT;
     this.busyWaitDuration = DEFAULT_BUSY_WAIT_DURATION;
     this.maxBusyWaitMultiplier = 2;
@@ -506,8 +509,10 @@ public class HRegion implements HeapSize
       // don't initialize coprocessors if not running within a regionserver
       // TODO: revisit if coprocessors should load in other cases
       this.coprocessorHost = new RegionCoprocessorHost(this, rsServices, conf);
-      this.metricsRegion = new MetricsRegion(new MetricsRegionWrapperImpl(this));
+      this.metricsRegionWrapper = new MetricsRegionWrapperImpl(this);
+      this.metricsRegion = new MetricsRegion(this.metricsRegionWrapper);
     } else {
+      this.metricsRegionWrapper = null;
       this.metricsRegion = null;
     }
     if (LOG.isDebugEnabled()) {
@@ -1062,6 +1067,9 @@ public class HRegion implements HeapSize
       if ( this.metricsRegion != null) {
         this.metricsRegion.close();
       }
+      if ( this.metricsRegionWrapper != null) {
+        Closeables.closeQuietly(this.metricsRegionWrapper);
+      }
       status.markComplete("Closed");
       LOG.info("Closed " + this);
       return result;
@@ -4876,7 +4884,7 @@ public class HRegion implements HeapSize
   public static final long FIXED_OVERHEAD = ClassSize.align(
       ClassSize.OBJECT +
       ClassSize.ARRAY +
-      40 * ClassSize.REFERENCE + 2 * Bytes.SIZEOF_INT +
+      41 * ClassSize.REFERENCE + 2 * Bytes.SIZEOF_INT +
       (9 * Bytes.SIZEOF_LONG) +
       Bytes.SIZEOF_BOOLEAN);
 

Modified: hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionWrapperImpl.java
URL: http://svn.apache.org/viewvc/hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionWrapperImpl.java?rev=1419172&r1=1419171&r2=1419172&view=diff
==============================================================================
--- hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionWrapperImpl.java (original)
+++ hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionWrapperImpl.java Sun Dec  9 22:06:39 2012
@@ -23,11 +23,14 @@ import org.apache.hadoop.hbase.HRegionIn
 import org.apache.hadoop.hbase.HTableDescriptor;
 import org.apache.hadoop.metrics2.MetricsExecutor;
 
+import java.io.Closeable;
+import java.io.IOException;
 import java.util.Map;
 import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.ScheduledFuture;
 import java.util.concurrent.TimeUnit;
 
-public class MetricsRegionWrapperImpl implements MetricsRegionWrapper {
+public class MetricsRegionWrapperImpl implements MetricsRegionWrapper, Closeable {
 
   public static final int PERIOD = 45;
 
@@ -38,11 +41,14 @@ public class MetricsRegionWrapperImpl im
   private long memstoreSize;
   private long storeFileSize;
 
+  private ScheduledFuture<?> regionMetricsUpdateTask;
+
   public MetricsRegionWrapperImpl(HRegion region) {
     this.region = region;
     this.executor = CompatibilitySingletonFactory.getInstance(MetricsExecutor.class).getExecutor();
     this.runnable = new HRegionMetricsWrapperRunnable();
-    this.executor.scheduleWithFixedDelay(this.runnable, PERIOD, PERIOD, TimeUnit.SECONDS);
+    this.regionMetricsUpdateTask = this.executor.scheduleWithFixedDelay(this.runnable, PERIOD,
+      PERIOD, TimeUnit.SECONDS);
   }
 
   @Override
@@ -119,4 +125,9 @@ public class MetricsRegionWrapperImpl im
     }
   }
 
-}
\ No newline at end of file
+  @Override
+  public void close() throws IOException {
+    regionMetricsUpdateTask.cancel(true);
+  }
+
+}