From 93d665494f8ae9242179d0067aef492bcf6572a7 Mon Sep 17 00:00:00 2001 From: chenheng Date: Tue, 8 Mar 2016 12:25:28 +0800 Subject: [PATCH] HBASE-15377 Per-RS Get metric is time based, per-region metric is size-based --- .../regionserver/MetricsRegionServerSource.java | 5 ++- .../hbase/regionserver/MetricsRegionSource.java | 8 +++- .../MetricsRegionServerSourceImpl.java | 8 ++-- .../regionserver/MetricsRegionSourceImpl.java | 28 +++++++++---- .../apache/hadoop/hbase/regionserver/HRegion.java | 9 ++-- .../hadoop/hbase/regionserver/MetricsRegion.java | 8 +++- .../hbase/regionserver/MetricsRegionServer.java | 2 +- .../hadoop/hbase/regionserver/RSRpcServices.java | 4 +- .../regionserver/TestRegionServerMetrics.java | 48 +++++++++++++++++++++- 9 files changed, 95 insertions(+), 25 deletions(-) diff --git a/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServerSource.java b/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServerSource.java index 18a77f4..44a2b80 100644 --- a/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServerSource.java +++ b/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServerSource.java @@ -64,7 +64,7 @@ public interface MetricsRegionServerSource extends BaseSource { * * @param t time it took */ - void updateGet(long t); + void updateGetTime(long t); /** * Update the Increment time histogram. @@ -258,7 +258,8 @@ public interface MetricsRegionServerSource extends BaseSource { String UPDATES_BLOCKED_DESC = "Number of MS updates have been blocked so that the memstore can be flushed."; String DELETE_KEY = "delete"; - String GET_KEY = "get"; + String GET_SIZE_KEY = "getSize"; + String GET_TIME_KEY = "getTime"; String INCREMENT_KEY = "increment"; String MUTATE_KEY = "mutate"; String APPEND_KEY = "append"; diff --git a/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionSource.java b/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionSource.java index 11fc068..90d5e8b 100644 --- a/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionSource.java +++ b/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionSource.java @@ -59,7 +59,13 @@ public interface MetricsRegionSource extends Comparable { * Update count and sizes of gets. * @param getSize size in bytes of the resulting key values for a get */ - void updateGet(long getSize); + void updateGetSize(long getSize); + + /** + * Update time of gets + * @param mills time for this get operation. + */ + void updateGetTime(long mills); /** * Update the count and sizes of resultScanner.next() diff --git a/hbase-hadoop2-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServerSourceImpl.java b/hbase-hadoop2-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServerSourceImpl.java index f869397..64aa44c 100644 --- a/hbase-hadoop2-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServerSourceImpl.java +++ b/hbase-hadoop2-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServerSourceImpl.java @@ -39,7 +39,7 @@ public class MetricsRegionServerSourceImpl final MetricsRegionServerWrapper rsWrap; private final MetricHistogram putHisto; private final MetricHistogram deleteHisto; - private final MetricHistogram getHisto; + private final MetricHistogram getTimeHisto; private final MetricHistogram incrementHisto; private final MetricHistogram appendHisto; private final MetricHistogram replayHisto; @@ -75,7 +75,7 @@ public class MetricsRegionServerSourceImpl deleteHisto = getMetricsRegistry().newTimeHistogram(DELETE_KEY); slowDelete = getMetricsRegistry().newCounter(SLOW_DELETE_KEY, SLOW_DELETE_DESC, 0L); - getHisto = getMetricsRegistry().newTimeHistogram(GET_KEY); + getTimeHisto = getMetricsRegistry().newTimeHistogram(GET_TIME_KEY); slowGet = getMetricsRegistry().newCounter(SLOW_GET_KEY, SLOW_GET_DESC, 0L); incrementHisto = getMetricsRegistry().newTimeHistogram(INCREMENT_KEY); @@ -106,8 +106,8 @@ public class MetricsRegionServerSourceImpl } @Override - public void updateGet(long t) { - getHisto.add(t); + public void updateGetTime(long t) { + getTimeHisto.add(t); } @Override diff --git a/hbase-hadoop2-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionSourceImpl.java b/hbase-hadoop2-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionSourceImpl.java index 39d665b..7c628dd 100644 --- a/hbase-hadoop2-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionSourceImpl.java +++ b/hbase-hadoop2-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionSourceImpl.java @@ -48,7 +48,8 @@ public class MetricsRegionSourceImpl implements MetricsRegionSource { private final String regionNamePrefix; private final String regionPutKey; private final String regionDeleteKey; - private final String regionGetKey; + private final String regionGetSizeKey; + private final String regionGetTimeKey; private final String regionIncrementKey; private final String regionAppendKey; private final String regionScanSizeKey; @@ -58,7 +59,8 @@ public class MetricsRegionSourceImpl implements MetricsRegionSource { private final MutableFastCounter regionDelete; private final MutableFastCounter regionIncrement; private final MutableFastCounter regionAppend; - private final MetricHistogram regionGet; + private final MetricHistogram regionGetSize; + private final MetricHistogram regionGetTime; private final MetricHistogram regionScanSize; private final MetricHistogram regionScanTime; private final int hashCode; @@ -93,8 +95,11 @@ public class MetricsRegionSourceImpl implements MetricsRegionSource { regionAppendKey = regionNamePrefix + MetricsRegionServerSource.APPEND_KEY + suffix; regionAppend = registry.getCounter(regionAppendKey, 0L); - regionGetKey = regionNamePrefix + MetricsRegionServerSource.GET_KEY; - regionGet = registry.newTimeHistogram(regionGetKey); + regionGetSizeKey = regionNamePrefix + MetricsRegionServerSource.GET_SIZE_KEY; + regionGetSize = registry.newSizeHistogram(regionGetSizeKey); + + regionGetTimeKey = regionNamePrefix + MetricsRegionServerSource.GET_TIME_KEY; + regionGetTime = registry.newTimeHistogram(regionGetTimeKey); regionScanSizeKey = regionNamePrefix + MetricsRegionServerSource.SCAN_SIZE_KEY; regionScanSize = registry.newSizeHistogram(regionScanSizeKey); @@ -129,10 +134,12 @@ public class MetricsRegionSourceImpl implements MetricsRegionSource { registry.removeMetric(regionDeleteKey); registry.removeMetric(regionIncrementKey); registry.removeMetric(regionAppendKey); - registry.removeMetric(regionGetKey); + registry.removeMetric(regionGetSizeKey); + registry.removeMetric(regionGetTimeKey); registry.removeMetric(regionScanSizeKey); registry.removeMetric(regionScanTimeKey); - registry.removeHistogramMetrics(regionGetKey); + registry.removeHistogramMetrics(regionGetSizeKey); + registry.removeHistogramMetrics(regionGetTimeKey); registry.removeHistogramMetrics(regionScanSizeKey); registry.removeHistogramMetrics(regionScanTimeKey); @@ -151,8 +158,13 @@ public class MetricsRegionSourceImpl implements MetricsRegionSource { } @Override - public void updateGet(long getSize) { - regionGet.add(getSize); + public void updateGetSize(long getSize) { + regionGetSize.add(getSize); + } + + @Override + public void updateGetTime(long mills) { + regionGetTime.add(mills); } @Override 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 c090b54..4bff838 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 @@ -6699,7 +6699,7 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver, Regi return results; } } - + long before = EnvironmentEdgeManager.currentTime(); Scan scan = new Scan(get); RegionScanner scanner = null; @@ -6716,12 +6716,12 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver, Regi coprocessorHost.postGet(get, results); } - metricsUpdateForGet(results); + metricsUpdateForGet(results, before); return results; } - void metricsUpdateForGet(List results) { + void metricsUpdateForGet(List results, long before) { if (this.metricsRegion != null) { long totalSize = 0L; for (Cell cell : results) { @@ -6729,7 +6729,8 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver, Regi // to know the serialization of how the codec works with it?? totalSize += CellUtil.estimatedSerializedSizeOf(cell); } - this.metricsRegion.updateGet(totalSize); + this.metricsRegion.updateGetSize(totalSize); + this.metricsRegion.updateGetTime(EnvironmentEdgeManager.currentTime() - before); } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegion.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegion.java index 56839ff..b4ef9c3 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegion.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegion.java @@ -49,8 +49,12 @@ public class MetricsRegion { source.updateDelete(); } - public void updateGet(final long getSize) { - source.updateGet(getSize); + public void updateGetSize(final long getSize) { + source.updateGetSize(getSize); + } + + public void updateGetTime(final long t) { + source.updateGetTime(t); } public void updateScanSize(final long scanSize) { 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 7ff9bed..dcbac8e 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 @@ -76,7 +76,7 @@ public class MetricsRegionServer { if (t > 1000) { serverSource.incrSlowGet(); } - serverSource.updateGet(t); + serverSource.updateGetTime(t); } public void updateIncrement(long t) { 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 035b2d1..f4a2574 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 @@ -2197,7 +2197,7 @@ public class RSRpcServices implements HBaseRPCErrorHandler, .create(results, get.isCheckExistenceOnly() ? !results.isEmpty() : null, stale); } } - + long before = EnvironmentEdgeManager.currentTime(); Scan scan = new Scan(get); RegionScanner scanner = null; @@ -2227,7 +2227,7 @@ public class RSRpcServices implements HBaseRPCErrorHandler, if (region.getCoprocessorHost() != null) { region.getCoprocessorHost().postGet(get, results); } - region.metricsUpdateForGet(results); + region.metricsUpdateForGet(results, before); return Result.create(results, get.isCheckExistenceOnly() ? !results.isEmpty() : null, stale); } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionServerMetrics.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionServerMetrics.java index 4754058..1e4f5e1 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionServerMetrics.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionServerMetrics.java @@ -158,7 +158,7 @@ public class TestRegionServerMetrics { "_table_"+tableNameString + "_region_" + i.getEncodedName()+ "_metric"; - metricsHelper.assertCounter(prefix + "_getNumOps", 10, agg); + metricsHelper.assertCounter(prefix + "_getSizeNumOps", 10, agg); metricsHelper.assertCounter(prefix + "_mutateCount", 31, agg); } } @@ -188,6 +188,52 @@ public class TestRegionServerMetrics { } @Test + public void testGet() throws Exception { + String tableNameString = "testGet"; + TableName tName = TableName.valueOf(tableNameString); + byte[] cfName = Bytes.toBytes("d"); + byte[] row = Bytes.toBytes("rk"); + byte[] qualifier = Bytes.toBytes("qual"); + byte[] initValue = Bytes.toBytes("Value"); + + TEST_UTIL.createTable(tName, cfName); + + Connection connection = TEST_UTIL.getConnection(); + connection.getTable(tName).close(); //wait for the table to come up. + + // Do a first put to be sure that the connection is established, meta is there and so on. + Table table = connection.getTable(tName); + Put p = new Put(row); + p.addColumn(cfName, qualifier, initValue); + table.put(p); + + Get g = new Get(row); + for (int i=0; i< 10; i++) { + table.get(g); + } + + metricsRegionServer.getRegionServerWrapper().forceRecompute(); + + try (RegionLocator locator = connection.getRegionLocator(tName)) { + for ( HRegionLocation location: locator.getAllRegionLocations()) { + HRegionInfo i = location.getRegionInfo(); + MetricsRegionAggregateSource agg = rs.getRegion(i.getRegionName()) + .getMetrics() + .getSource() + .getAggregateSource(); + String prefix = "namespace_"+NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR+ + "_table_"+tableNameString + + "_region_" + i.getEncodedName()+ + "_metric"; + metricsHelper.assertCounter(prefix + "_getSizeNumOps", 10, agg); + metricsHelper.assertCounter(prefix + "_getTimeNumOps", 10, agg); + } + metricsHelper.assertCounter("GetTime_num_ops", 10, serverSource); + } + table.close(); + } + + @Test public void testMutationsWithoutWal() throws Exception { TableName tableName = TableName.valueOf("testMutationsWithoutWal"); byte[] cf = Bytes.toBytes("d"); -- 1.9.3 (Apple Git-50)