From 078817462d8dd156dcfd776d0bfc4db6fbee5f44 Mon Sep 17 00:00:00 2001
From: Zhihong Yu <tedyu@apache.org>
Date: Sun, 9 Dec 2012 22:06:39 +0000
Subject: [PATCH] HBASE-7309 Metrics refresh-task is not canceled when regions
 are closed, leaking HRegion objects (Adrian Muraru)

git-svn-id: https://svn.apache.org/repos/asf/hbase/trunk@1419172 13f79535-47bb-0310-9956-ffa450edef68
---
 .../hadoop/hbase/regionserver/HRegion.java      | 14 +++++++++++---
 .../regionserver/MetricsRegionWrapperImpl.java  | 17 ++++++++++++++---
 2 files changed, 25 insertions(+), 6 deletions(-)

diff --git 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
index 7907073206d..93d194358e4 100644
--- 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
@@ -143,6 +143,7 @@ import com.google.common.collect.ImmutableList;
 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 { // , Writable{
   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 { // , Writable{
     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 { // , Writable{
       // 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 { // , Writable{
       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 { // , Writable{
   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);
 
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionWrapperImpl.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionWrapperImpl.java
index ea013565eed..9c9c5774415 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionWrapperImpl.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionWrapperImpl.java
@@ -23,11 +23,14 @@ import org.apache.hadoop.hbase.HRegionInfo;
 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 implements MetricsRegionWrapper {
   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 implements MetricsRegionWrapper {
     }
   }
 
-}
\ No newline at end of file
+  @Override
+  public void close() throws IOException {
+    regionMetricsUpdateTask.cancel(true);
+  }
+
+}