You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@phoenix.apache.org by "ASF GitHub Bot (Jira)" <ji...@apache.org> on 2021/10/04 00:44:00 UTC
[jira] [Commented] (PHOENIX-5838) Add Histograms for Table level
Metrics.
[ https://issues.apache.org/jira/browse/PHOENIX-5838?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17423744#comment-17423744 ]
ASF GitHub Bot commented on PHOENIX-5838:
-----------------------------------------
dbwong commented on a change in pull request #1231:
URL: https://github.com/apache/phoenix/pull/1231#discussion_r720941481
##########
File path: phoenix-core/src/it/java/org/apache/phoenix/monitoring/PhoenixTableLevelMetricsIT.java
##########
@@ -1148,13 +1195,156 @@ private static void assertMetricValue(Metric m, MetricType checkType, long compa
}
}
+ @Test
+ public void testHistogramMetricsForMutations() throws Exception {
+ String tableName = generateUniqueName();
+ // Reset table level metrics to capture histogram metrics for upsert.
+ try (Connection conn = getConnFromTestDriver()) {
+ createTableAndInsertValues(tableName, true, true, 10, true, conn, false);
+ }
+ // Metrics will be reset after creation of table so below we will get latency
+ // just for upsert queries.
+ // Since we are recording latency histograms after every executeMutation method and
+ // since we are not batch upserting, it will record histogram event after every upsert.
+ assertHistogramMetricsForMutations(tableName, true, 1, 1, true);
+
+ // Reset table histograms as well as global metrics
+ PhoenixRuntime.clearTableLevelMetrics();
+ PhoenixMetricsIT.resetGlobalMetrics();
+ try (Connection connection = getConnFromTestDriver();
+ Statement statement = connection.createStatement()) {
+ String delete = "DELETE FROM " + tableName;
+ statement.execute(delete);
+ connection.commit();
+ }
+ // Verify metrics for delete mutations
+ assertHistogramMetricsForMutations(tableName, false, 1, 1, true);
+ PhoenixRuntime.clearTableLevelMetrics();
+ }
+
+ @Test
+ public void testHistogramMetricsForMutationsAutoCommitTrue() throws Exception {
+ String tableName = generateUniqueName();
+ // Reset table level metrics to capture histogram metrics for upsert.
+ try (Connection conn = getConnFromTestDriver()) {
+ conn.setAutoCommit(true);
+ createTableAndInsertValues(tableName, true, true, 10, false, conn, false);
+ }
+ // Metrics will be reset after creation of table so below we will get latency
+ // just for upsert queries.
+ // Since we are recording latency histograms after every executeMutation method and
+ // since we are not batch upserting, it will record histogram event after every upsert.
+ assertHistogramMetricsForMutations(tableName, true, 10, 10, false);
+
+ // Reset table histograms as well as global metrics
+ PhoenixRuntime.clearTableLevelMetrics();
+ PhoenixMetricsIT.resetGlobalMetrics();
+ try (Connection connection = getConnFromTestDriver();
+ Statement statement = connection.createStatement()) {
+ connection.setAutoCommit(true);
+ String delete = "DELETE FROM " + tableName;
+ statement.execute(delete);
+ }
+ // Verify metrics for delete mutations. We won't get any data point for
+ // size histogram since delete happened on server side using ServerSelectDeleteMutationPlan.
Review comment:
How difficult is it to include server side metrics back up into table metrics?
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/monitoring/TableHistograms.java
##########
@@ -0,0 +1,120 @@
+/*
+ * 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 maynot 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 applicablelaw 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.phoenix.monitoring;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+import org.apache.hadoop.conf.Configuration;
+
+public class TableHistograms {
+ private String tableName;
+ private LatencyHistogram queryLatencyHisto;
+ private SizeHistogram querySizeHisto;
+ private LatencyHistogram upsertLatencyHisto;
+ private SizeHistogram upsertSizeHisto;
+ private LatencyHistogram deleteLatencyHisto;
+ private SizeHistogram deleteSizeHisto;
+ private LatencyHistogram pointLookupLatencyHisto;
+ private SizeHistogram pointLookupSizeHisto;
+ private LatencyHistogram rangeScanLatencyHisto;
+ private SizeHistogram rangeScanSizeHisto;
+
+ public TableHistograms(String tableName, Configuration conf) {
+ this.tableName = tableName;
+ queryLatencyHisto = new LatencyHistogram("QueryTime", "Query time latency", conf);
+ querySizeHisto = new SizeHistogram("QuerySize", "Query size", conf);
+
+ upsertLatencyHisto = new LatencyHistogram("UpsertTime", "Upsert time latency", conf);
+ upsertSizeHisto = new SizeHistogram("UpsertSize", "Upsert size", conf);
+
+ deleteLatencyHisto = new LatencyHistogram("DeleteTime", "Delete time latency", conf);
+ deleteSizeHisto = new SizeHistogram("DeleteSize", "Delete size", conf);
+
+ pointLookupLatencyHisto = new LatencyHistogram("PointLookupTime",
+ "Point Lookup Query time latency", conf);
+ pointLookupSizeHisto = new SizeHistogram("PointLookupSize",
+ "Point Lookup Query Size", conf);
+
+ rangeScanLatencyHisto = new LatencyHistogram("RangeScanTime",
+ "Range Scan Query time latency", conf);
+ rangeScanSizeHisto = new SizeHistogram("RangeScanSize",
+ "Range Scan Query size", conf);
+ }
+
+ public String getTableName() {
+ return tableName;
+ }
+
+ public LatencyHistogram getQueryLatencyHisto() {
+ return queryLatencyHisto;
+ }
+
+ public SizeHistogram getQuerySizeHisto() {
+ return querySizeHisto;
+ }
+
+
+ public LatencyHistogram getPointLookupLatencyHisto() {
+ return pointLookupLatencyHisto;
+ }
+
+ public SizeHistogram getPointLookupSizeHisto() {
+ return pointLookupSizeHisto;
+ }
+
+ public LatencyHistogram getRangeScanLatencyHisto() {
+ return rangeScanLatencyHisto;
+ }
+
+ public SizeHistogram getRangeScanSizeHisto() {
+ return rangeScanSizeHisto;
+ }
+
+ public LatencyHistogram getUpsertLatencyHisto() {
+ return upsertLatencyHisto;
+ }
+
+ public SizeHistogram getUpsertSizeHisto() {
+ return upsertSizeHisto;
+ }
+
+ public LatencyHistogram getDeleteLatencyHisto() {
+ return deleteLatencyHisto;
+ }
+
+ public SizeHistogram getDeleteSizeHisto() {
+ return deleteSizeHisto;
+ }
+
+ public List<HistogramDistribution> getTableLatencyHistogramsDistribution() {
+ List<HistogramDistribution> list = new ArrayList<>(Arrays.asList(queryLatencyHisto.getRangeHistogramDistribution(),
+ upsertLatencyHisto.getRangeHistogramDistribution(), deleteLatencyHisto.getRangeHistogramDistribution(),
+ pointLookupLatencyHisto.getRangeHistogramDistribution(), rangeScanLatencyHisto.getRangeHistogramDistribution()));
+ return Collections.unmodifiableList(list);
Review comment:
Why do we need to wrap this list in an unmodifiable list?
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/monitoring/HistogramDistributionImpl.java
##########
@@ -0,0 +1,74 @@
+/*
+ * 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.phoenix.monitoring;
+
+import java.util.Map;
+
+public class HistogramDistributionImpl implements HistogramDistribution {
+ private String histoName;
+ private long min;
+ private long max;
+ private long count;
+ private Map<String, Long> rangeDistribution;
+
+ public HistogramDistributionImpl(String histoName) {
+ this.histoName = histoName;
+ }
+
+ public void setMin(long min) {
Review comment:
Does this class have to be mutable? I don't see us using this very often.
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/monitoring/RangeHistogram.java
##########
@@ -0,0 +1,113 @@
+/*
+ * 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 maynot 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.phoenix.monitoring;
+
+import com.google.common.base.Preconditions;
+import java.util.HashMap;
+import java.util.Map;
+import org.HdrHistogram.ConcurrentHistogram;
+import org.HdrHistogram.Histogram;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/*
+ Creates a histogram with the specified range.
+ */
+public class RangeHistogram {
+ private Histogram histogram;
+ private long[] ranges;
+ private String name;
+ private String desc;
+ private static final Logger LOGGER = LoggerFactory.getLogger(RangeHistogram.class);
+
+ public RangeHistogram(long[] ranges, String name, String description) {
+ Preconditions.checkNotNull(ranges);
+ Preconditions.checkArgument(ranges.length != 0);
+ this.ranges = ranges;
+ this.name = name;
+ this.desc = description;
+ /*
+ Below is the memory footprint per precision as of hdrhistogram version 2.1.12
+ Histogram#getEstimatedFootprintInBytes provide a (conservatively high) estimate
+ of the Histogram's total footprint in bytes.
+ |-----------------------------------------|
+ |PRECISION | ERROR RATE | SIZE IN BYTES |
+ | 1 | 10% | 3,584 |
+ | 2 | 1% | 22,016 |
+ | 3 | 0.1% | 147,968 |
+ | 4 | 0.01% | 1,835,520 |
+ | 5 | 0.001% | 11,534,848 |
+ |-----------------------------------------|
+ */
+ // highestTrackable value is the last value in the provided range.
+ this.histogram = new ConcurrentHistogram(this.ranges[this.ranges.length-1], 2);
+ }
+
+ public void add(long value) {
+ if (value > histogram.getHighestTrackableValue()) {
+ // Ignoring recording value more than maximum trackable value.
+ LOGGER.debug("Histogram recording higher value than maximum. Ignoring it.");
+ return;
+ }
+ histogram.recordValue(value);
+ }
+
+ public Histogram getHistogram() {
+ return histogram;
+ }
+
+ public long[] getRanges() {
+ return ranges;
+ }
+
+ public String getName() {
+ return name;
+ }
+
+ public String getDesc() {
+ return desc;
+ }
+
+ public HistogramDistribution getRangeHistogramDistribution() {
+ // Generate distribution from the snapshot.
+ Histogram snapshot = histogram.copy();
+ HistogramDistributionImpl distribution = new HistogramDistributionImpl(name);
+ distribution.setMin(snapshot.getMinValue());
Review comment:
See notes on mutability above
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/monitoring/PhoenixTableMetricImpl.java
##########
@@ -74,4 +74,5 @@ public PhoenixTableMetricImpl(MetricType type) {
metric.decrement();
numberOfSamples.incrementAndGet();
}
+
Review comment:
Don't need this change
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/monitoring/RangeHistogram.java
##########
@@ -0,0 +1,113 @@
+/*
+ * 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 maynot 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.phoenix.monitoring;
+
+import com.google.common.base.Preconditions;
+import java.util.HashMap;
+import java.util.Map;
+import org.HdrHistogram.ConcurrentHistogram;
+import org.HdrHistogram.Histogram;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/*
+ Creates a histogram with the specified range.
+ */
+public class RangeHistogram {
+ private Histogram histogram;
+ private long[] ranges;
+ private String name;
+ private String desc;
+ private static final Logger LOGGER = LoggerFactory.getLogger(RangeHistogram.class);
+
+ public RangeHistogram(long[] ranges, String name, String description) {
+ Preconditions.checkNotNull(ranges);
+ Preconditions.checkArgument(ranges.length != 0);
+ this.ranges = ranges;
+ this.name = name;
+ this.desc = description;
+ /*
+ Below is the memory footprint per precision as of hdrhistogram version 2.1.12
+ Histogram#getEstimatedFootprintInBytes provide a (conservatively high) estimate
+ of the Histogram's total footprint in bytes.
+ |-----------------------------------------|
+ |PRECISION | ERROR RATE | SIZE IN BYTES |
+ | 1 | 10% | 3,584 |
+ | 2 | 1% | 22,016 |
+ | 3 | 0.1% | 147,968 |
+ | 4 | 0.01% | 1,835,520 |
+ | 5 | 0.001% | 11,534,848 |
+ |-----------------------------------------|
+ */
+ // highestTrackable value is the last value in the provided range.
+ this.histogram = new ConcurrentHistogram(this.ranges[this.ranges.length-1], 2);
+ }
+
+ public void add(long value) {
+ if (value > histogram.getHighestTrackableValue()) {
+ // Ignoring recording value more than maximum trackable value.
+ LOGGER.debug("Histogram recording higher value than maximum. Ignoring it.");
Review comment:
Consider warn since this appears to be dropping metrics
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/monitoring/SizeHistogram.java
##########
@@ -0,0 +1,43 @@
+/*
+ * 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 maynot 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.phoenix.monitoring;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.phoenix.mapreduce.util.PhoenixConfigurationUtil;
+import org.apache.phoenix.query.QueryServices;
+
+/**
+ * Histogram for calculating sizes (for eg: bytes read, bytes scanned). We read ranges using
+ * config property {@link QueryServices#PHOENIX_HISTOGRAM_SIZE_RANGES}. If this property is not set
+ * then it will default to {@link org.apache.hadoop.metrics2.lib.MutableSizeHistogram#RANGES}
+ * values.
+ */
+public class SizeHistogram extends RangeHistogram {
+
+ public static long[] DEFAULT_RANGE = {10,100,1000,10000,100000,1000000,10000000,100000000};
+ public SizeHistogram(String name, String description, Configuration conf) {
+ super(initializeRanges(conf), name, description);
+ initializeRanges(conf);
+ }
+
+ private static long[] initializeRanges(Configuration conf) {
+ long[] ranges = PhoenixConfigurationUtil.getLongs(conf,
+ QueryServices.PHOENIX_HISTOGRAM_SIZE_RANGES);
+ return ranges != null ? ranges : DEFAULT_RANGE;
+ }
+}
Review comment:
+1
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/monitoring/TableHistograms.java
##########
@@ -0,0 +1,120 @@
+/*
+ * 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 maynot 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 applicablelaw 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.phoenix.monitoring;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+import org.apache.hadoop.conf.Configuration;
+
+public class TableHistograms {
+ private String tableName;
+ private LatencyHistogram queryLatencyHisto;
+ private SizeHistogram querySizeHisto;
+ private LatencyHistogram upsertLatencyHisto;
+ private SizeHistogram upsertSizeHisto;
+ private LatencyHistogram deleteLatencyHisto;
+ private SizeHistogram deleteSizeHisto;
+ private LatencyHistogram pointLookupLatencyHisto;
+ private SizeHistogram pointLookupSizeHisto;
+ private LatencyHistogram rangeScanLatencyHisto;
+ private SizeHistogram rangeScanSizeHisto;
+
+ public TableHistograms(String tableName, Configuration conf) {
+ this.tableName = tableName;
+ queryLatencyHisto = new LatencyHistogram("QueryTime", "Query time latency", conf);
+ querySizeHisto = new SizeHistogram("QuerySize", "Query size", conf);
+
+ upsertLatencyHisto = new LatencyHistogram("UpsertTime", "Upsert time latency", conf);
+ upsertSizeHisto = new SizeHistogram("UpsertSize", "Upsert size", conf);
+
+ deleteLatencyHisto = new LatencyHistogram("DeleteTime", "Delete time latency", conf);
+ deleteSizeHisto = new SizeHistogram("DeleteSize", "Delete size", conf);
+
+ pointLookupLatencyHisto = new LatencyHistogram("PointLookupTime",
+ "Point Lookup Query time latency", conf);
+ pointLookupSizeHisto = new SizeHistogram("PointLookupSize",
+ "Point Lookup Query Size", conf);
+
+ rangeScanLatencyHisto = new LatencyHistogram("RangeScanTime",
+ "Range Scan Query time latency", conf);
+ rangeScanSizeHisto = new SizeHistogram("RangeScanSize",
+ "Range Scan Query size", conf);
+ }
+
+ public String getTableName() {
+ return tableName;
+ }
+
+ public LatencyHistogram getQueryLatencyHisto() {
+ return queryLatencyHisto;
+ }
+
+ public SizeHistogram getQuerySizeHisto() {
+ return querySizeHisto;
+ }
+
+
+ public LatencyHistogram getPointLookupLatencyHisto() {
+ return pointLookupLatencyHisto;
+ }
+
+ public SizeHistogram getPointLookupSizeHisto() {
+ return pointLookupSizeHisto;
+ }
+
+ public LatencyHistogram getRangeScanLatencyHisto() {
+ return rangeScanLatencyHisto;
+ }
+
+ public SizeHistogram getRangeScanSizeHisto() {
+ return rangeScanSizeHisto;
+ }
+
+ public LatencyHistogram getUpsertLatencyHisto() {
+ return upsertLatencyHisto;
+ }
+
+ public SizeHistogram getUpsertSizeHisto() {
+ return upsertSizeHisto;
+ }
+
+ public LatencyHistogram getDeleteLatencyHisto() {
+ return deleteLatencyHisto;
+ }
+
+ public SizeHistogram getDeleteSizeHisto() {
+ return deleteSizeHisto;
+ }
+
+ public List<HistogramDistribution> getTableLatencyHistogramsDistribution() {
+ List<HistogramDistribution> list = new ArrayList<>(Arrays.asList(queryLatencyHisto.getRangeHistogramDistribution(),
+ upsertLatencyHisto.getRangeHistogramDistribution(), deleteLatencyHisto.getRangeHistogramDistribution(),
+ pointLookupLatencyHisto.getRangeHistogramDistribution(), rangeScanLatencyHisto.getRangeHistogramDistribution()));
+ return Collections.unmodifiableList(list);
+ }
+
+ public List<HistogramDistribution> getTableSizeHistogramsDistribution() {
+ List<HistogramDistribution> list = new ArrayList<>(Arrays.asList(querySizeHisto.getRangeHistogramDistribution(),
+ upsertSizeHisto.getRangeHistogramDistribution(), deleteSizeHisto.getRangeHistogramDistribution(),
+ pointLookupSizeHisto.getRangeHistogramDistribution(), rangeScanSizeHisto.getRangeHistogramDistribution()));
+ return Collections.unmodifiableList(list);
Review comment:
Same question, Why do we need to wrap this list in an unmodifiable list?
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/monitoring/TableMetricsManager.java
##########
@@ -287,4 +287,179 @@ public void clearTableLevelMetrics() {
public void clear() {
TableMetricsManager.clearTableLevelMetricsMethod();
}
+
+ public static Map<String, List<HistogramDistribution>> getSizeHistogramsForAllTables() {
+ Map<String, List<HistogramDistribution>> map = new HashMap<>();
+ for (Map.Entry<String, TableClientMetrics> entry: tableClientMetricsMapping.entrySet()) {
+ TableHistograms tableHistograms = entry.getValue().getTableHistograms();
+ map.put(entry.getKey(), tableHistograms.getTableSizeHistogramsDistribution());
+ }
+ return map;
+ }
+
+ public static Map<String, List<HistogramDistribution>> getLatencyHistogramsForAllTables() {
+ Map<String, List<HistogramDistribution>> map = new HashMap<>();
+ for (Map.Entry<String, TableClientMetrics> entry: tableClientMetricsMapping.entrySet()) {
+ TableHistograms tableHistograms = entry.getValue().getTableHistograms();
+ map.put(entry.getKey(), tableHistograms.getTableLatencyHistogramsDistribution());
+ }
+ return map;
+ }
+
+ public static LatencyHistogram getUpsertLatencyHistogramForTable(String tableName) {
Review comment:
A ton of these methods have the same patter of get the TableClientMetrics then null handle it... any way we can reduce the copy paste/boiler plate code here?
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/monitoring/TableHistograms.java
##########
@@ -0,0 +1,120 @@
+/*
+ * 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 maynot 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 applicablelaw 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.phoenix.monitoring;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+import org.apache.hadoop.conf.Configuration;
+
+public class TableHistograms {
+ private String tableName;
+ private LatencyHistogram queryLatencyHisto;
+ private SizeHistogram querySizeHisto;
+ private LatencyHistogram upsertLatencyHisto;
+ private SizeHistogram upsertSizeHisto;
+ private LatencyHistogram deleteLatencyHisto;
+ private SizeHistogram deleteSizeHisto;
+ private LatencyHistogram pointLookupLatencyHisto;
+ private SizeHistogram pointLookupSizeHisto;
+ private LatencyHistogram rangeScanLatencyHisto;
+ private SizeHistogram rangeScanSizeHisto;
+
+ public TableHistograms(String tableName, Configuration conf) {
+ this.tableName = tableName;
+ queryLatencyHisto = new LatencyHistogram("QueryTime", "Query time latency", conf);
+ querySizeHisto = new SizeHistogram("QuerySize", "Query size", conf);
+
+ upsertLatencyHisto = new LatencyHistogram("UpsertTime", "Upsert time latency", conf);
+ upsertSizeHisto = new SizeHistogram("UpsertSize", "Upsert size", conf);
+
+ deleteLatencyHisto = new LatencyHistogram("DeleteTime", "Delete time latency", conf);
+ deleteSizeHisto = new SizeHistogram("DeleteSize", "Delete size", conf);
+
+ pointLookupLatencyHisto = new LatencyHistogram("PointLookupTime",
+ "Point Lookup Query time latency", conf);
+ pointLookupSizeHisto = new SizeHistogram("PointLookupSize",
+ "Point Lookup Query Size", conf);
+
+ rangeScanLatencyHisto = new LatencyHistogram("RangeScanTime",
+ "Range Scan Query time latency", conf);
+ rangeScanSizeHisto = new SizeHistogram("RangeScanSize",
+ "Range Scan Query size", conf);
+ }
+
+ public String getTableName() {
+ return tableName;
+ }
+
+ public LatencyHistogram getQueryLatencyHisto() {
+ return queryLatencyHisto;
+ }
+
+ public SizeHistogram getQuerySizeHisto() {
+ return querySizeHisto;
+ }
+
+
+ public LatencyHistogram getPointLookupLatencyHisto() {
+ return pointLookupLatencyHisto;
+ }
+
+ public SizeHistogram getPointLookupSizeHisto() {
+ return pointLookupSizeHisto;
+ }
+
+ public LatencyHistogram getRangeScanLatencyHisto() {
+ return rangeScanLatencyHisto;
+ }
+
+ public SizeHistogram getRangeScanSizeHisto() {
+ return rangeScanSizeHisto;
+ }
+
+ public LatencyHistogram getUpsertLatencyHisto() {
+ return upsertLatencyHisto;
+ }
+
+ public SizeHistogram getUpsertSizeHisto() {
+ return upsertSizeHisto;
+ }
+
+ public LatencyHistogram getDeleteLatencyHisto() {
+ return deleteLatencyHisto;
+ }
+
+ public SizeHistogram getDeleteSizeHisto() {
+ return deleteSizeHisto;
+ }
+
+ public List<HistogramDistribution> getTableLatencyHistogramsDistribution() {
+ List<HistogramDistribution> list = new ArrayList<>(Arrays.asList(queryLatencyHisto.getRangeHistogramDistribution(),
Review comment:
Why are we using new and Arrays?
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/monitoring/LatencyHistogram.java
##########
@@ -0,0 +1,45 @@
+/*
+ * 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 maynot 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 applicablelaw 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.phoenix.monitoring;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.phoenix.mapreduce.util.PhoenixConfigurationUtil;
+import org.apache.phoenix.query.QueryServices;
+
+/**
+ * Histogram for calculating latencies. We read ranges using
+ * config property {@link QueryServices#PHOENIX_HISTOGRAM_LATENCY_RANGES}.
+ * If this property is not set then it will default to
+ * {@link org.apache.hadoop.metrics2.lib.MutableTimeHistogram#RANGES} values.
+ */
+public class LatencyHistogram extends RangeHistogram {
+
+ public final static long[] DEFAULT_RANGE =
+ { 1, 3, 10, 30, 100, 300, 1000, 3000, 10000, 30000, 60000, 120000, 300000, 600000};
Review comment:
Can you add a comment on what these ranges do?
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/monitoring/RangeHistogram.java
##########
@@ -0,0 +1,112 @@
+/*
+ * 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 maynot 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.phoenix.monitoring;
+
+import com.google.common.base.Preconditions;
+import java.util.HashMap;
+import java.util.Map;
+import org.HdrHistogram.ConcurrentHistogram;
+import org.HdrHistogram.Histogram;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/*
+ Creates a histogram with the specified range.
+ */
+public class RangeHistogram {
+ private Histogram histogram;
+ private long[] ranges;
+ private String name;
+ private String desc;
+ private static final Logger LOGGER = LoggerFactory.getLogger(RangeHistogram.class);
+
+ public RangeHistogram(long[] ranges, String name, String description) {
+ Preconditions.checkNotNull(ranges);
+ Preconditions.checkArgument(ranges.length != 0);
+ this.ranges = ranges;
+ this.name = name;
+ this.desc = description;
+ /*
+ Below is the memory footprint per precision as of hdrhistogram version 2.1.12
+ Histogram#getEstimatedFootprintInBytes provide a (conservatively high) estimate
+ of the Histogram's total footprint in bytes.
+ |-----------------------------------------|
+ |PRECISION | ERROR RATE | SIZE IN BYTES |
+ | 1 | 10% | 3,584 |
+ | 2 | 1% | 22,016 |
+ | 3 | 0.1% | 147,968 |
+ | 4 | 0.01% | 1,835,520 |
+ | 5 | 0.001% | 11,534,848 |
+ |-----------------------------------------|
+ */
+ // highestTrackable value is the last value in the provided range.
+ this.histogram = new ConcurrentHistogram(this.ranges[this.ranges.length-1], 2);
+ }
+
+ public void add(long value) {
+ if (value > histogram.getHighestTrackableValue()) {
+ // Ignoring recording value more than maximum trackable value.
+ LOGGER.debug("Histogram recording higher value than maximum. Ignoring it.");
+ return;
+ }
+ histogram.recordValue(value);
+ }
+
+ public Histogram getHistogram() {
+ return histogram;
+ }
+
+ public long[] getRanges() {
+ return ranges;
+ }
+
+ public String getName() {
+ return name;
+ }
+
+ public String getDesc() {
+ return desc;
+ }
+
+ public HistogramDistribution getRangeHistogramDistribution() {
+ // Generate distribution from the snapshot.
+ Histogram snapshot = histogram.copy();
+ HistogramDistributionImpl distribution = new HistogramDistributionImpl(name);
+ distribution.setMin(snapshot.getMinValue());
+ distribution.setMax(snapshot.getMaxValue());
+ distribution.setCount(snapshot.getTotalCount());
+ distribution.setRangeDistributionMap(generateDistributionMap(snapshot));
+ return distribution;
+ }
+
+ private Map<String, Long> generateDistributionMap(Histogram snapshot) {
+ long priorRange = 0;
+ Map<String, Long> map = new HashMap<>();
+ for (int i = 0; i < ranges.length; i++) {
+ // We get the next non equivalent range to avoid double counting.
+ // getCountBetweenValues is inclusive of both values but since we are getting
+ // next non equivalent value from the lower bound it will be more than priorRange.
+ long nextNonEquivalentRange = histogram.nextNonEquivalentValue(priorRange);
+ // lower exclusive upper inclusive
+ long val = snapshot.getCountBetweenValues(nextNonEquivalentRange, ranges[i]);
+ map.put(priorRange + "," + ranges[i], val);
+ priorRange = ranges[i];
+ }
+ return map;
+ }
+}
Review comment:
+1
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/monitoring/LatencyHistogram.java
##########
@@ -0,0 +1,44 @@
+/*
+ * 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 maynot 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 applicablelaw 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.phoenix.monitoring;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.phoenix.mapreduce.util.PhoenixConfigurationUtil;
+import org.apache.phoenix.query.QueryServices;
+
+/**
+ * Histogram for calculating latencies. We read ranges using
+ * config property {@link QueryServices#PHOENIX_HISTOGRAM_LATENCY_RANGES}.
+ * If this property is not set then it will default to
+ * {@link org.apache.hadoop.metrics2.lib.MutableTimeHistogram#RANGES} values.
+ */
+public class LatencyHistogram extends RangeHistogram {
+
+ public final static long[] DEFAULT_RANGE =
+ { 1, 3, 10, 30, 100, 300, 1000, 3000, 10000, 30000, 60000, 120000, 300000, 600000};
+
+ public LatencyHistogram(String name, String description, Configuration conf) {
+ super(initializeRanges(conf), name, description);
+ }
+
+ private static long[] initializeRanges(Configuration conf) {
+ long[] ranges = PhoenixConfigurationUtil.getLongs(conf,
+ QueryServices.PHOENIX_HISTOGRAM_LATENCY_RANGES);
+ return ranges != null ? ranges : DEFAULT_RANGE;
+ }
+}
Review comment:
+1
##########
File path: phoenix-core/src/it/java/org/apache/phoenix/monitoring/PhoenixTableLevelMetricsIT.java
##########
@@ -1148,13 +1195,156 @@ private static void assertMetricValue(Metric m, MetricType checkType, long compa
}
}
+ @Test
+ public void testHistogramMetricsForMutations() throws Exception {
+ String tableName = generateUniqueName();
+ // Reset table level metrics to capture histogram metrics for upsert.
+ try (Connection conn = getConnFromTestDriver()) {
+ createTableAndInsertValues(tableName, true, true, 10, true, conn, false);
+ }
+ // Metrics will be reset after creation of table so below we will get latency
+ // just for upsert queries.
+ // Since we are recording latency histograms after every executeMutation method and
+ // since we are not batch upserting, it will record histogram event after every upsert.
+ assertHistogramMetricsForMutations(tableName, true, 1, 1, true);
+
+ // Reset table histograms as well as global metrics
+ PhoenixRuntime.clearTableLevelMetrics();
+ PhoenixMetricsIT.resetGlobalMetrics();
+ try (Connection connection = getConnFromTestDriver();
+ Statement statement = connection.createStatement()) {
+ String delete = "DELETE FROM " + tableName;
+ statement.execute(delete);
+ connection.commit();
+ }
+ // Verify metrics for delete mutations
+ assertHistogramMetricsForMutations(tableName, false, 1, 1, true);
+ PhoenixRuntime.clearTableLevelMetrics();
+ }
+
+ @Test
+ public void testHistogramMetricsForMutationsAutoCommitTrue() throws Exception {
+ String tableName = generateUniqueName();
+ // Reset table level metrics to capture histogram metrics for upsert.
+ try (Connection conn = getConnFromTestDriver()) {
+ conn.setAutoCommit(true);
+ createTableAndInsertValues(tableName, true, true, 10, false, conn, false);
+ }
+ // Metrics will be reset after creation of table so below we will get latency
+ // just for upsert queries.
+ // Since we are recording latency histograms after every executeMutation method and
+ // since we are not batch upserting, it will record histogram event after every upsert.
+ assertHistogramMetricsForMutations(tableName, true, 10, 10, false);
+
+ // Reset table histograms as well as global metrics
+ PhoenixRuntime.clearTableLevelMetrics();
+ PhoenixMetricsIT.resetGlobalMetrics();
+ try (Connection connection = getConnFromTestDriver();
+ Statement statement = connection.createStatement()) {
+ connection.setAutoCommit(true);
+ String delete = "DELETE FROM " + tableName;
+ statement.execute(delete);
+ }
+ // Verify metrics for delete mutations. We won't get any data point for
+ // size histogram since delete happened on server side using ServerSelectDeleteMutationPlan.
+ assertHistogramMetricsForMutations(tableName, false,1, 0, false);
+ PhoenixRuntime.clearTableLevelMetrics();
+ }
+
+ @Test
+ public void testHistogramMetricsForQueries() throws Exception {
+ String tableName = generateUniqueName();
+ // Reset table level metrics to capture histogram metrics for select queries.
+ try (Connection conn = getConnFromTestDriver()) {
+ createTableAndInsertValues(tableName, true, true, 10, true, conn, true);
+ }
+ // Reset table metrics as well as global metrics
+ PhoenixRuntime.clearTableLevelMetrics();
+ PhoenixMetricsIT.resetGlobalMetrics();
+ DelayedOrFailingRegionServer.setDelayEnabled(true);
+ DelayedOrFailingRegionServer.setDelayScan(30);
Review comment:
I don't see us using the delayed rs here. Do we need this? Were we thinking of asserting the delay was at least 30?
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/monitoring/TableHistograms.java
##########
@@ -0,0 +1,120 @@
+/*
+ * 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 maynot 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 applicablelaw 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.phoenix.monitoring;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+import org.apache.hadoop.conf.Configuration;
+
+public class TableHistograms {
+ private String tableName;
+ private LatencyHistogram queryLatencyHisto;
+ private SizeHistogram querySizeHisto;
+ private LatencyHistogram upsertLatencyHisto;
+ private SizeHistogram upsertSizeHisto;
+ private LatencyHistogram deleteLatencyHisto;
+ private SizeHistogram deleteSizeHisto;
+ private LatencyHistogram pointLookupLatencyHisto;
+ private SizeHistogram pointLookupSizeHisto;
+ private LatencyHistogram rangeScanLatencyHisto;
+ private SizeHistogram rangeScanSizeHisto;
+
+ public TableHistograms(String tableName, Configuration conf) {
+ this.tableName = tableName;
+ queryLatencyHisto = new LatencyHistogram("QueryTime", "Query time latency", conf);
+ querySizeHisto = new SizeHistogram("QuerySize", "Query size", conf);
+
+ upsertLatencyHisto = new LatencyHistogram("UpsertTime", "Upsert time latency", conf);
+ upsertSizeHisto = new SizeHistogram("UpsertSize", "Upsert size", conf);
+
+ deleteLatencyHisto = new LatencyHistogram("DeleteTime", "Delete time latency", conf);
+ deleteSizeHisto = new SizeHistogram("DeleteSize", "Delete size", conf);
+
+ pointLookupLatencyHisto = new LatencyHistogram("PointLookupTime",
+ "Point Lookup Query time latency", conf);
+ pointLookupSizeHisto = new SizeHistogram("PointLookupSize",
+ "Point Lookup Query Size", conf);
+
+ rangeScanLatencyHisto = new LatencyHistogram("RangeScanTime",
+ "Range Scan Query time latency", conf);
+ rangeScanSizeHisto = new SizeHistogram("RangeScanSize",
+ "Range Scan Query size", conf);
+ }
+
+ public String getTableName() {
+ return tableName;
+ }
+
+ public LatencyHistogram getQueryLatencyHisto() {
+ return queryLatencyHisto;
+ }
+
+ public SizeHistogram getQuerySizeHisto() {
+ return querySizeHisto;
+ }
+
+
+ public LatencyHistogram getPointLookupLatencyHisto() {
+ return pointLookupLatencyHisto;
+ }
+
+ public SizeHistogram getPointLookupSizeHisto() {
+ return pointLookupSizeHisto;
+ }
+
+ public LatencyHistogram getRangeScanLatencyHisto() {
+ return rangeScanLatencyHisto;
+ }
+
+ public SizeHistogram getRangeScanSizeHisto() {
+ return rangeScanSizeHisto;
+ }
+
+ public LatencyHistogram getUpsertLatencyHisto() {
+ return upsertLatencyHisto;
+ }
+
+ public SizeHistogram getUpsertSizeHisto() {
+ return upsertSizeHisto;
+ }
+
+ public LatencyHistogram getDeleteLatencyHisto() {
+ return deleteLatencyHisto;
+ }
+
+ public SizeHistogram getDeleteSizeHisto() {
+ return deleteSizeHisto;
+ }
+
+ public List<HistogramDistribution> getTableLatencyHistogramsDistribution() {
+ List<HistogramDistribution> list = new ArrayList<>(Arrays.asList(queryLatencyHisto.getRangeHistogramDistribution(),
+ upsertLatencyHisto.getRangeHistogramDistribution(), deleteLatencyHisto.getRangeHistogramDistribution(),
+ pointLookupLatencyHisto.getRangeHistogramDistribution(), rangeScanLatencyHisto.getRangeHistogramDistribution()));
+ return Collections.unmodifiableList(list);
+ }
+
+ public List<HistogramDistribution> getTableSizeHistogramsDistribution() {
+ List<HistogramDistribution> list = new ArrayList<>(Arrays.asList(querySizeHisto.getRangeHistogramDistribution(),
Review comment:
Why are we using new and Arrays?
--
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: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
> Add Histograms for Table level Metrics.
> ----------------------------------------
>
> Key: PHOENIX-5838
> URL: https://issues.apache.org/jira/browse/PHOENIX-5838
> Project: Phoenix
> Issue Type: Sub-task
> Reporter: vikas meka
> Assignee: vikas meka
> Priority: Major
> Labels: metric-collector, metrics
>
--
This message was sent by Atlassian Jira
(v8.3.4#803005)