From fc79ba338ad8200a43f1aa23a6b70e9bd9c2a913 Mon Sep 17 00:00:00 2001 From: Andrew Purtell Date: Thu, 24 Sep 2015 11:16:36 -0700 Subject: [PATCH] HBASE-14205 RegionCoprocessorHost System.nanoTime() performance bottleneck --- .../regionserver/MetricsRegionWrapper.java | 9 --- .../regionserver/MetricsRegionSourceImpl.java | 32 --------- .../TestMetricsRegionSourceImpl.java | 5 -- .../tmpl/regionserver/RegionListTmpl.jamon | 67 ------------------- .../MetricsRegionWrapperImpl.java | 11 --- .../regionserver/RegionCoprocessorHost.java | 37 ---------- .../MetricsRegionWrapperStub.java | 10 --- 7 files changed, 171 deletions(-) diff --git a/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionWrapper.java b/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionWrapper.java index 46bc37acd6f..0997f7c8d54 100644 --- a/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionWrapper.java +++ b/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionWrapper.java @@ -18,10 +18,6 @@ package org.apache.hadoop.hbase.regionserver; -import java.util.Map; - -import org.apache.commons.math.stat.descriptive.DescriptiveStatistics; - /** * Interface of class that will wrap an HRegion and export numbers so they can be * used in MetricsRegionSource @@ -86,11 +82,6 @@ public interface MetricsRegionWrapper { int getRegionHashCode(); - /** - * Get the time spent by coprocessors in this region. - */ - Map getCoprocessorExecutionStatistics(); - /** * Get the replica id of this region. */ 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 bd123b9a0b1..b31e71d990f 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 @@ -18,12 +18,10 @@ package org.apache.hadoop.hbase.regionserver; -import java.util.Map; import java.util.concurrent.atomic.AtomicBoolean; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; -import org.apache.commons.math.stat.descriptive.DescriptiveStatistics; import org.apache.hadoop.hbase.classification.InterfaceAudience; import org.apache.hadoop.metrics2.MetricsRecordBuilder; import org.apache.hadoop.metrics2.lib.DynamicMetricsRegistry; @@ -245,36 +243,6 @@ public class MetricsRegionSourceImpl implements MetricsRegionSource { mrb.addCounter(Interns.info(regionNamePrefix + MetricsRegionSource.REPLICA_ID, MetricsRegionSource.REPLICA_ID_DESC), this.regionWrapper.getReplicaId()); - - for (Map.Entry entry : this.regionWrapper - .getCoprocessorExecutionStatistics() - .entrySet()) { - DescriptiveStatistics ds = entry.getValue(); - mrb.addGauge(Interns.info(regionNamePrefix + " " + entry.getKey() + " " - + MetricsRegionSource.COPROCESSOR_EXECUTION_STATISTICS, - MetricsRegionSource.COPROCESSOR_EXECUTION_STATISTICS_DESC + "Min: "), - ds.getMin() / 1000); - mrb.addGauge(Interns.info(regionNamePrefix + " " + entry.getKey() + " " - + MetricsRegionSource.COPROCESSOR_EXECUTION_STATISTICS, - MetricsRegionSource.COPROCESSOR_EXECUTION_STATISTICS_DESC + "Mean: "), - ds.getMean() / 1000); - mrb.addGauge(Interns.info(regionNamePrefix + " " + entry.getKey() + " " - + MetricsRegionSource.COPROCESSOR_EXECUTION_STATISTICS, - MetricsRegionSource.COPROCESSOR_EXECUTION_STATISTICS_DESC + "Max: "), - ds.getMax() / 1000); - mrb.addGauge(Interns.info(regionNamePrefix + " " + entry.getKey() + " " - + MetricsRegionSource.COPROCESSOR_EXECUTION_STATISTICS, - MetricsRegionSource.COPROCESSOR_EXECUTION_STATISTICS_DESC + "90th percentile: "), ds - .getPercentile(90d) / 1000); - mrb.addGauge(Interns.info(regionNamePrefix + " " + entry.getKey() + " " - + MetricsRegionSource.COPROCESSOR_EXECUTION_STATISTICS, - MetricsRegionSource.COPROCESSOR_EXECUTION_STATISTICS_DESC + "95th percentile: "), ds - .getPercentile(95d) / 1000); - mrb.addGauge(Interns.info(regionNamePrefix + " " + entry.getKey() + " " - + MetricsRegionSource.COPROCESSOR_EXECUTION_STATISTICS, - MetricsRegionSource.COPROCESSOR_EXECUTION_STATISTICS_DESC + "99th percentile: "), ds - .getPercentile(99d) / 1000); - } } } diff --git a/hbase-hadoop2-compat/src/test/java/org/apache/hadoop/hbase/regionserver/TestMetricsRegionSourceImpl.java b/hbase-hadoop2-compat/src/test/java/org/apache/hadoop/hbase/regionserver/TestMetricsRegionSourceImpl.java index 3242b67cfda..f9b902abfba 100644 --- a/hbase-hadoop2-compat/src/test/java/org/apache/hadoop/hbase/regionserver/TestMetricsRegionSourceImpl.java +++ b/hbase-hadoop2-compat/src/test/java/org/apache/hadoop/hbase/regionserver/TestMetricsRegionSourceImpl.java @@ -129,11 +129,6 @@ public class TestMetricsRegionSourceImpl { return regionName.hashCode(); } - @Override - public Map getCoprocessorExecutionStatistics() { - return null; - } - /** * Always return 0 for testing */ diff --git a/hbase-server/src/main/jamon/org/apache/hadoop/hbase/tmpl/regionserver/RegionListTmpl.jamon b/hbase-server/src/main/jamon/org/apache/hadoop/hbase/tmpl/regionserver/RegionListTmpl.jamon index bc05c0627d3..bf143b97c79 100644 --- a/hbase-server/src/main/jamon/org/apache/hadoop/hbase/tmpl/regionserver/RegionListTmpl.jamon +++ b/hbase-server/src/main/jamon/org/apache/hadoop/hbase/tmpl/regionserver/RegionListTmpl.jamon @@ -22,7 +22,6 @@ <%import> java.util.*; - org.apache.commons.math.stat.descriptive.DescriptiveStatistics; org.apache.hadoop.hbase.regionserver.HRegionServer; org.apache.hadoop.hbase.util.Bytes; org.apache.hadoop.hbase.HRegionInfo; @@ -48,7 +47,6 @@
  • Storefile Metrics
  • Memstore Metrics
  • Compaction Metrics
    • -
  • Coprocessor Metrics
    • @@ -66,9 +64,6 @@
    <& compactStats; onlineRegions = onlineRegions; &>
    -
    - <& coprocessorStats; onlineRegions = onlineRegions; &> -

    Region names are made of the containing table's name, a comma, @@ -233,65 +228,3 @@ - -<%def coprocessorStats> -<%args> - List onlineRegions; - - - - - - - - - <%for HRegionInfo r: onlineRegions %> - <%java> - Region region = regionServer.getFromOnlineRegions(r.getEncodedName()); - MetricsRegionWrapper mWrap = region == null ? null: region.getMetrics().getRegionWrapper(); - - - <%if mWrap != null %> - - <%for Map.Entry entry: mWrap.getCoprocessorExecutionStatistics().entrySet() %> - - <%java> - String coprocessorName = entry.getKey(); - DescriptiveStatistics ds = entry.getValue(); - - - - - - - - -
    Region NameCoprocessorExecution Time Statistics
    <% r.getRegionNameAsString() %><% coprocessorName %> - - - - - - - - - - - - - - - - - - - - - - - - - -
    Min Time <% String.format("%.3f%n", ds.getMin()/1000/1000) %>ms
    Avg Time <% String.format("%.3f%n", ds.getMean()/1000/1000) %>ms
    Max Time <% String.format("%.3f%n", ds.getMax()/1000/1000) %>ms
    90th percentile <% String.format("%.3f%n", ds.getPercentile(90d)/1000/1000) %>ms
    95th percentile <% String.format("%.3f%n", ds.getPercentile(95d)/1000/1000) %>ms
    99th percentile <% String.format("%.3f%n", ds.getPercentile(99d)/1000/1000) %>ms
    -
    - 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 3f1da850cd5..08865e6aeb0 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 @@ -20,13 +20,11 @@ package org.apache.hadoop.hbase.regionserver; import java.io.Closeable; import java.io.IOException; -import java.util.HashMap; import java.util.Map; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; -import org.apache.commons.math.stat.descriptive.DescriptiveStatistics; import org.apache.hadoop.hbase.classification.InterfaceAudience; import org.apache.hadoop.hbase.CompatibilitySingletonFactory; import org.apache.hadoop.hbase.HRegionInfo; @@ -45,7 +43,6 @@ public class MetricsRegionWrapperImpl implements MetricsRegionWrapper, Closeable private long numStoreFiles; private long memstoreSize; private long storeFileSize; - private Map coprocessorTimes; private ScheduledFuture regionMetricsUpdateTask; @@ -55,7 +52,6 @@ public class MetricsRegionWrapperImpl implements MetricsRegionWrapper, Closeable this.runnable = new HRegionMetricsWrapperRunnable(); this.regionMetricsUpdateTask = this.executor.scheduleWithFixedDelay(this.runnable, PERIOD, PERIOD, TimeUnit.SECONDS); - this.coprocessorTimes = new HashMap(); } @Override @@ -159,8 +155,6 @@ public class MetricsRegionWrapperImpl implements MetricsRegionWrapper, Closeable numStoreFiles = tempNumStoreFiles; memstoreSize = tempMemstoreSize; storeFileSize = tempStoreFileSize; - coprocessorTimes = region.getCoprocessorHost().getCoprocessorExecutionStatistics(); - } } @@ -169,11 +163,6 @@ public class MetricsRegionWrapperImpl implements MetricsRegionWrapper, Closeable regionMetricsUpdateTask.cancel(true); } - @Override - public Map getCoprocessorExecutionStatistics() { - return coprocessorTimes; - } - /** * Get the replica id of this region. */ diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionCoprocessorHost.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionCoprocessorHost.java index d528b558233..fa7429244c2 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionCoprocessorHost.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionCoprocessorHost.java @@ -21,8 +21,6 @@ package org.apache.hadoop.hbase.regionserver; import java.io.IOException; import java.util.ArrayList; -import java.util.Collection; -import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.NavigableSet; @@ -36,7 +34,6 @@ import org.apache.commons.collections.map.ReferenceMap; import org.apache.commons.lang.ClassUtils; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; -import org.apache.commons.math.stat.descriptive.DescriptiveStatistics; import org.apache.hadoop.hbase.classification.InterfaceAudience; import org.apache.hadoop.hbase.classification.InterfaceStability; import org.apache.hadoop.conf.Configuration; @@ -77,7 +74,6 @@ import org.apache.hadoop.hbase.regionserver.compactions.CompactionRequest; import org.apache.hadoop.hbase.regionserver.wal.HLogKey; import org.apache.hadoop.hbase.wal.WALKey; import org.apache.hadoop.hbase.regionserver.wal.WALEdit; -import org.apache.hadoop.hbase.util.BoundedConcurrentLinkedQueue; import org.apache.hadoop.hbase.util.Bytes; import org.apache.hadoop.hbase.util.CoprocessorClassLoader; import org.apache.hadoop.hbase.util.Pair; @@ -111,9 +107,6 @@ public class RegionCoprocessorHost private Region region; private RegionServerServices rsServices; ConcurrentMap sharedData; - private static final int LATENCY_BUFFER_SIZE = 100; - private final BoundedConcurrentLinkedQueue coprocessorTimeNanos = - new BoundedConcurrentLinkedQueue(LATENCY_BUFFER_SIZE); private final boolean useLegacyPre; private final boolean useLegacyPost; @@ -160,16 +153,6 @@ public class RegionCoprocessorHost return sharedData; } - public void offerExecutionLatency(long latencyNanos) { - coprocessorTimeNanos.offer(latencyNanos); - } - - public Collection getExecutionLatenciesNanos() { - final List latencies = Lists.newArrayListWithCapacity(coprocessorTimeNanos.size()); - coprocessorTimeNanos.drainTo(latencies); - return latencies; - } - @Override public HRegionInfo getRegionInfo() { return region.getRegionInfo(); @@ -1634,24 +1617,6 @@ public class RegionCoprocessorHost }); } - public Map getCoprocessorExecutionStatistics() { - Map results = new HashMap(); - for (RegionEnvironment env : coprocessors) { - DescriptiveStatistics ds = new DescriptiveStatistics(); - if (env.getInstance() instanceof RegionObserver) { - for (Long time : env.getExecutionLatenciesNanos()) { - ds.addValue(time); - } - // Ensures that web ui circumvents the display of NaN values when there are zero samples. - if (ds.getN() == 0) { - ds.addValue(0); - } - results.put(env.getInstance().getClass().getSimpleName(), ds); - } - } - return results; - } - private static abstract class CoprocessorOperation extends ObserverContext { public abstract void call(Coprocessor observer, @@ -1739,7 +1704,6 @@ public class RegionCoprocessorHost for (RegionEnvironment env: coprocessors) { Coprocessor observer = env.getInstance(); if (ctx.hasCall(observer)) { - long startTime = System.nanoTime(); ctx.prepare(env); Thread currentThread = Thread.currentThread(); ClassLoader cl = currentThread.getContextClassLoader(); @@ -1751,7 +1715,6 @@ public class RegionCoprocessorHost } finally { currentThread.setContextClassLoader(cl); } - env.offerExecutionLatency(System.nanoTime() - startTime); bypass |= ctx.shouldBypass(); if (earlyExit && ctx.shouldComplete()) { break; diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/MetricsRegionWrapperStub.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/MetricsRegionWrapperStub.java index 2b1a9b7b4cf..c43ccc32b7f 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/MetricsRegionWrapperStub.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/MetricsRegionWrapperStub.java @@ -18,11 +18,6 @@ package org.apache.hadoop.hbase.regionserver; -import java.util.HashMap; -import java.util.Map; - -import org.apache.commons.math.stat.descriptive.DescriptiveStatistics; - public class MetricsRegionWrapperStub implements MetricsRegionWrapper { int replicaid = 0; @@ -105,11 +100,6 @@ public class MetricsRegionWrapperStub implements MetricsRegionWrapper { return 42; } - @Override - public Map getCoprocessorExecutionStatistics() { - return new HashMap(); - } - /** * Get the replica id of this region. */