bbeaudreault commented on code in PR #5072:
URL: https://github.com/apache/hbase/pull/5072#discussion_r1126646507
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServer.java:
##########
@@ -122,89 +102,75 @@ public MetricsRegionServerWrapper
getRegionServerWrapper() {
return regionServerWrapper;
}
- public void updatePutBatch(TableName tn, long t) {
- if (tableMetrics != null && tn != null) {
- tableMetrics.updatePutBatch(tn, t);
+ public void ifTableRequestsMetricsExist(HRegion region,
Consumer<MetricsTableRequests> consumer) {
+ if (region.getMetricsTableRequests() != null) {
+ consumer.accept(region.getMetricsTableRequests());
}
+ }
+
+ public void updatePutBatch(HRegion region, long t) {
+ ifTableRequestsMetricsExist(region, metrics -> metrics.updatePutBatch(t));
Review Comment:
can you replace these with normal null checks? similarly not sure we need
the overhead of a lambda.
(i had originally suggested a method like this back when the RSRpcServices
diff was really large, but now that it's all nicely encapsulated I don't think
it's needed.
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/metrics/MetricsTableRequests.java:
##########
@@ -0,0 +1,310 @@
+/*
+ * 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.metrics;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.metrics.Histogram;
+import org.apache.hadoop.hbase.metrics.Meter;
+import org.apache.hadoop.hbase.metrics.MetricRegistries;
+import org.apache.hadoop.hbase.metrics.MetricRegistry;
+import org.apache.hadoop.hbase.metrics.MetricRegistryInfo;
+import org.apache.yetus.audience.InterfaceAudience;
+
[email protected]
+public class MetricsTableRequests {
+
+ public static final String ENABLE_TABLE_LATENCIES_METRICS_KEY =
+ "hbase.regionserver.enable.table.latencies";
+
+ public static final boolean ENABLE_TABLE_LATENCIES_METRICS_DEFAULT = true;
+
+ public static final String ENABLE_TABLE_QUERY_METER_METRICS_KEY =
+ "hbase.regionserver.enable.table.query.meter";
+
+ public static final boolean ENABLE_TABLE_QUERY_METER_METRICS_KEY_DEFAULT =
false;
+
+ /**
+ * The name of the metrics
+ */
+ private final static String METRICS_NAME = "TableRequests";
+
+ /**
+ * The name of the metrics context that metrics will be under.
+ */
+ private final static String METRICS_CONTEXT = "regionserver";
+
+ /**
+ * Description
+ */
+ private final static String METRICS_DESCRIPTION =
+ "Metrics about Tables on a single HBase RegionServer";
+
+ /**
+ * The name of the metrics context that metrics will be under in jmx
+ */
+ private final static String METRICS_JMX_CONTEXT = "RegionServer,sub=" +
METRICS_NAME;
+
+ private final static String GET_TIME = "getTime";
+ private final static String SCAN_TIME = "scanTime";
+ private final static String SCAN_SIZE = "scanSize";
+ private final static String PUT_TIME = "putTime";
+ private final static String PUT_BATCH_TIME = "putBatchTime";
+ private final static String DELETE_TIME = "deleteTime";
+ private final static String DELETE_BATCH_TIME = "deleteBatchTime";
+ private final static String INCREMENT_TIME = "incrementTime";
+ private final static String APPEND_TIME = "appendTime";
+ private final static String CHECK_AND_DELETE_TIME = "checkAndDeleteTime";
+ private final static String CHECK_AND_PUT_TIME = "checkAndPutTime";
+ private final static String CHECK_AND_MUTATE_TIME = "checkAndMutateTime";
+ private final static String TABLE_READ_QUERY_PER_SECOND =
"tableReadQueryPerSecond";
+ private final static String TABLE_WRITE_QUERY_PER_SECOND =
"tableWriteQueryPerSecond";
+
+ private Histogram getTimeHistogram;
+ private Histogram scanTimeHistogram;
+ private Histogram scanSizeHistogram;
+ private Histogram putTimeHistogram;
+ private Histogram putBatchTimeHistogram;
+ private Histogram deleteTimeHistogram;
+ private Histogram deleteBatchTimeHistogram;
+ private Histogram incrementTimeHistogram;
+ private Histogram appendTimeHistogram;
+ private Histogram checkAndDeleteTimeHistogram;
+ private Histogram checkAndPutTimeHistogram;
+ private Histogram checkAndMutateTimeHistogram;
+
+ private Meter readMeter;
+ private Meter writeMeter;
+
+ private MetricRegistry registry;
+ private TableName tableName;
+ private Configuration conf;
+ private MetricRegistryInfo registryInfo;
+
+ private boolean enableTableLatenciesMetrics;
+ private boolean enabTableQueryMeterMetrics;
+
+ public boolean isEnableTableLatenciesMetrics() {
+ return enableTableLatenciesMetrics;
+ }
+
+ public boolean isEnabTableQueryMeterMetrics() {
+ return enabTableQueryMeterMetrics;
+ }
+
+ public MetricsTableRequests(TableName tableName, Configuration conf) {
+ init(tableName, conf);
+ }
+
+ private void init(TableName tableName, Configuration conf) {
+ this.tableName = tableName;
+ this.conf = conf;
+ enableTableLatenciesMetrics =
this.conf.getBoolean(ENABLE_TABLE_LATENCIES_METRICS_KEY,
+ ENABLE_TABLE_LATENCIES_METRICS_DEFAULT);
+ enabTableQueryMeterMetrics =
this.conf.getBoolean(ENABLE_TABLE_QUERY_METER_METRICS_KEY,
+ ENABLE_TABLE_QUERY_METER_METRICS_KEY_DEFAULT);
+ if (enableTableLatenciesMetrics || enabTableQueryMeterMetrics) {
+ registry = createRegistryForTableRequests();
+ if (enableTableLatenciesMetrics) {
+ getTimeHistogram = registry.histogram(GET_TIME);
+ scanTimeHistogram = registry.histogram(SCAN_TIME);
+ scanSizeHistogram = registry.histogram(SCAN_SIZE);
+ putTimeHistogram = registry.histogram(PUT_TIME);
+ putBatchTimeHistogram = registry.histogram(PUT_BATCH_TIME);
+ deleteTimeHistogram = registry.histogram(DELETE_TIME);
+ deleteBatchTimeHistogram = registry.histogram(DELETE_BATCH_TIME);
+ incrementTimeHistogram = registry.histogram(INCREMENT_TIME);
+ appendTimeHistogram = registry.histogram(APPEND_TIME);
+ checkAndDeleteTimeHistogram =
registry.histogram(CHECK_AND_DELETE_TIME);
+ checkAndPutTimeHistogram = registry.histogram(CHECK_AND_PUT_TIME);
+ checkAndMutateTimeHistogram =
registry.histogram(CHECK_AND_MUTATE_TIME);
+ }
+
+ if (enabTableQueryMeterMetrics) {
+ readMeter = registry.meter(TABLE_READ_QUERY_PER_SECOND);
+ writeMeter = registry.meter(TABLE_WRITE_QUERY_PER_SECOND);
+ }
+ }
+ }
+
+ private MetricRegistry createRegistryForTableRequests() {
+ return
MetricRegistries.global().create(createRegistryInfoForTableRequests());
+ }
+
+ private MetricRegistryInfo createRegistryInfoForTableRequests() {
+ registryInfo = new MetricRegistryInfo(qualifyMetrics(METRICS_NAME,
tableName),
+ METRICS_DESCRIPTION, qualifyMetrics(METRICS_JMX_CONTEXT, tableName),
METRICS_CONTEXT, false);
+ return registryInfo;
+ }
+
+ public void removeRegistry() {
+ if (enableTableLatenciesMetrics || enabTableQueryMeterMetrics) {
+ MetricRegistries.global().remove(registry.getMetricRegistryInfo());
+ }
+ }
+
+ private static String qualifyMetrics(String prefix, TableName tableName) {
+ StringBuilder sb = new StringBuilder();
+ sb.append(prefix).append("_");
+ sb.append("Namespace_").append(tableName.getNamespaceAsString());
+ sb.append("_table_").append(tableName.getQualifierAsString());
+ return sb.toString();
+ }
+
+ private void ifEnableTableLatenciesMetrics(Runnable runnable) {
+ if (isEnableTableLatenciesMetrics()) {
+ runnable.run();
+ }
+ }
+
+ private void ifEnabTableQueryMeterMetrics(Runnable runnable) {
+ if (isEnabTableQueryMeterMetrics()) {
+ runnable.run();
+ }
+ }
+
+ /**
+ * Update the Put time histogram
+ * @param t time it took
+ */
+ public void updatePut(long t) {
+ ifEnableTableLatenciesMetrics(() -> putTimeHistogram.update(t));
Review Comment:
can you repalce these lambda calls with normal if statements?
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java:
##########
@@ -962,6 +970,7 @@ long initialize(final CancelableProgressable reporter)
throws IOException {
}
}
+ Optional.ofNullable(metricsTableRequests).ifPresent(metrics ->
metrics.removeRegistry());
Review Comment:
can you replace this and below with normal null checks?
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/metrics/MetricsTableRequests.java:
##########
@@ -0,0 +1,310 @@
+/*
+ * 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.metrics;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.metrics.Histogram;
+import org.apache.hadoop.hbase.metrics.Meter;
+import org.apache.hadoop.hbase.metrics.MetricRegistries;
+import org.apache.hadoop.hbase.metrics.MetricRegistry;
+import org.apache.hadoop.hbase.metrics.MetricRegistryInfo;
+import org.apache.yetus.audience.InterfaceAudience;
+
[email protected]
+public class MetricsTableRequests {
+
+ public static final String ENABLE_TABLE_LATENCIES_METRICS_KEY =
+ "hbase.regionserver.enable.table.latencies";
+
+ public static final boolean ENABLE_TABLE_LATENCIES_METRICS_DEFAULT = true;
+
+ public static final String ENABLE_TABLE_QUERY_METER_METRICS_KEY =
+ "hbase.regionserver.enable.table.query.meter";
+
+ public static final boolean ENABLE_TABLE_QUERY_METER_METRICS_KEY_DEFAULT =
false;
+
+ /**
+ * The name of the metrics
+ */
+ private final static String METRICS_NAME = "TableRequests";
+
+ /**
+ * The name of the metrics context that metrics will be under.
+ */
+ private final static String METRICS_CONTEXT = "regionserver";
+
+ /**
+ * Description
+ */
+ private final static String METRICS_DESCRIPTION =
+ "Metrics about Tables on a single HBase RegionServer";
+
+ /**
+ * The name of the metrics context that metrics will be under in jmx
+ */
+ private final static String METRICS_JMX_CONTEXT = "RegionServer,sub=" +
METRICS_NAME;
+
+ private final static String GET_TIME = "getTime";
+ private final static String SCAN_TIME = "scanTime";
+ private final static String SCAN_SIZE = "scanSize";
+ private final static String PUT_TIME = "putTime";
+ private final static String PUT_BATCH_TIME = "putBatchTime";
+ private final static String DELETE_TIME = "deleteTime";
+ private final static String DELETE_BATCH_TIME = "deleteBatchTime";
+ private final static String INCREMENT_TIME = "incrementTime";
+ private final static String APPEND_TIME = "appendTime";
+ private final static String CHECK_AND_DELETE_TIME = "checkAndDeleteTime";
+ private final static String CHECK_AND_PUT_TIME = "checkAndPutTime";
+ private final static String CHECK_AND_MUTATE_TIME = "checkAndMutateTime";
+ private final static String TABLE_READ_QUERY_PER_SECOND =
"tableReadQueryPerSecond";
+ private final static String TABLE_WRITE_QUERY_PER_SECOND =
"tableWriteQueryPerSecond";
+
+ private Histogram getTimeHistogram;
+ private Histogram scanTimeHistogram;
+ private Histogram scanSizeHistogram;
+ private Histogram putTimeHistogram;
+ private Histogram putBatchTimeHistogram;
+ private Histogram deleteTimeHistogram;
+ private Histogram deleteBatchTimeHistogram;
+ private Histogram incrementTimeHistogram;
+ private Histogram appendTimeHistogram;
+ private Histogram checkAndDeleteTimeHistogram;
+ private Histogram checkAndPutTimeHistogram;
+ private Histogram checkAndMutateTimeHistogram;
+
+ private Meter readMeter;
+ private Meter writeMeter;
+
+ private MetricRegistry registry;
+ private TableName tableName;
+ private Configuration conf;
+ private MetricRegistryInfo registryInfo;
+
+ private boolean enableTableLatenciesMetrics;
+ private boolean enabTableQueryMeterMetrics;
+
+ public boolean isEnableTableLatenciesMetrics() {
+ return enableTableLatenciesMetrics;
+ }
+
+ public boolean isEnabTableQueryMeterMetrics() {
+ return enabTableQueryMeterMetrics;
+ }
+
+ public MetricsTableRequests(TableName tableName, Configuration conf) {
+ init(tableName, conf);
+ }
+
+ private void init(TableName tableName, Configuration conf) {
+ this.tableName = tableName;
+ this.conf = conf;
+ enableTableLatenciesMetrics =
this.conf.getBoolean(ENABLE_TABLE_LATENCIES_METRICS_KEY,
+ ENABLE_TABLE_LATENCIES_METRICS_DEFAULT);
+ enabTableQueryMeterMetrics =
this.conf.getBoolean(ENABLE_TABLE_QUERY_METER_METRICS_KEY,
+ ENABLE_TABLE_QUERY_METER_METRICS_KEY_DEFAULT);
+ if (enableTableLatenciesMetrics || enabTableQueryMeterMetrics) {
+ registry = createRegistryForTableRequests();
+ if (enableTableLatenciesMetrics) {
+ getTimeHistogram = registry.histogram(GET_TIME);
+ scanTimeHistogram = registry.histogram(SCAN_TIME);
+ scanSizeHistogram = registry.histogram(SCAN_SIZE);
+ putTimeHistogram = registry.histogram(PUT_TIME);
+ putBatchTimeHistogram = registry.histogram(PUT_BATCH_TIME);
+ deleteTimeHistogram = registry.histogram(DELETE_TIME);
+ deleteBatchTimeHistogram = registry.histogram(DELETE_BATCH_TIME);
+ incrementTimeHistogram = registry.histogram(INCREMENT_TIME);
+ appendTimeHistogram = registry.histogram(APPEND_TIME);
+ checkAndDeleteTimeHistogram =
registry.histogram(CHECK_AND_DELETE_TIME);
+ checkAndPutTimeHistogram = registry.histogram(CHECK_AND_PUT_TIME);
+ checkAndMutateTimeHistogram =
registry.histogram(CHECK_AND_MUTATE_TIME);
+ }
+
+ if (enabTableQueryMeterMetrics) {
+ readMeter = registry.meter(TABLE_READ_QUERY_PER_SECOND);
+ writeMeter = registry.meter(TABLE_WRITE_QUERY_PER_SECOND);
+ }
+ }
+ }
+
+ private MetricRegistry createRegistryForTableRequests() {
+ return
MetricRegistries.global().create(createRegistryInfoForTableRequests());
+ }
+
+ private MetricRegistryInfo createRegistryInfoForTableRequests() {
+ registryInfo = new MetricRegistryInfo(qualifyMetrics(METRICS_NAME,
tableName),
+ METRICS_DESCRIPTION, qualifyMetrics(METRICS_JMX_CONTEXT, tableName),
METRICS_CONTEXT, false);
+ return registryInfo;
+ }
+
+ public void removeRegistry() {
+ if (enableTableLatenciesMetrics || enabTableQueryMeterMetrics) {
+ MetricRegistries.global().remove(registry.getMetricRegistryInfo());
+ }
+ }
+
+ private static String qualifyMetrics(String prefix, TableName tableName) {
+ StringBuilder sb = new StringBuilder();
+ sb.append(prefix).append("_");
+ sb.append("Namespace_").append(tableName.getNamespaceAsString());
+ sb.append("_table_").append(tableName.getQualifierAsString());
+ return sb.toString();
+ }
+
+ private void ifEnableTableLatenciesMetrics(Runnable runnable) {
+ if (isEnableTableLatenciesMetrics()) {
+ runnable.run();
+ }
+ }
+
+ private void ifEnabTableQueryMeterMetrics(Runnable runnable) {
+ if (isEnabTableQueryMeterMetrics()) {
+ runnable.run();
+ }
+ }
+
+ /**
+ * Update the Put time histogram
+ * @param t time it took
+ */
+ public void updatePut(long t) {
+ ifEnableTableLatenciesMetrics(() -> putTimeHistogram.update(t));
+ }
+
+ /**
+ * Update the batch Put time histogram
+ * @param t time it took
+ */
+ public void updatePutBatch(long t) {
+ ifEnableTableLatenciesMetrics(() -> putBatchTimeHistogram.update(t));
+ }
+
+ /**
+ * Update the Delete time histogram
+ * @param t time it took
+ */
+ public void updateDelete(long t) {
+ ifEnableTableLatenciesMetrics(() -> deleteTimeHistogram.update(t));
+ }
+
+ /**
+ * Update the batch Delete time histogram
+ * @param t time it took
+ */
+ public void updateDeleteBatch(long t) {
+ ifEnableTableLatenciesMetrics(() -> deleteBatchTimeHistogram.update(t));
+ }
+
+ /**
+ * Update the Get time histogram .
+ * @param t time it took
+ */
+ public void updateGet(long t) {
+ ifEnableTableLatenciesMetrics(() -> getTimeHistogram.update(t));
+ }
+
+ /**
+ * Update the Increment time histogram.
+ * @param t time it took
+ */
+ public void updateIncrement(long t) {
+ ifEnableTableLatenciesMetrics(() -> incrementTimeHistogram.update(t));
+ }
+
+ /**
+ * Update the Append time histogram.
+ * @param t time it took
+ */
+ public void updateAppend(long t) {
+ ifEnableTableLatenciesMetrics(() -> appendTimeHistogram.update(t));
+ }
+
+ /**
+ * Update the scan size.
+ * @param scanSize size of the scan
+ */
+ public void updateScanSize(long scanSize) {
+ ifEnableTableLatenciesMetrics(() -> scanSizeHistogram.update(scanSize));
+ }
+
+ /**
+ * Update the scan time.
+ * @param t time it took
+ */
+ public void updateScanTime(long t) {
+ ifEnableTableLatenciesMetrics(() -> scanTimeHistogram.update(t));
+ }
+
+ /**
+ * Update the CheckAndDelete time histogram.
+ * @param time time it took
+ */
+ public void updateCheckAndDelete(long time) {
+ ifEnableTableLatenciesMetrics(() ->
checkAndDeleteTimeHistogram.update(time));
+ }
+
+ /**
+ * Update the CheckAndPut time histogram.
+ * @param time time it took
+ */
+ public void updateCheckAndPut(long time) {
+ ifEnableTableLatenciesMetrics(() -> checkAndPutTimeHistogram.update(time));
+ }
+
+ /**
+ * Update the CheckAndMutate time histogram.
+ * @param time time it took
+ */
+ public void updateCheckAndMutate(long time) {
+ ifEnableTableLatenciesMetrics(() ->
checkAndMutateTimeHistogram.update(time));
+ }
+
+ /**
+ * Update table read QPS
+ * @param count Number of occurrences to record
+ */
+ public void updateTableReadQueryMeter(long count) {
+ ifEnabTableQueryMeterMetrics(() -> readMeter.mark(count));
+ }
+
+ /**
+ * Update table read QPS
+ */
+ public void updateTableReadQueryMeter() {
+ ifEnabTableQueryMeterMetrics(() -> readMeter.mark());
+ }
+
+ /**
+ * Update table write QPS
+ * @param count Number of occurrences to record
+ */
+ public void updateTableWriteQueryMeter(long count) {
+ ifEnabTableQueryMeterMetrics(() -> writeMeter.mark(count));
+ }
+
+ /**
+ * Update table write QPS
+ */
+ public void updateTableWriteQueryMeter() {
+ ifEnabTableQueryMeterMetrics(() -> writeMeter.mark());
+ }
+
+ public MetricRegistryInfo getMetricRegistryInfo() {
Review Comment:
Can you add a `// Visible for testing` comment?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
