diff -u b/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/master/MetricsAssignmentManagerSource.java b/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/master/MetricsAssignmentManagerSource.java --- b/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/master/MetricsAssignmentManagerSource.java +++ b/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/master/MetricsAssignmentManagerSource.java @@ -19,6 +19,7 @@ package org.apache.hadoop.hbase.master; import org.apache.hadoop.hbase.metrics.BaseSource; +import org.apache.hadoop.hbase.metrics.OperationMetrics; public interface MetricsAssignmentManagerSource extends BaseSource { @@ -55,24 +56,10 @@ String RIT_DURATION_DESC = "Total durations in milliseconds for all Regions in Transition (Histogram)."; - // Region assign metrics - String ASSIGN_SUBMITTED_COUNT_NAME = "assignSubmittedCount"; - String ASSIGN_TIME_NAME = "assignTime"; - String ASSIGN_FAILED_COUNT_NAME = "assignFailedCount"; - - // Region unassign metrics - String UNASSIGN_SUBMITTED_COUNT_NAME = "unassignSubmittedCount"; - String UNASSIGN_TIME_NAME = "unassignTime"; - String UNASSIGN_FAILED_COUNT_NAME = "unassignFailedCount"; - - // split/ merge metrics - String SPLIT_SUBMITTED_COUNT_NAME = "splitSubmittedCount"; - String SPLIT_TIME_NAME = "splitTime"; - String SPLIT_FAILED_COUNT_NAME = "splitFailedCount"; - - String MERGE_SUBMITTED_COUNT_NAME = "mergeSubmittedCount"; - String MERGE_TIME_NAME = "mergeTime"; - String MERGE_FAILED_COUNT_NAME = "mergeFailedCount"; + String ASSIGN_METRIC_PREFIX = "assign"; + String UNASSIGN_METRIC_PREFIX = "unassign"; + String SPLIT_METRIC_PREFIX = "split"; + String MERGE_METRIC_PREFIX = "merge"; String OPERATION_COUNT_NAME = "operationCount"; diff -u b/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/master/MetricsMasterSource.java b/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/master/MetricsMasterSource.java --- b/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/master/MetricsMasterSource.java +++ b/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/master/MetricsMasterSource.java @@ -19,6 +19,7 @@ package org.apache.hadoop.hbase.master; import org.apache.hadoop.hbase.metrics.BaseSource; +import org.apache.hadoop.hbase.metrics.OperationMetrics; /** * Interface that classes that expose metrics about the master will implement. @@ -75,9 +76,7 @@ String SPLIT_PLAN_COUNT_DESC = "Number of Region Split Plans executed"; String MERGE_PLAN_COUNT_DESC = "Number of Region Merge Plans executed"; - String SERVER_CRASH_SUBMITTED_COUNT_NAME = "serverCrashSubmittedCount"; - String SERVER_CRASH_TIME_NAME = "serverCrashTime"; - String SERVER_CRASH_FAILED_COUNT_NAME = "serverCrashFailedCount"; + String SERVER_CRASH_METRIC_PREFIX = "serverCrash"; /** * Increment the number of requests the cluster has seen. reverted: --- b/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/metrics/BaseSource.java +++ a/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/metrics/BaseSource.java @@ -23,27 +23,6 @@ * In hbase-hadoop{1|2}-compat there is an implementation of this interface. */ public interface BaseSource { - /** - * Container interface for commonly collected metrics for most operations. Instead of adding - * per metrics methods in the extended interfaces, set of metrics can be directly accessed - * through this container. - */ - interface OperationMetrics { - /** - * @return Total number of submitted operations - */ - Counter getSubmittedCounter(); - - /** - * @return Histogram of operation runtimes - */ - Histogram getTimeHisto(); - - /** - * @return Total number of failed operations - */ - Counter getFailedCounter(); - } String HBASE_METRICS_SYSTEM_NAME = "HBase"; diff -u b/hbase-hadoop2-compat/src/main/java/org/apache/hadoop/hbase/master/MetricsAssignmentManagerSourceImpl.java b/hbase-hadoop2-compat/src/main/java/org/apache/hadoop/hbase/master/MetricsAssignmentManagerSourceImpl.java --- b/hbase-hadoop2-compat/src/main/java/org/apache/hadoop/hbase/master/MetricsAssignmentManagerSourceImpl.java +++ b/hbase-hadoop2-compat/src/main/java/org/apache/hadoop/hbase/master/MetricsAssignmentManagerSourceImpl.java @@ -20,6 +20,7 @@ import org.apache.hadoop.hbase.classification.InterfaceAudience; import org.apache.hadoop.hbase.metrics.BaseSourceImpl; +import org.apache.hadoop.hbase.metrics.OperationMetrics; import org.apache.hadoop.metrics2.MetricHistogram; import org.apache.hadoop.metrics2.lib.MutableFastCounter; import org.apache.hadoop.metrics2.lib.MutableGaugeLong; @@ -36,10 +37,10 @@ private MutableFastCounter operationCounter; - private OperationMetricsImpl assignMetrics; - private OperationMetricsImpl unassignMetrics; - private OperationMetricsImpl splitMetrics; - private OperationMetricsImpl mergeMetrics; + private OperationMetrics assignMetrics; + private OperationMetrics unassignMetrics; + private OperationMetrics splitMetrics; + private OperationMetrics mergeMetrics; public MetricsAssignmentManagerSourceImpl() { this(METRICS_NAME, METRICS_DESCRIPTION, METRICS_CONTEXT, METRICS_JMX_CONTEXT); @@ -63,30 +64,11 @@ * NOTE: Please refer to HBASE-9774 and HBASE-14282. Based on these two issues, HBase is * moving away from using Hadoop's metric2 to having independent HBase specific Metrics. Use * {@link BaseSourceImpl#registry} to register the new metrics. - * - * TODO: As of now, Metrics description cannot be added/ registered with - * {@link BaseSourceImpl#registry}. As metric names are unambiguous but concise, descriptions of - * metrics need to be made available to the user somewhere. */ - assignMetrics = new OperationMetricsImpl(); - assignMetrics.setSubmittedCounter(registry.counter(ASSIGN_SUBMITTED_COUNT_NAME)); - assignMetrics.setTimeHisto(registry.histogram(ASSIGN_TIME_NAME)); - assignMetrics.setFailedCounter(registry.counter(ASSIGN_FAILED_COUNT_NAME)); - - unassignMetrics = new OperationMetricsImpl(); - unassignMetrics.setSubmittedCounter(registry.counter(UNASSIGN_SUBMITTED_COUNT_NAME)); - unassignMetrics.setTimeHisto(registry.histogram(UNASSIGN_TIME_NAME)); - unassignMetrics.setFailedCounter(registry.counter(UNASSIGN_FAILED_COUNT_NAME)); - - splitMetrics = new OperationMetricsImpl(); - splitMetrics.setSubmittedCounter(registry.counter(SPLIT_SUBMITTED_COUNT_NAME)); - splitMetrics.setTimeHisto(registry.histogram(SPLIT_TIME_NAME)); - splitMetrics.setFailedCounter(registry.counter(SPLIT_FAILED_COUNT_NAME)); - - mergeMetrics = new OperationMetricsImpl(); - mergeMetrics.setSubmittedCounter(registry.counter(MERGE_SUBMITTED_COUNT_NAME)); - mergeMetrics.setTimeHisto(registry.histogram(MERGE_TIME_NAME)); - mergeMetrics.setFailedCounter(registry.counter(MERGE_FAILED_COUNT_NAME)); + assignMetrics = new OperationMetrics(registry, ASSIGN_METRIC_PREFIX); + unassignMetrics = new OperationMetrics(registry, UNASSIGN_METRIC_PREFIX); + splitMetrics = new OperationMetrics(registry, SPLIT_METRIC_PREFIX); + mergeMetrics = new OperationMetrics(registry, MERGE_METRIC_PREFIX); } @Override diff -u b/hbase-hadoop2-compat/src/main/java/org/apache/hadoop/hbase/master/MetricsMasterSourceImpl.java b/hbase-hadoop2-compat/src/main/java/org/apache/hadoop/hbase/master/MetricsMasterSourceImpl.java --- b/hbase-hadoop2-compat/src/main/java/org/apache/hadoop/hbase/master/MetricsMasterSourceImpl.java +++ b/hbase-hadoop2-compat/src/main/java/org/apache/hadoop/hbase/master/MetricsMasterSourceImpl.java @@ -21,6 +21,7 @@ import org.apache.hadoop.hbase.classification.InterfaceAudience; import org.apache.hadoop.hbase.metrics.BaseSourceImpl; import org.apache.hadoop.hbase.metrics.Interns; +import org.apache.hadoop.hbase.metrics.OperationMetrics; import org.apache.hadoop.metrics2.MetricsCollector; import org.apache.hadoop.metrics2.MetricsRecordBuilder; import org.apache.hadoop.metrics2.lib.MutableFastCounter; @@ -37,7 +38,7 @@ private final MetricsMasterWrapper masterWrapper; private MutableFastCounter clusterRequestsCounter; - private OperationMetricsImpl serverCrashMetrics; + private OperationMetrics serverCrashMetrics; public MetricsMasterSourceImpl(MetricsMasterWrapper masterWrapper) { this(METRICS_NAME, @@ -66,15 +67,8 @@ * NOTE: Please refer to HBASE-9774 and HBASE-14282. Based on these two issues, HBase is * moving away from using Hadoop's metric2 to having independent HBase specific Metrics. Use * {@link BaseSourceImpl#registry} to register the new metrics. - * - * TODO: As of now, Metrics description cannot be added/ registered with - * {@link BaseSourceImpl#registry}. As metric names are unambiguous but concise, description of - * the metrics need to be made available someplace. */ - serverCrashMetrics = new OperationMetricsImpl(); - serverCrashMetrics.setSubmittedCounter(registry.counter(SERVER_CRASH_SUBMITTED_COUNT_NAME)); - serverCrashMetrics.setTimeHisto(registry.histogram(SERVER_CRASH_TIME_NAME)); - serverCrashMetrics.setFailedCounter(registry.counter(SERVER_CRASH_FAILED_COUNT_NAME)); + serverCrashMetrics = new OperationMetrics(registry, SERVER_CRASH_METRIC_PREFIX); } @Override reverted: --- b/hbase-hadoop2-compat/src/main/java/org/apache/hadoop/hbase/metrics/BaseSourceImpl.java +++ a/hbase-hadoop2-compat/src/main/java/org/apache/hadoop/hbase/metrics/BaseSourceImpl.java @@ -41,39 +41,6 @@ @InterfaceAudience.Private public class BaseSourceImpl implements BaseSource, MetricsSource { - public class OperationMetricsImpl implements OperationMetrics { - private Counter submittedCounter; - private Histogram timeHisto; - private Counter failedCounter; - - @Override - public Counter getSubmittedCounter() { - return submittedCounter; - } - - public void setSubmittedCounter(final Counter submittedCounter) { - this.submittedCounter = submittedCounter; - } - - @Override - public Histogram getTimeHisto() { - return timeHisto; - } - - public void setTimeHisto(final Histogram timeHisto) { - this.timeHisto = timeHisto; - } - - @Override - public Counter getFailedCounter() { - return failedCounter; - } - - public void setFailedCounter(final Counter failedCounter) { - this.failedCounter = failedCounter; - } - } - private static enum DefaultMetricsSystemInitializer { INSTANCE; private boolean inited = false; diff -u b/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/Procedure.java b/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/Procedure.java --- b/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/Procedure.java +++ b/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/Procedure.java @@ -270,10 +270,9 @@ } Counter submittedCounter = metrics.getSubmittedCounter(); - if (submittedCounter == null) { - return; + if (submittedCounter != null) { + submittedCounter.increment(); } - submittedCounter.increment(); } /** @@ -300,16 +299,14 @@ if (success) { Histogram timeHisto = metrics.getTimeHisto(); - if (timeHisto == null) { - return; + if (timeHisto != null) { + timeHisto.update(runtime); } - timeHisto.update(runtime); } else { Counter failedCounter = metrics.getFailedCounter(); - if (failedCounter == null) { - return; + if (failedCounter != null) { + failedCounter.increment(); } - failedCounter.increment(); } } diff -u b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterServices.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterServices.java --- b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterServices.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterServices.java @@ -125,7 +125,7 @@ ProcedureExecutor getMasterProcedureExecutor(); /** - * @return Master's instnace of {@link MetricsMaster} + * @return Master's instance of {@link MetricsMaster} */ MetricsMaster getMasterMetrics(); diff -u b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MetricsMaster.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MetricsMaster.java --- b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MetricsMaster.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MetricsMaster.java @@ -25,6 +25,7 @@ import org.apache.hadoop.hbase.CompatibilitySingletonFactory; import org.apache.hadoop.hbase.metrics.Counter; import org.apache.hadoop.hbase.metrics.Histogram; +import org.apache.hadoop.hbase.metrics.OperationMetrics; import org.apache.hadoop.hbase.procedure2.ProcedureMetrics; /** @@ -75,20 +76,16 @@ } /** - * This is utility function that converts - * {@link org.apache.hadoop.hbase.metrics.BaseSource.OperationMetrics} to - * {@link ProcedureMetrics}. + * This is utility function that converts {@link OperationMetrics} to {@link ProcedureMetrics}. * * NOTE: Procedure framework in hbase-procedure module accesses metrics common to most procedures * through {@link ProcedureMetrics} interface. Metrics source classes in hbase-hadoop-compat - * module provides similar interface - * {@link org.apache.hadoop.hbase.metrics.BaseSource.OperationMetrics} that contains metrics - * common to most operations. As both hbase-procedure and hbase-hadoop-compat are lower level - * modules used by hbase-server (this) module and there is no dependency between them, this - * method does the required conversion. + * module provides similar interface {@link OperationMetrics} that contains metrics common to + * most operations. As both hbase-procedure and hbase-hadoop-compat are lower level modules used + * by hbase-server (this) module and there is no dependency between them, this method does the + * required conversion. */ - public static ProcedureMetrics convertToProcedureMetrics( - final MetricsAssignmentManagerSource.OperationMetrics metrics) { + public static ProcedureMetrics convertToProcedureMetrics(final OperationMetrics metrics) { return new ProcedureMetrics() { @Override public Counter getSubmittedCounter() { only in patch2: unchanged: --- /dev/null +++ b/hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/metrics/OperationMetrics.java @@ -0,0 +1,61 @@ +/** + * 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.metrics; + +import com.google.common.base.Preconditions; + +/** + * Container class for commonly collected metrics for most operations. Instantiate this class to + * collect submitted count, failed count and time histogram for an operation. + */ +public class OperationMetrics { + private static final String SUBMITTED_COUNT = "SubmittedCount"; + private static final String TIME = "Time"; + private static final String FAILED_COUNT = "FailedCount"; + + final private Counter submittedCounter; + final private Histogram timeHisto; + final private Counter failedCounter; + + public OperationMetrics(final MetricRegistry registry, final String metricNamePrefix) { + Preconditions.checkNotNull(registry); + Preconditions.checkNotNull(metricNamePrefix); + + /** + * TODO: As of now, Metrics description cannot be added/ registered with + * {@link MetricRegistry}. As metric names are unambiguous but concise, descriptions of + * metrics need to be made available someplace for users. + */ + submittedCounter = registry.counter(metricNamePrefix + SUBMITTED_COUNT); + timeHisto = registry.histogram(metricNamePrefix + TIME); + failedCounter = registry.counter(metricNamePrefix + FAILED_COUNT); + } + + public Counter getSubmittedCounter() { + return submittedCounter; + } + + public Histogram getTimeHisto() { + return timeHisto; + } + + public Counter getFailedCounter() { + return failedCounter; + } +}