You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by st...@apache.org on 2012/04/26 20:00:42 UTC

svn commit: r1330998 - in /hbase/branches/0.94/src: main/java/org/apache/hadoop/hbase/regionserver/ main/java/org/apache/hadoop/hbase/regionserver/metrics/ test/java/org/apache/hadoop/hbase/regionserver/

Author: stack
Date: Thu Apr 26 18:00:42 2012
New Revision: 1330998

URL: http://svn.apache.org/viewvc?rev=1330998&view=rev
Log:
HBASE-5862 After Region Close remove the Operation Metrics

Modified:
    hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
    hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
    hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/metrics/OperationMetrics.java
    hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/metrics/RegionMetricsStorage.java
    hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/metrics/RegionServerDynamicMetrics.java
    hbase/branches/0.94/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionServerMetrics.java

Modified: hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java?rev=1330998&r1=1330997&r2=1330998&view=diff
==============================================================================
--- hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java (original)
+++ hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java Thu Apr 26 18:00:42 2012
@@ -926,6 +926,7 @@ public class HRegion implements HeapSize
         status.setStatus("Running coprocessor post-close hooks");
         this.coprocessorHost.postClose(abort);
       }
+      this.opMetrics.closeMetrics();
       status.markComplete("Closed");
       LOG.info("Closed " + this);
       return result;

Modified: hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java?rev=1330998&r1=1330997&r2=1330998&view=diff
==============================================================================
--- hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java (original)
+++ hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java Thu Apr 26 18:00:42 2012
@@ -3019,6 +3019,14 @@ public class HRegionServer implements HR
   public boolean removeFromOnlineRegions(final String encodedName) {
     HRegion toReturn = null;
     toReturn = this.onlineRegions.remove(encodedName);
+    
+    //Clear all of the dynamic metrics as they are now probably useless.
+    //This is a clear because dynamic metrics could include metrics per cf and
+    //per hfile.  Figuring out which cfs, hfiles, and regions are still relevant to
+    //this region server would be an onerous task.  Instead just clear everything
+    //and on the next tick of the metrics everything that is still relevant will be
+    //re-added.
+    this.dynamicMetrics.clear();
     return toReturn != null;
   }
 

Modified: hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/metrics/OperationMetrics.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/metrics/OperationMetrics.java?rev=1330998&r1=1330997&r2=1330998&view=diff
==============================================================================
--- hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/metrics/OperationMetrics.java (original)
+++ hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/metrics/OperationMetrics.java Thu Apr 26 18:00:42 2012
@@ -29,7 +29,6 @@ import org.apache.hadoop.hbase.client.Ge
 import org.apache.hadoop.hbase.client.HTable;
 import org.apache.hadoop.hbase.client.Increment;
 import org.apache.hadoop.hbase.client.Put;
-import org.apache.hadoop.hbase.regionserver.HRegion;
 import org.apache.hadoop.hbase.util.Bytes;
 
 /**
@@ -64,12 +63,12 @@ public class OperationMetrics {
    * @param conf The Configuration of the HRegion reporting operations coming in.
    * @param regionInfo The region info
    */
-  public OperationMetrics(Configuration conf, HRegionInfo regionInfo) {
+  public OperationMetrics(Configuration conf, HRegionInfo regionInfo) { 
     // Configure SchemaMetrics before trying to create a RegionOperationMetrics instance as
     // RegionOperationMetrics relies on SchemaMetrics to do naming.
     if (conf != null) {
       SchemaMetrics.configureGlobally(conf);
-
+      
       this.conf = conf;
       if (regionInfo != null) {
         this.tableName = regionInfo.getTableNameAsString();
@@ -172,6 +171,13 @@ public class OperationMetrics {
   public void updateDeleteMetrics(Set<byte[]> columnFamilies, long value) {
     doUpdateTimeVarying(columnFamilies, DELETE_KEY, value);
   }
+  
+  /**
+   * This deletes all old metrics this instance has ever created or updated.
+   */
+  public void closeMetrics() {
+    RegionMetricsStorage.clear();
+  }
 
   /**
    * Method to send updates for cf and region metrics. This is the normal method
@@ -199,7 +205,8 @@ public class OperationMetrics {
   private void doSafeIncTimeVarying(String prefix, String key, long value) {
     if (conf.getBoolean(CONF_KEY, true)) {
       if (prefix != null && !prefix.isEmpty() && key != null && !key.isEmpty()) {
-        RegionMetricsStorage.incrTimeVaryingMetric(prefix + key, value);
+        String m = prefix + key;
+        RegionMetricsStorage.incrTimeVaryingMetric(m, value);
       }
     }
   }

Modified: hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/metrics/RegionMetricsStorage.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/metrics/RegionMetricsStorage.java?rev=1330998&r1=1330997&r2=1330998&view=diff
==============================================================================
--- hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/metrics/RegionMetricsStorage.java (original)
+++ hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/metrics/RegionMetricsStorage.java Thu Apr 26 18:00:42 2012
@@ -127,4 +127,12 @@ public class RegionMetricsStorage {
     return m.get();
   }
 
+  /**
+   * Clear all copies of the metrics this stores.
+   */
+  public static void clear() {
+    timeVaryingMetrics.clear();
+    numericMetrics.clear();
+    numericPersistentMetrics.clear();
+  }
 }

Modified: hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/metrics/RegionServerDynamicMetrics.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/metrics/RegionServerDynamicMetrics.java?rev=1330998&r1=1330997&r2=1330998&view=diff
==============================================================================
--- hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/metrics/RegionServerDynamicMetrics.java (original)
+++ hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/regionserver/metrics/RegionServerDynamicMetrics.java Thu Apr 26 18:00:42 2012
@@ -20,7 +20,9 @@
 
 package org.apache.hadoop.hbase.regionserver.metrics;
 
+import java.lang.reflect.Field;
 import java.lang.reflect.Method;
+import java.util.Map;
 import java.util.Map.Entry;
 import java.util.concurrent.atomic.AtomicLong;
 import java.util.concurrent.atomic.AtomicInteger;
@@ -50,12 +52,18 @@ import org.apache.hadoop.metrics.util.Me
  *
  */
 public class RegionServerDynamicMetrics implements Updater {
+  private static final String UNABLE_TO_CLEAR = "Unable to clear RegionServerDynamicMetrics";
+  
   private MetricsRecord metricsRecord;
   private MetricsContext context;
   private final RegionServerDynamicStatistics rsDynamicStatistics;
   private Method updateMbeanInfoIfMetricsListChanged = null;
   private static final Log LOG =
     LogFactory.getLog(RegionServerDynamicStatistics.class);
+  
+  private boolean reflectionInitialized = false;
+  private Field recordMetricMapField;
+  private Field registryMetricMapField;
 
   /**
    * The metrics variables are public:
@@ -124,6 +132,60 @@ public class RegionServerDynamicMetrics 
       m.inc(numOps, amt);
     }
   }
+  
+  /**
+   * Clear all metrics this exposes. 
+   * Uses reflection to clear them from hadoop metrics side as well.
+   */
+  @SuppressWarnings("rawtypes")
+  public void clear() {
+    
+    // If this is the first clear use reflection to get the two maps that hold copies of our 
+    // metrics on the hadoop metrics side. We have to use reflection because there is not 
+    // remove metrics on the hadoop side. If we can't get them then clearing old metrics 
+    // is not possible and bailing out early is our best option.
+    if (!this.reflectionInitialized) {
+      this.reflectionInitialized = true;
+      try {
+        this.recordMetricMapField = this.metricsRecord.getClass().getDeclaredField("metricTable");
+        this.recordMetricMapField.setAccessible(true);
+      } catch (SecurityException e) {
+        LOG.debug(UNABLE_TO_CLEAR);
+        return;
+      } catch (NoSuchFieldException e) {
+        LOG.debug(UNABLE_TO_CLEAR);
+        return;
+      }
+
+      try {
+        this.registryMetricMapField = this.registry.getClass().getDeclaredField("metricsList");
+        this.registryMetricMapField.setAccessible(true);
+      } catch (SecurityException e) {
+        LOG.debug(UNABLE_TO_CLEAR);
+        return;
+      } catch (NoSuchFieldException e) {
+        LOG.debug(UNABLE_TO_CLEAR);
+        return;
+      } 
+    }
+
+    
+    //If we found both fields then try and clear the maps.
+    if (this.recordMetricMapField != null && this.registryMetricMapField != null) {
+      try {
+        Map recordMap = (Map) this.recordMetricMapField.get(this.metricsRecord);
+        recordMap.clear();
+        Map registryMap = (Map) this.registryMetricMapField.get(this.registry);
+        registryMap.clear();
+      } catch (IllegalArgumentException e) {
+        LOG.debug(UNABLE_TO_CLEAR);
+      } catch (IllegalAccessException e) {
+        LOG.debug(UNABLE_TO_CLEAR);
+      }
+    } else {
+      LOG.debug(UNABLE_TO_CLEAR);
+    }
+  }
 
   /**
    * Push the metrics to the monitoring subsystem on doUpdate() call.

Modified: hbase/branches/0.94/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionServerMetrics.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.94/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionServerMetrics.java?rev=1330998&r1=1330997&r2=1330998&view=diff
==============================================================================
--- hbase/branches/0.94/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionServerMetrics.java (original)
+++ hbase/branches/0.94/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionServerMetrics.java Thu Apr 26 18:00:42 2012
@@ -47,6 +47,7 @@ import org.junit.Before;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
 
+
 /**
  * Test metrics incremented on region server operations.
  */
@@ -200,6 +201,32 @@ public class TestRegionServerMetrics {
     }
   
   @Test
+  public void testRemoveRegionMetrics() throws IOException, InterruptedException {
+    String cf = "REMOVECF";
+    HTable hTable = TEST_UTIL.createTable(TABLE_NAME.getBytes(), cf.getBytes());
+    HRegionInfo[] regionInfos =
+        hTable.getRegionLocations().keySet()
+            .toArray(new HRegionInfo[hTable.getRegionLocations().keySet().size()]);
+
+    String regionName = regionInfos[0].getEncodedName();
+
+    // Do some operations so there are metrics.
+    Put pOne = new Put("TEST".getBytes());
+    pOne.add(cf.getBytes(), "test".getBytes(), "test".getBytes());
+    hTable.put(pOne);
+
+    Get g = new Get("TEST".getBytes());
+    g.addFamily(cf.getBytes());
+    hTable.get(g);
+    assertTimeVaryingMetricCount(1, TABLE_NAME, cf, regionName, "get_");
+    HBaseAdmin admin = TEST_UTIL.getHBaseAdmin();
+    admin.disableTable(TABLE_NAME.getBytes());
+    admin.deleteTable(TABLE_NAME.getBytes());
+
+    assertTimeVaryingMetricCount(0, TABLE_NAME, cf, regionName, "get_");
+  }
+  
+  @Test
   public void testMultipleRegions() throws IOException, InterruptedException {
 
     TEST_UTIL.createRandomTable(