You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by el...@apache.org on 2017/11/30 22:53:26 UTC

[3/3] hbase git commit: HBASE-19285 Implements table-level latency histograms

HBASE-19285 Implements table-level latency histograms

For a egionserver's view of a table (the regions
that belong to a table hosted on a regionserver),
this change tracks the latencies of operations that
affect the regions for this table.

Tracking at the per-table level avoids the memory bloat
and performance impact that accompanied the previous
per-region latency metrics while still providing important
details for operators to consume.

Signed-Off-By: Andrew Purtell <ap...@apache.org>


Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/5dc97e29
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/5dc97e29
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/5dc97e29

Branch: refs/heads/branch-1.3
Commit: 5dc97e298f79327198507c8f8f26270fdced40f9
Parents: 561f336
Author: Josh Elser <el...@apache.org>
Authored: Fri Nov 17 13:39:43 2017 -0500
Committer: Josh Elser <el...@apache.org>
Committed: Thu Nov 30 17:45:57 2017 -0500

----------------------------------------------------------------------
 .../regionserver/MetricsTableLatencies.java     | 107 +++++++++++++
 .../hadoop/hbase/test/MetricsAssertHelper.java  |   7 +
 .../regionserver/MetricsTableLatenciesImpl.java | 152 +++++++++++++++++++
 ...oop.hbase.regionserver.MetricsTableLatencies |  17 +++
 .../hbase/test/MetricsAssertHelperImpl.java     |   9 +-
 .../hbase/regionserver/HRegionServer.java       |   3 +-
 .../hbase/regionserver/MetricsRegionServer.java |  63 ++++++--
 .../hbase/regionserver/RSRpcServices.java       |  30 ++--
 .../regionserver/RegionServerTableMetrics.java  |  63 ++++++++
 .../regionserver/TestMetricsRegionServer.java   |  23 +--
 .../regionserver/TestMetricsTableLatencies.java |  68 +++++++++
 11 files changed, 508 insertions(+), 34 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/5dc97e29/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsTableLatencies.java
----------------------------------------------------------------------
diff --git a/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsTableLatencies.java b/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsTableLatencies.java
new file mode 100644
index 0000000..46232bd
--- /dev/null
+++ b/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsTableLatencies.java
@@ -0,0 +1,107 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.regionserver;
+
+/**
+ * Latency metrics for a specific table in a RegionServer.
+ */
+public interface MetricsTableLatencies {
+
+  /**
+   * The name of the metrics
+   */
+  String METRICS_NAME = "TableLatencies";
+
+  /**
+   * The name of the metrics context that metrics will be under.
+   */
+  String METRICS_CONTEXT = "regionserver";
+
+  /**
+   * Description
+   */
+  String METRICS_DESCRIPTION = "Metrics about Tables on a single HBase RegionServer";
+
+  /**
+   * The name of the metrics context that metrics will be under in jmx
+   */
+  String METRICS_JMX_CONTEXT = "RegionServer,sub=" + METRICS_NAME;
+
+  String GET_TIME = "getTime";
+  String SCAN_TIME = "scanTime";
+  String SCAN_SIZE = "scanSize";
+  String PUT_TIME = "putTime";
+  String DELETE_TIME = "deleteTime";
+  String INCREMENT_TIME = "incrementTime";
+  String APPEND_TIME = "appendTime";
+
+  /**
+   * Update the Put time histogram
+   *
+   * @param tableName The table the metric is for
+   * @param t time it took
+   */
+  void updatePut(String tableName, long t);
+
+  /**
+   * Update the Delete time histogram
+   *
+   * @param tableName The table the metric is for
+   * @param t time it took
+   */
+  void updateDelete(String tableName, long t);
+
+  /**
+   * Update the Get time histogram .
+   *
+   * @param tableName The table the metric is for
+   * @param t time it took
+   */
+  void updateGet(String tableName, long t);
+
+  /**
+   * Update the Increment time histogram.
+   *
+   * @param tableName The table the metric is for
+   * @param t time it took
+   */
+  void updateIncrement(String tableName, long t);
+
+  /**
+   * Update the Append time histogram.
+   *
+   * @param tableName The table the metric is for
+   * @param t time it took
+   */
+  void updateAppend(String tableName, long t);
+
+  /**
+   * Update the scan size.
+   *
+   * @param tableName The table the metric is for
+   * @param scanSize size of the scan
+   */
+  void updateScanSize(String tableName, long scanSize);
+
+  /**
+   * Update the scan time.
+   *
+   * @param tableName The table the metric is for
+   * @param t time it took
+   */
+  void updateScanTime(String tableName, long t);
+}

http://git-wip-us.apache.org/repos/asf/hbase/blob/5dc97e29/hbase-hadoop-compat/src/test/java/org/apache/hadoop/hbase/test/MetricsAssertHelper.java
----------------------------------------------------------------------
diff --git a/hbase-hadoop-compat/src/test/java/org/apache/hadoop/hbase/test/MetricsAssertHelper.java b/hbase-hadoop-compat/src/test/java/org/apache/hadoop/hbase/test/MetricsAssertHelper.java
index 70f77f1..52e0d09 100644
--- a/hbase-hadoop-compat/src/test/java/org/apache/hadoop/hbase/test/MetricsAssertHelper.java
+++ b/hbase-hadoop-compat/src/test/java/org/apache/hadoop/hbase/test/MetricsAssertHelper.java
@@ -168,4 +168,11 @@ public interface MetricsAssertHelper {
    * @return long value of the gauge.
    */
   long getGaugeLong(String name, BaseSource source);
+
+  /**
+   * Generates a representation of all metrics exported by the given {@code source}.
+   * @param source The {@link BaseSource} that will provide the metrics.
+   * @return A representation of the metrics as a String.
+   */
+  String toDebugString(BaseSource source);
 }

http://git-wip-us.apache.org/repos/asf/hbase/blob/5dc97e29/hbase-hadoop2-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsTableLatenciesImpl.java
----------------------------------------------------------------------
diff --git a/hbase-hadoop2-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsTableLatenciesImpl.java b/hbase-hadoop2-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsTableLatenciesImpl.java
new file mode 100644
index 0000000..274ea93
--- /dev/null
+++ b/hbase-hadoop2-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsTableLatenciesImpl.java
@@ -0,0 +1,152 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.regionserver;
+
+import java.util.HashMap;
+
+import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.classification.InterfaceAudience;
+import org.apache.hadoop.hbase.metrics.BaseSourceImpl;
+import org.apache.hadoop.metrics2.MetricHistogram;
+import org.apache.hadoop.metrics2.lib.DynamicMetricsRegistry;
+
+import com.google.common.annotations.VisibleForTesting;
+
+/**
+ * Implementation of {@link MetricsTableLatencies} to track latencies for one table in a
+ * RegionServer.
+ */
+@InterfaceAudience.Private
+public class MetricsTableLatenciesImpl extends BaseSourceImpl implements MetricsTableLatencies {
+
+  private final HashMap<TableName,TableHistograms> histogramsByTable = new HashMap<>();
+
+  @VisibleForTesting
+  public static class TableHistograms {
+    final MetricHistogram getTimeHisto;
+    final MetricHistogram incrementTimeHisto;
+    final MetricHistogram appendTimeHisto;
+    final MetricHistogram putTimeHisto;
+    final MetricHistogram deleteTimeHisto;
+    final MetricHistogram scanTimeHisto;
+    final MetricHistogram scanSizeHisto;
+
+    TableHistograms(DynamicMetricsRegistry registry, TableName tn) {
+      getTimeHisto = registry.newTimeHistogram(qualifyMetricsName(tn, GET_TIME));
+      incrementTimeHisto = registry.newTimeHistogram(
+          qualifyMetricsName(tn, INCREMENT_TIME));
+      appendTimeHisto = registry.newTimeHistogram(qualifyMetricsName(tn, APPEND_TIME));
+      putTimeHisto = registry.newTimeHistogram(qualifyMetricsName(tn, PUT_TIME));
+      deleteTimeHisto = registry.newTimeHistogram(qualifyMetricsName(tn, DELETE_TIME));
+      scanTimeHisto = registry.newTimeHistogram(qualifyMetricsName(tn, SCAN_TIME));
+      scanSizeHisto = registry.newSizeHistogram(qualifyMetricsName(tn, SCAN_SIZE));
+    }
+
+    public void updatePut(long time) {
+      putTimeHisto.add(time);
+    }
+
+    public void updateDelete(long t) {
+      deleteTimeHisto.add(t);
+    }
+
+    public void updateGet(long t) {
+      getTimeHisto.add(t);
+    }
+
+    public void updateIncrement(long t) {
+      incrementTimeHisto.add(t);
+    }
+
+    public void updateAppend(long t) {
+      appendTimeHisto.add(t);
+    }
+
+    public void updateScanSize(long scanSize) {
+      scanSizeHisto.add(scanSize);
+    }
+
+    public void updateScanTime(long t) {
+      scanTimeHisto.add(t);
+    }
+  }
+
+  @VisibleForTesting
+  public static String qualifyMetricsName(TableName tableName, String metric) {
+    StringBuilder sb = new StringBuilder();
+    sb.append("Namespace_").append(tableName.getNamespaceAsString());
+    sb.append("_table_").append(tableName.getQualifierAsString());
+    sb.append("_metric_").append(metric);
+    return sb.toString();
+  }
+
+  @VisibleForTesting
+  public TableHistograms getOrCreateTableHistogram(String tableName) {
+    // TODO Java8's ConcurrentHashMap#computeIfAbsent would be stellar instead
+    final TableName tn = TableName.valueOf(tableName);
+    TableHistograms latency = histogramsByTable.get(tn);
+    if (latency == null) {
+      latency = new TableHistograms(getMetricsRegistry(), tn);
+      histogramsByTable.put(tn, latency);
+    }
+    return latency;
+  }
+
+  public MetricsTableLatenciesImpl() {
+    this(METRICS_NAME, METRICS_DESCRIPTION, METRICS_CONTEXT, METRICS_JMX_CONTEXT);
+  }
+
+  public MetricsTableLatenciesImpl(String metricsName, String metricsDescription,
+      String metricsContext, String metricsJmxContext) {
+    super(metricsName, metricsDescription, metricsContext, metricsJmxContext);
+  }
+
+  @Override
+  public void updatePut(String tableName, long t) {
+    getOrCreateTableHistogram(tableName).updatePut(t);
+  }
+
+  @Override
+  public void updateDelete(String tableName, long t) {
+    getOrCreateTableHistogram(tableName).updateDelete(t);
+  }
+
+  @Override
+  public void updateGet(String tableName, long t) {
+    getOrCreateTableHistogram(tableName).updateGet(t);
+  }
+
+  @Override
+  public void updateIncrement(String tableName, long t) {
+    getOrCreateTableHistogram(tableName).updateIncrement(t);
+  }
+
+  @Override
+  public void updateAppend(String tableName, long t) {
+    getOrCreateTableHistogram(tableName).updateAppend(t);
+  }
+
+  @Override
+  public void updateScanSize(String tableName, long scanSize) {
+    getOrCreateTableHistogram(tableName).updateScanSize(scanSize);
+  }
+
+  @Override
+  public void updateScanTime(String tableName, long t) {
+    getOrCreateTableHistogram(tableName).updateScanTime(t);
+  }
+}

http://git-wip-us.apache.org/repos/asf/hbase/blob/5dc97e29/hbase-hadoop2-compat/src/main/resources/META-INF/services/org.apache.hadoop.hbase.regionserver.MetricsTableLatencies
----------------------------------------------------------------------
diff --git a/hbase-hadoop2-compat/src/main/resources/META-INF/services/org.apache.hadoop.hbase.regionserver.MetricsTableLatencies b/hbase-hadoop2-compat/src/main/resources/META-INF/services/org.apache.hadoop.hbase.regionserver.MetricsTableLatencies
new file mode 100644
index 0000000..c0aea23
--- /dev/null
+++ b/hbase-hadoop2-compat/src/main/resources/META-INF/services/org.apache.hadoop.hbase.regionserver.MetricsTableLatencies
@@ -0,0 +1,17 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+org.apache.hadoop.hbase.regionserver.MetricsTableLatenciesImpl
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/hbase/blob/5dc97e29/hbase-hadoop2-compat/src/test/java/org/apache/hadoop/hbase/test/MetricsAssertHelperImpl.java
----------------------------------------------------------------------
diff --git a/hbase-hadoop2-compat/src/test/java/org/apache/hadoop/hbase/test/MetricsAssertHelperImpl.java b/hbase-hadoop2-compat/src/test/java/org/apache/hadoop/hbase/test/MetricsAssertHelperImpl.java
index e19e391..2c2c69e 100644
--- a/hbase-hadoop2-compat/src/test/java/org/apache/hadoop/hbase/test/MetricsAssertHelperImpl.java
+++ b/hbase-hadoop2-compat/src/test/java/org/apache/hadoop/hbase/test/MetricsAssertHelperImpl.java
@@ -19,7 +19,6 @@
 package org.apache.hadoop.hbase.test;
 
 import org.apache.hadoop.hbase.metrics.BaseSource;
-import org.apache.hadoop.hbase.metrics.BaseSourceImpl;
 import org.apache.hadoop.metrics2.AbstractMetric;
 import org.apache.hadoop.metrics2.MetricsCollector;
 import org.apache.hadoop.metrics2.MetricsInfo;
@@ -228,6 +227,14 @@ public class MetricsAssertHelperImpl implements MetricsAssertHelper {
     return gauges.get(cName).longValue();
   }
 
+  @Override
+  public String toDebugString(BaseSource source) {
+    getMetrics(source);
+    StringBuilder sb = new StringBuilder();
+    sb.append("Tags=").append(tags).append(", Counters=").append(counters);
+    return sb.append(", Gauges=").append(gauges).toString();
+  }
+
   private void reset() {
     tags.clear();
     gauges.clear();

http://git-wip-us.apache.org/repos/asf/hbase/blob/5dc97e29/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
index 5959871..d8176da 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
@@ -1422,7 +1422,8 @@ public class HRegionServer extends HasThread implements
       this.cacheConfig = new CacheConfig(conf);
       this.walFactory = setupWALAndReplication();
       // Init in here rather than in constructor after thread name has been set
-      this.metricsRegionServer = new MetricsRegionServer(new MetricsRegionServerWrapperImpl(this));
+      this.metricsRegionServer = new MetricsRegionServer(
+          new MetricsRegionServerWrapperImpl(this), conf);
       this.metricsTable = new MetricsTable(new MetricsTableWrapperAggregateImpl(this));
 
       startServiceThreads();

http://git-wip-us.apache.org/repos/asf/hbase/blob/5dc97e29/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServer.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServer.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServer.java
index 8bca6c5..fa383cc 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServer.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServer.java
@@ -20,7 +20,9 @@ package org.apache.hadoop.hbase.regionserver;
 import com.google.common.annotations.VisibleForTesting;
 import org.apache.hadoop.hbase.classification.InterfaceAudience;
 import org.apache.hadoop.hbase.classification.InterfaceStability;
+import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.hbase.CompatibilitySingletonFactory;
+import org.apache.hadoop.hbase.TableName;
 
 /**
  * <p>
@@ -33,20 +35,38 @@ import org.apache.hadoop.hbase.CompatibilitySingletonFactory;
 @InterfaceStability.Evolving
 @InterfaceAudience.Private
 public class MetricsRegionServer {
+  public static final String RS_ENABLE_TABLE_METRICS_KEY =
+      "hbase.regionserver.enable.table.latencies";
+  public static final boolean RS_ENABLE_TABLE_METRICS_DEFAULT = true;
+
   private MetricsRegionServerSource serverSource;
   private MetricsRegionServerWrapper regionServerWrapper;
+  private RegionServerTableMetrics tableMetrics;
 
-  public MetricsRegionServer(MetricsRegionServerWrapper regionServerWrapper) {
+  public MetricsRegionServer(
+      MetricsRegionServerWrapper regionServerWrapper, Configuration conf) {
     this(regionServerWrapper,
         CompatibilitySingletonFactory.getInstance(MetricsRegionServerSourceFactory.class)
-            .createServer(regionServerWrapper));
-
+            .createServer(regionServerWrapper),
+        createTableMetrics(conf));
   }
 
   MetricsRegionServer(MetricsRegionServerWrapper regionServerWrapper,
-                      MetricsRegionServerSource serverSource) {
+                      MetricsRegionServerSource serverSource,
+                      RegionServerTableMetrics tableMetrics) {
     this.regionServerWrapper = regionServerWrapper;
     this.serverSource = serverSource;
+    this.tableMetrics = tableMetrics;
+  }
+
+  /**
+   * Creates an instance of {@link RegionServerTableMetrics} only if the feature is enabled.
+   */
+  static RegionServerTableMetrics createTableMetrics(Configuration conf) {
+    if (conf.getBoolean(RS_ENABLE_TABLE_METRICS_KEY, RS_ENABLE_TABLE_METRICS_DEFAULT)) {
+      return new RegionServerTableMetrics();
+    }
+    return null;
   }
 
   @VisibleForTesting
@@ -58,35 +78,50 @@ public class MetricsRegionServer {
     return regionServerWrapper;
   }
 
-  public void updatePut(long t) {
+  public void updatePut(TableName tn, long t) {
+    if (tableMetrics != null && tn != null) {
+      tableMetrics.updatePut(tn, t);
+    }
     if (t > 1000) {
       serverSource.incrSlowPut();
     }
     serverSource.updatePut(t);
   }
 
-  public void updateDelete(long t) {
+  public void updateDelete(TableName tn, long t) {
+    if (tableMetrics != null && tn != null) {
+      tableMetrics.updateDelete(tn, t);
+    }
     if (t > 1000) {
       serverSource.incrSlowDelete();
     }
     serverSource.updateDelete(t);
   }
 
-  public void updateGet(long t) {
+  public void updateGet(TableName tn, long t) {
+    if (tableMetrics != null && tn != null) {
+      tableMetrics.updateGet(tn, t);
+    }
     if (t > 1000) {
       serverSource.incrSlowGet();
     }
     serverSource.updateGet(t);
   }
 
-  public void updateIncrement(long t) {
+  public void updateIncrement(TableName tn, long t) {
+    if (tableMetrics != null && tn != null) {
+      tableMetrics.updateIncrement(tn, t);
+    }
     if (t > 1000) {
       serverSource.incrSlowIncrement();
     }
     serverSource.updateIncrement(t);
   }
 
-  public void updateAppend(long t) {
+  public void updateAppend(TableName tn, long t) {
+    if (tableMetrics != null && tn != null) {
+      tableMetrics.updateAppend(tn, t);
+    }
     if (t > 1000) {
       serverSource.incrSlowAppend();
     }
@@ -97,11 +132,17 @@ public class MetricsRegionServer {
     serverSource.updateReplay(t);
   }
 
-  public void updateScanSize(long scanSize){
+  public void updateScanSize(TableName tn, long scanSize){
+    if (tableMetrics != null && tn != null) {
+      tableMetrics.updateScanSize(tn, scanSize);
+    }
     serverSource.updateScanSize(scanSize);
   }
 
-  public void updateScanTime(long t) {
+  public void updateScanTime(TableName tn, long t) {
+    if (tableMetrics != null && tn != null) {
+      tableMetrics.updateScanTime(tn, t);
+    }
     serverSource.updateScanTime(t);
   }
 

http://git-wip-us.apache.org/repos/asf/hbase/blob/5dc97e29/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java
index cda48fc..08c5663 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java
@@ -587,6 +587,7 @@ public class RSRpcServices implements HBaseRPCErrorHandler,
     }
     if (regionServer.metricsRegionServer != null) {
       regionServer.metricsRegionServer.updateAppend(
+          region.getTableDesc().getTableName(),
         EnvironmentEdgeManager.currentTime() - before);
     }
     return r;
@@ -637,7 +638,8 @@ public class RSRpcServices implements HBaseRPCErrorHandler,
     }
     if (regionServer.metricsRegionServer != null) {
       regionServer.metricsRegionServer.updateIncrement(
-        EnvironmentEdgeManager.currentTime() - before);
+          region.getTableDesc().getTableName(),
+          EnvironmentEdgeManager.currentTime() - before);
     }
     return r;
   }
@@ -718,7 +720,8 @@ public class RSRpcServices implements HBaseRPCErrorHandler,
           } finally {
             if (regionServer.metricsRegionServer != null) {
               regionServer.metricsRegionServer.updateGet(
-                EnvironmentEdgeManager.currentTime() - before);
+                  region.getTableDesc().getTableName(),
+                  EnvironmentEdgeManager.currentTime() - before);
             }
           }
         } else if (action.hasServiceCall()) {
@@ -875,10 +878,12 @@ public class RSRpcServices implements HBaseRPCErrorHandler,
     if (regionServer.metricsRegionServer != null) {
       long after = EnvironmentEdgeManager.currentTime();
       if (batchContainsPuts) {
-        regionServer.metricsRegionServer.updatePut(after - before);
+        regionServer.metricsRegionServer.updatePut(
+            region.getTableDesc().getTableName(), after - before);
       }
       if (batchContainsDelete) {
-        regionServer.metricsRegionServer.updateDelete(after - before);
+        regionServer.metricsRegionServer.updateDelete(
+            region.getTableDesc().getTableName(), after - before);
       }
     }
   }
@@ -950,10 +955,12 @@ public class RSRpcServices implements HBaseRPCErrorHandler,
       if (regionServer.metricsRegionServer != null) {
         long after = EnvironmentEdgeManager.currentTime();
           if (batchContainsPuts) {
-          regionServer.metricsRegionServer.updatePut(after - before);
+          regionServer.metricsRegionServer.updatePut(
+              region.getTableDesc().getTableName(), after - before);
         }
         if (batchContainsDelete) {
-          regionServer.metricsRegionServer.updateDelete(after - before);
+          regionServer.metricsRegionServer.updateDelete(
+              region.getTableDesc().getTableName(), after - before);
         }
       }
     }
@@ -2128,11 +2135,12 @@ public class RSRpcServices implements HBaseRPCErrorHandler,
       final GetRequest request) throws ServiceException {
     long before = EnvironmentEdgeManager.currentTime();
     OperationQuota quota = null;
+    Region region = null;
     try {
       checkOpen();
       requestCount.increment();
       rpcGetRequestCount.increment();
-      Region region = getRegion(request.getRegion());
+      region = getRegion(request.getRegion());
 
       GetResponse.Builder builder = GetResponse.newBuilder();
       ClientProtos.Get get = request.getGet();
@@ -2192,7 +2200,7 @@ public class RSRpcServices implements HBaseRPCErrorHandler,
     } finally {
       if (regionServer.metricsRegionServer != null) {
         regionServer.metricsRegionServer.updateGet(
-          EnvironmentEdgeManager.currentTime() - before);
+            region.getTableDesc().getTableName(), EnvironmentEdgeManager.currentTime() - before);
       }
       if (quota != null) {
         quota.close();
@@ -2740,8 +2748,10 @@ public class RSRpcServices implements HBaseRPCErrorHandler,
       long responseCellSize = context != null ? context.getResponseCellSize() : 0;
       region.getMetrics().updateScanTime(end - before);
       if (regionServer.metricsRegionServer != null) {
-        regionServer.metricsRegionServer.updateScanSize(responseCellSize);
-        regionServer.metricsRegionServer.updateScanTime(end - before);
+        regionServer.metricsRegionServer.updateScanSize(
+            region.getTableDesc().getTableName(), responseCellSize);
+        regionServer.metricsRegionServer.updateScanTime(
+            region.getTableDesc().getTableName(), end - before);
       }
     } finally {
       region.closeRegionOperation();

http://git-wip-us.apache.org/repos/asf/hbase/blob/5dc97e29/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerTableMetrics.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerTableMetrics.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerTableMetrics.java
new file mode 100644
index 0000000..a6fb3aa
--- /dev/null
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerTableMetrics.java
@@ -0,0 +1,63 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.regionserver;
+
+import org.apache.hadoop.hbase.CompatibilitySingletonFactory;
+import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.classification.InterfaceAudience;
+
+/**
+ * Captures operation metrics by table. Separates metrics collection for table metrics away from
+ * {@link MetricsRegionServer} for encapsulation and ease of testing.
+ */
+@InterfaceAudience.Private
+public class RegionServerTableMetrics {
+
+  private final MetricsTableLatencies latencies;
+
+  public RegionServerTableMetrics() {
+    latencies = CompatibilitySingletonFactory.getInstance(MetricsTableLatencies.class);
+  }
+
+  public void updatePut(TableName table, long time) {
+    latencies.updatePut(table.getNameAsString(), time);
+  }
+
+  public void updateGet(TableName table, long time) {
+    latencies.updateGet(table.getNameAsString(), time);
+  }
+
+  public void updateIncrement(TableName table, long time) {
+    latencies.updateIncrement(table.getNameAsString(), time);
+  }
+
+  public void updateAppend(TableName table, long time) {
+    latencies.updateAppend(table.getNameAsString(), time);
+  }
+
+  public void updateDelete(TableName table, long time) {
+    latencies.updateDelete(table.getNameAsString(), time);
+  }
+
+  public void updateScanTime(TableName table, long time) {
+    latencies.updateScanTime(table.getNameAsString(), time);
+  }
+
+  public void updateScanSize(TableName table, long size) {
+    latencies.updateScanSize(table.getNameAsString(), size);
+  }
+}

http://git-wip-us.apache.org/repos/asf/hbase/blob/5dc97e29/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMetricsRegionServer.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMetricsRegionServer.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMetricsRegionServer.java
index 52c3c32..cde6d22 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMetricsRegionServer.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMetricsRegionServer.java
@@ -17,6 +17,7 @@
  */
 package org.apache.hadoop.hbase.regionserver;
 
+import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.hbase.CompatibilityFactory;
 import org.apache.hadoop.hbase.testclassification.SmallTests;
 import org.apache.hadoop.hbase.test.MetricsAssertHelper;
@@ -47,7 +48,7 @@ public class TestMetricsRegionServer {
   @Before
   public void setUp() {
     wrapper = new MetricsRegionServerWrapperStub();
-    rsm = new MetricsRegionServer(wrapper);
+    rsm = new MetricsRegionServer(wrapper, new Configuration(false));
     serverSource = rsm.getMetricsSource();
   }
 
@@ -103,24 +104,24 @@ public class TestMetricsRegionServer {
   @Test
   public void testSlowCount() {
     for (int i=0; i < 12; i ++) {
-      rsm.updateAppend(12);
-      rsm.updateAppend(1002);
+      rsm.updateAppend(null, 12);
+      rsm.updateAppend(null, 1002);
     }
     for (int i=0; i < 13; i ++) {
-      rsm.updateDelete(13);
-      rsm.updateDelete(1003);
+      rsm.updateDelete(null, 13);
+      rsm.updateDelete(null, 1003);
     }
     for (int i=0; i < 14; i ++) {
-      rsm.updateGet(14);
-      rsm.updateGet(1004);
+      rsm.updateGet(null, 14);
+      rsm.updateGet(null, 1004);
     }
     for (int i=0; i < 15; i ++) {
-      rsm.updateIncrement(15);
-      rsm.updateIncrement(1005);
+      rsm.updateIncrement(null, 15);
+      rsm.updateIncrement(null, 1005);
     }
     for (int i=0; i < 16; i ++) {
-      rsm.updatePut(16);
-      rsm.updatePut(1006);
+      rsm.updatePut(null, 16);
+      rsm.updatePut(null, 1006);
     }
 
     HELPER.assertCounter("appendNumOps", 24, serverSource);

http://git-wip-us.apache.org/repos/asf/hbase/blob/5dc97e29/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMetricsTableLatencies.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMetricsTableLatencies.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMetricsTableLatencies.java
new file mode 100644
index 0000000..32ffbf8
--- /dev/null
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMetricsTableLatencies.java
@@ -0,0 +1,68 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.regionserver;
+
+import static org.junit.Assert.assertTrue;
+
+import java.io.IOException;
+import org.apache.hadoop.hbase.CompatibilityFactory;
+import org.apache.hadoop.hbase.CompatibilitySingletonFactory;
+import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.test.MetricsAssertHelper;
+import org.apache.hadoop.hbase.testclassification.RegionServerTests;
+import org.apache.hadoop.hbase.testclassification.SmallTests;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+@Category({RegionServerTests.class, SmallTests.class})
+public class TestMetricsTableLatencies {
+
+  public static MetricsAssertHelper HELPER =
+      CompatibilityFactory.getInstance(MetricsAssertHelper.class);
+
+  @Test
+  public void testTableWrapperAggregateMetrics() throws IOException {
+    TableName tn1 = TableName.valueOf("table1");
+    TableName tn2 = TableName.valueOf("table2");
+    MetricsTableLatencies latencies = CompatibilitySingletonFactory.getInstance(
+        MetricsTableLatencies.class);
+    assertTrue("'latencies' is actually " + latencies.getClass(),
+        latencies instanceof MetricsTableLatenciesImpl);
+    MetricsTableLatenciesImpl latenciesImpl = (MetricsTableLatenciesImpl) latencies;
+    RegionServerTableMetrics tableMetrics = new RegionServerTableMetrics();
+
+    // Metrics to each table should be disjoint
+    // N.B. each call to assertGauge removes all previously acquired metrics so we have to
+    //   make the metrics call and then immediately verify it. Trying to do multiple metrics
+    //   updates followed by multiple verifications will fail on the 2nd verification (as the
+    //   first verification cleaned the data structures in MetricsAssertHelperImpl).
+    tableMetrics.updateGet(tn1, 500L);
+    HELPER.assertGauge(MetricsTableLatenciesImpl.qualifyMetricsName(
+        tn1, MetricsTableLatencies.GET_TIME + "_" + "999th_percentile"), 500L, latenciesImpl);
+    tableMetrics.updatePut(tn1, 50L);
+    HELPER.assertGauge(MetricsTableLatenciesImpl.qualifyMetricsName(
+        tn1, MetricsTableLatencies.PUT_TIME + "_" + "99th_percentile"), 50L, latenciesImpl);
+
+    tableMetrics.updateGet(tn2, 300L);
+    HELPER.assertGauge(MetricsTableLatenciesImpl.qualifyMetricsName(
+        tn2, MetricsTableLatencies.GET_TIME + "_" + "999th_percentile"), 300L, latenciesImpl);
+    tableMetrics.updatePut(tn2, 75L);
+    HELPER.assertGauge(MetricsTableLatenciesImpl.qualifyMetricsName(
+        tn2, MetricsTableLatencies.PUT_TIME + "_" + "99th_percentile"), 75L, latenciesImpl);
+  }
+}
\ No newline at end of file