From d91e485f920c5f4ec6e95a56b3175c9cfbebcc2e Mon Sep 17 00:00:00 2001 From: vmeka2020 Date: Fri, 14 May 2021 00:59:16 -0700 Subject: [PATCH 01/11] PHOENIX-5838 Add Histograms for Table level Metrics --- phoenix-core/pom.xml | 4 + .../PhoenixTableLevelMetricsIT.java | 191 ++++++++++++++- .../phoenix/compile/UpsertCompiler.java | 2 +- .../apache/phoenix/execute/MutationState.java | 26 +- .../phoenix/jdbc/PhoenixConnection.java | 6 +- .../apache/phoenix/jdbc/PhoenixResultSet.java | 14 ++ .../apache/phoenix/jdbc/PhoenixStatement.java | 11 + .../util/PhoenixConfigurationUtil.java | 24 ++ .../monitoring/HistogramDistribution.java | 32 +++ .../monitoring/HistogramDistributionImpl.java | 73 ++++++ .../phoenix/monitoring/LatencyHistogram.java | 44 ++++ .../phoenix/monitoring/RangeHistogram.java | 113 +++++++++ .../phoenix/monitoring/SizeHistogram.java | 43 ++++ .../monitoring/TableClientMetrics.java | 9 +- .../phoenix/monitoring/TableHistograms.java | 119 ++++++++++ .../monitoring/TableMetricsManager.java | 176 +++++++++++++- .../apache/phoenix/query/QueryServices.java | 5 + .../monitoring/LatencyHistogramTest.java | 94 ++++++++ .../phoenix/monitoring/SizeHistogramTest.java | 79 ++++++ .../monitoring/TableClientMetricsTest.java | 5 +- .../monitoring/TableHistogramsTest.java | 46 ++++ .../monitoring/TableMetricsManagerTest.java | 224 ++++++++++++++++++ pom.xml | 6 + 23 files changed, 1338 insertions(+), 8 deletions(-) create mode 100644 phoenix-core/src/main/java/org/apache/phoenix/monitoring/HistogramDistribution.java create mode 100644 phoenix-core/src/main/java/org/apache/phoenix/monitoring/HistogramDistributionImpl.java create mode 100644 phoenix-core/src/main/java/org/apache/phoenix/monitoring/LatencyHistogram.java create mode 100644 phoenix-core/src/main/java/org/apache/phoenix/monitoring/RangeHistogram.java create mode 100644 phoenix-core/src/main/java/org/apache/phoenix/monitoring/SizeHistogram.java create mode 100644 phoenix-core/src/main/java/org/apache/phoenix/monitoring/TableHistograms.java create mode 100644 phoenix-core/src/test/java/org/apache/phoenix/monitoring/LatencyHistogramTest.java create mode 100644 phoenix-core/src/test/java/org/apache/phoenix/monitoring/SizeHistogramTest.java create mode 100644 phoenix-core/src/test/java/org/apache/phoenix/monitoring/TableHistogramsTest.java diff --git a/phoenix-core/pom.xml b/phoenix-core/pom.xml index 1498012cf3c..b2a8feb1939 100644 --- a/phoenix-core/pom.xml +++ b/phoenix-core/pom.xml @@ -507,6 +507,10 @@ org.jruby.jcodings jcodings + + org.hdrhistogram + HdrHistogram + diff --git a/phoenix-core/src/it/java/org/apache/phoenix/monitoring/PhoenixTableLevelMetricsIT.java b/phoenix-core/src/it/java/org/apache/phoenix/monitoring/PhoenixTableLevelMetricsIT.java index 13ec1c8b3c5..4d1dff41757 100644 --- a/phoenix-core/src/it/java/org/apache/phoenix/monitoring/PhoenixTableLevelMetricsIT.java +++ b/phoenix-core/src/it/java/org/apache/phoenix/monitoring/PhoenixTableLevelMetricsIT.java @@ -57,6 +57,9 @@ import static org.apache.phoenix.exception.SQLExceptionCode.OPERATION_TIMED_OUT; import static org.apache.phoenix.monitoring.MetricType.ATOMIC_UPSERT_COMMIT_TIME; import static org.apache.phoenix.monitoring.MetricType.ATOMIC_UPSERT_SQL_COUNTER; +import static org.apache.phoenix.monitoring.GlobalClientMetrics.GLOBAL_MUTATION_BYTES; +import static org.apache.phoenix.monitoring.GlobalClientMetrics.GLOBAL_QUERY_TIME; +import static org.apache.phoenix.monitoring.GlobalClientMetrics.GLOBAL_SCAN_BYTES; import static org.apache.phoenix.monitoring.MetricType.DELETE_AGGREGATE_FAILURE_SQL_COUNTER; import static org.apache.phoenix.monitoring.MetricType.DELETE_AGGREGATE_SUCCESS_SQL_COUNTER; import static org.apache.phoenix.monitoring.MetricType.DELETE_BATCH_FAILED_COUNTER; @@ -403,6 +406,50 @@ private static void assertMutationTableMetrics(final boolean isUpsert, final Str } } + private void assertHistogramMetricsForMutations(String tableName, boolean isUpsert, + long ltCount, long szCount, boolean verifyMetricValues) { + LatencyHistogram ltHisto; + SizeHistogram szHisto; + if (isUpsert) { + ltHisto = TableMetricsManager.getUpsertLatencyHistogramForTable(tableName); + szHisto = TableMetricsManager.getUpsertSizeHistogramForTable(tableName); + } else { + ltHisto = TableMetricsManager.getDeleteLatencyHistogramForTable(tableName); + szHisto = TableMetricsManager.getDeleteSizeHistogramForTable(tableName); + } + assertNotNull(ltHisto); + assertNotNull(szHisto); + assertEquals(ltCount, ltHisto.getHistogram().getTotalCount()); + assertEquals(szCount, szHisto.getHistogram().getTotalCount()); + + // If we are just comparing one data point then we can compare with table metrics + // or global metrics but if there are multiple data points then we can't compare histogram + // data points with global metrics. + if (verifyMetricValues) { + long sqlTime; + if (isUpsert) { + sqlTime = getMetricFromTableMetrics(tableName, MetricType.UPSERT_SQL_QUERY_TIME); + } else { + sqlTime = getMetricFromTableMetrics(tableName, MetricType.DELETE_SQL_QUERY_TIME); + + } + long commitTime = getMetricFromTableMetrics(tableName, MetricType.MUTATION_COMMIT_TIME); + // Latency metric for mutation is sum of time spent in executeMutation + // and PhoenixConnection#commit time. + long totalCommitTimeFromMetrics = sqlTime + commitTime; + + // Histogram#maxValue is the last value in the bucket. So we can't compare directly + // maxValue with totalCommitTimeFromMetrics. + Assert.assertTrue(ltHisto.getHistogram().valuesAreEquivalent(totalCommitTimeFromMetrics, + ltHisto.getHistogram().getMaxValue())); + + long mutationBytesFromGlobalMetrics = GLOBAL_MUTATION_BYTES.getMetric().getValue(); + Assert.assertTrue(szHisto.getHistogram().valuesAreEquivalent(mutationBytesFromGlobalMetrics, + szHisto.getHistogram().getMaxValue())); + } + + } + /** * Checks that if the metric is of the passed in type, it has the expected value * (based on the CompareOp). If the metric type is different than checkType, ignore @@ -1159,7 +1206,7 @@ private static void assertMetricValue(Metric m, MetricType checkType, long compa Connection conn = null; Throwable exception = null; int numAtomicUpserts = 4; - try { + try {x conn = getConnFromTestDriver(); String ddl = "create table " + tableName + "(pk varchar primary key, counter1 bigint)"; conn.createStatement().execute(ddl); @@ -1196,6 +1243,148 @@ private static void assertMetricValue(Metric m, MetricType checkType, long compa assertEquals(numAtomicUpserts, getMetricFromTableMetrics(tableName, ATOMIC_UPSERT_SQL_COUNTER)); assertTrue(getMetricFromTableMetrics(tableName, ATOMIC_UPSERT_COMMIT_TIME) > 0); } + + @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); + try (Connection conn = getConnFromTestDriver(); + Statement statement = conn.createStatement()) { + String select = "SELECT * FROM " + tableName; + ResultSet resultSet = statement.executeQuery(select); + while (resultSet.next()) { + // do nothing + } + resultSet.close(); + } // conn close will close the rs at which point we will increment the scan_bytes counter + + // Verify that value from histogram is equal to metric from global metrics. + LatencyHistogram ltHisto = TableMetricsManager.getQueryLatencyHistogramForTable(tableName); + SizeHistogram szHisto = TableMetricsManager.getQuerySizeHistogramForTable(tableName); + + assertHistogramMetricsForQueries(tableName, ltHisto, szHisto, 1, 1); + } + + @Test + public void testHistogramMetricsForRangeScan() 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 global metrics and table level metrics. + PhoenixMetricsIT.resetGlobalMetrics(); + PhoenixRuntime.clearTableLevelMetrics(); + try (Connection conn = getConnFromTestDriver(); + Statement statement = conn.createStatement()) { + String select = "SELECT * FROM " + tableName; + ResultSet resultSet = statement.executeQuery(select); + while (resultSet.next()) { + // do nothing + } + } // conn close will close the rs at which point we will increment the scan_bytes counter + + // Make sure that point lookup histograms are empty since this is a range scan query. + LatencyHistogram pointLookupLtHisto = + TableMetricsManager.getPointLookupLatencyHistogramForTable(tableName); + SizeHistogram pointLookupSzHisto = + TableMetricsManager.getPointLookupSizeHistogramForTable(tableName); + Assert.assertEquals(0, pointLookupLtHisto.getHistogram().getTotalCount()); + Assert.assertEquals(0, pointLookupSzHisto.getHistogram().getTotalCount()); + + LatencyHistogram ltHistogram = + TableMetricsManager.getRangeScanLatencyHistogramForTable(tableName); + Assert.assertEquals(1, ltHistogram.getHistogram().getTotalCount()); + SizeHistogram sizeHistogram = + TableMetricsManager.getRangeScanSizeHistogramForTable(tableName); + Assert.assertEquals(1, sizeHistogram.getHistogram().getTotalCount()); + + // Verify that value from histogram is equal to metric from global metrics. + assertHistogramMetricsForQueries(tableName, ltHistogram, sizeHistogram, 1, 1); + } + + // Verify that there is a histogram counter for the operation and verify with table level metrics + private void assertHistogramMetricsForQueries(String tableName, LatencyHistogram ltHistogram, + SizeHistogram sizeHistogram, int ltCount, int szCount) { + Assert.assertEquals(ltCount, ltHistogram.getHistogram().getTotalCount()); + Assert.assertEquals(szCount, sizeHistogram.getHistogram().getTotalCount()); + + // Get latency metrics from table level metrics + Long queryTime = GLOBAL_QUERY_TIME.getMetric().getValue(); + long rsNextTime = getMetricFromTableMetrics(tableName, MetricType.RESULT_SET_TIME_MS); + // Latency for queries is sum of time spent in executeQuery phase and rs.next phase. + long totalLatency = queryTime + rsNextTime; + long maxLtValue = ltHistogram.getHistogram().getMaxValue(); + Assert.assertTrue(ltHistogram.getHistogram().valuesAreEquivalent(totalLatency, maxLtValue)); + + Long scanBytes = GLOBAL_SCAN_BYTES.getMetric().getValue(); + long maxSzValue = sizeHistogram.getHistogram().getMaxValue(); + Assert.assertTrue(sizeHistogram.getHistogram().valuesAreEquivalent(scanBytes, maxSzValue)); +>>>>>>> PHOENIX-5838 Add Histograms for Table level Metrics } private Connection getConnFromTestDriver() throws SQLException { diff --git a/phoenix-core/src/main/java/org/apache/phoenix/compile/UpsertCompiler.java b/phoenix-core/src/main/java/org/apache/phoenix/compile/UpsertCompiler.java index a7015de7b6c..bbba71d9e57 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/compile/UpsertCompiler.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/compile/UpsertCompiler.java @@ -1038,7 +1038,7 @@ private static void throwIfNotUpdatable(TableRef tableRef, Set overlapV } } - private class ServerUpsertSelectMutationPlan implements MutationPlan { + public class ServerUpsertSelectMutationPlan implements MutationPlan { private final QueryPlan queryPlan; private final TableRef tableRef; private final QueryPlan originalQueryPlan; diff --git a/phoenix-core/src/main/java/org/apache/phoenix/execute/MutationState.java b/phoenix-core/src/main/java/org/apache/phoenix/execute/MutationState.java index e8513e7f8f6..d09326aab61 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/execute/MutationState.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/execute/MutationState.java @@ -38,6 +38,7 @@ import java.util.Arrays; import java.util.Collection; import java.util.Collections; +import java.util.HashMap; import java.util.Iterator; import java.util.LinkedHashMap; import java.util.List; @@ -164,6 +165,7 @@ public class MutationState implements SQLCloseable { private final MutationMetricQueue mutationMetricQueue; private ReadMetricQueue readMetricQueue; + private Map timeInExecuteMutationMap = new HashMap<>(); private static boolean allUpsertsMutations = true; private static boolean allDeletesMutations = true; @@ -1497,7 +1499,16 @@ public List getMutationList() { TableMetricsManager.updateMetricsMethod(htableNameStr, allUpsertsMutations ? UPSERT_AGGREGATE_FAILURE_SQL_COUNTER : DELETE_AGGREGATE_FAILURE_SQL_COUNTER, 1); } - + // Update size and latency histogram metrics. + TableMetricsManager.updateSizeHistogramMetricsForMutations(htableNameStr, + committedMutationsMetric.getMutationsSizeBytes().getValue(), allUpsertsMutations); + Long latency = timeInExecuteMutationMap.get(htableNameStr); + if (latency == null) { + latency = 0l; + } + latency += mutationCommitTime; + TableMetricsManager.updateLatencyHistogramForMutations(htableNameStr, + latency, allUpsertsMutations); } resetAllMutationState(); @@ -2176,4 +2187,17 @@ public MutationMetricQueue getMutationMetricQueue() { return mutationMetricQueue; } + public void addExecuteMutationTime(long time, String tableName) { + Long timeSpent = timeInExecuteMutationMap.get(tableName); + if (timeSpent == null) { + timeSpent = 0l; + } + timeSpent += time; + timeInExecuteMutationMap.put(tableName, timeSpent); + } + + public void resetExecuteMutationTimeMap() { + timeInExecuteMutationMap.clear(); + } + } diff --git a/phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixConnection.java b/phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixConnection.java index 7c3b8ccf02e..d74fe6db276 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixConnection.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixConnection.java @@ -730,7 +730,11 @@ public void commit() throws SQLException { @Override public Void call() throws SQLException { checkOpen(); - mutationState.commit(); + try { + mutationState.commit(); + } finally { + mutationState.resetExecuteMutationTimeMap(); + } return null; } }, Tracing.withTracing(this, "committing mutations")); diff --git a/phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixResultSet.java b/phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixResultSet.java index af39bdd56aa..cbce413ddc8 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixResultSet.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixResultSet.java @@ -909,6 +909,20 @@ private void updateTableLevelReadMetrics(String tableName, boolean isPointLookup metricsFromOverallQuery.put(tableName, overAllReadMetrics); TableMetricsManager.pushMetricsFromConnInstanceMethod(metricsFromOverallQuery); if (readMetrics.get(tableName) != null) { + Long scanBytes = readMetrics.get(tableName).get(MetricType.SCAN_BYTES); + if (scanBytes == null) { + scanBytes = 0L; + } + TableMetricsManager.updateHistogramMetricsForQueryScanBytes( + scanBytes, tableName, isPointLookup); + Long timeSpentInRSNext = overAllReadMetrics.get(MetricType.RESULT_SET_TIME_MS); + + if (timeSpentInRSNext == null) { + timeSpentInRSNext = 0l; + } + timeSpentInRSNext += queryTime; + TableMetricsManager.updateHistogramMetricsForQueryLatency(tableName, timeSpentInRSNext, isPointLookup); + TableMetricsManager.updateMetricsMethod(tableName, this.exception == null ? MetricType.SELECT_AGGREGATE_SUCCESS_SQL_COUNTER : MetricType.SELECT_AGGREGATE_FAILURE_SQL_COUNTER, 1); diff --git a/phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixStatement.java b/phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixStatement.java index 11512f80d94..05c82f08f56 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixStatement.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixStatement.java @@ -620,6 +620,17 @@ public Integer call() throws SQLException { TableMetricsManager.updateMetricsMethod(tableName, isUpsert ? UPSERT_AGGREGATE_FAILURE_SQL_COUNTER: DELETE_AGGREGATE_FAILURE_SQL_COUNTER, 1); } + if (plan instanceof DeleteCompiler.ServerSelectDeleteMutationPlan + || plan instanceof UpsertCompiler.ServerUpsertSelectMutationPlan) { + TableMetricsManager.updateLatencyHistogramForMutations( + tableName, executeMutationTimeSpent, false); + // We won't have size histograms for delete mutations when auto commit is set to true and + // if plan is of ServerSelectDeleteMutationPlan or ServerUpsertSelectMutationPlan + // since the update happens on server. + } else { + state.addExecuteMutationTime( + executeMutationTimeSpent, tableName); + } } } diff --git a/phoenix-core/src/main/java/org/apache/phoenix/mapreduce/util/PhoenixConfigurationUtil.java b/phoenix-core/src/main/java/org/apache/phoenix/mapreduce/util/PhoenixConfigurationUtil.java index f17510e68dd..7ab7c25a0c6 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/mapreduce/util/PhoenixConfigurationUtil.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/mapreduce/util/PhoenixConfigurationUtil.java @@ -904,4 +904,28 @@ public static boolean isMRSnapshotManagedExternally(final Configuration configur return isSnapshotRestoreManagedExternally; } + /** + * Get the value of the name property as a set of comma-delimited + * long values. + * If no such property exists, null is returned. + * Hadoop Configuration object has support for getting ints delimited by comma + * but doesn't support for long. + * @param name property name + * @return property value interpreted as an array of comma-delimited + * long values + */ + public static long[] getLongs(Configuration conf, String name) { + String[] strings = conf.getTrimmedStrings(name); + // Configuration#getTrimmedStrings will never return null. + // If key is not found, it will return empty array. + if (strings.length == 0) { + return null; + } + long[] longs = new long[strings.length]; + for (int i = 0; i < strings.length; i++) { + longs[i] = Long.parseLong(strings[i]); + } + return longs; + } + } diff --git a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/HistogramDistribution.java b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/HistogramDistribution.java new file mode 100644 index 00000000000..7c28b45a975 --- /dev/null +++ b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/HistogramDistribution.java @@ -0,0 +1,32 @@ +/* + * 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 interface HistogramDistribution { + public long getMin(); + + public long getMax(); + + public long getCount(); + + public String getHistoName(); + + public Map getRangeDistributionMap(); +} diff --git a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/HistogramDistributionImpl.java b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/HistogramDistributionImpl.java new file mode 100644 index 00000000000..eb8b63c8456 --- /dev/null +++ b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/HistogramDistributionImpl.java @@ -0,0 +1,73 @@ +/* + * 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 rangeDistribution; + + public HistogramDistributionImpl(String histoName) { + this.histoName = histoName; + } + + public void setMin(long min) { + this.min = min; + } + + public void setMax(long max) { + this.max = max; + } + + public void setCount(long count) { + this.count = count; + } + + public void setRangeDistributionMap(Map distributionMap) { + this.rangeDistribution = distributionMap; + } + + @Override + public long getMin() { + return min; + } + + @Override + public long getMax() { + return max; + } + + @Override + public long getCount() { + return count; + } + + @Override + public String getHistoName() { + return histoName; + } + + @Override + public Map getRangeDistributionMap() { + return rangeDistribution; + } +} diff --git a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/LatencyHistogram.java b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/LatencyHistogram.java new file mode 100644 index 00000000000..144372e43f8 --- /dev/null +++ b/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; + } +} \ No newline at end of file diff --git a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/RangeHistogram.java b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/RangeHistogram.java new file mode 100644 index 00000000000..e935726b325 --- /dev/null +++ b/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 java.util.HashMap; +import java.util.Map; +import org.HdrHistogram.ConcurrentHistogram; +import org.HdrHistogram.Histogram; +import org.apache.hbase.thirdparty.com.google.common.base.Preconditions; +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 generateDistributionMap(Histogram snapshot) { + long priorRange = 0; + Map 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; + } +} \ No newline at end of file diff --git a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/SizeHistogram.java b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/SizeHistogram.java new file mode 100644 index 00000000000..2af4ea31551 --- /dev/null +++ b/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; + } +} \ No newline at end of file diff --git a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/TableClientMetrics.java b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/TableClientMetrics.java index 21e64f3c62d..6bf619b6711 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/TableClientMetrics.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/TableClientMetrics.java @@ -16,6 +16,8 @@ package org.apache.phoenix.monitoring; +import org.apache.hadoop.conf.Configuration; + import java.util.ArrayList; import java.util.HashMap; import java.util.List; @@ -139,14 +141,16 @@ public enum TableMetrics { private final String tableName; private Map metricRegister; + private TableHistograms tableHistograms; - public TableClientMetrics(final String tableName) { + public TableClientMetrics(final String tableName, Configuration conf) { this.tableName = tableName; metricRegister = new HashMap<>(); for (TableMetrics tableMetric : TableMetrics.values()) { tableMetric.metric = new PhoenixTableMetricImpl(tableMetric.metricType); metricRegister.put(tableMetric.metricType, tableMetric.metric); } + tableHistograms = new TableHistograms(tableName, conf); } /** @@ -185,4 +189,7 @@ public Map getMetricRegistry() { return metricRegister; } + public TableHistograms getTableHistograms() { + return tableHistograms; + } } \ No newline at end of file diff --git a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/TableHistograms.java b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/TableHistograms.java new file mode 100644 index 00000000000..1de0030d3de --- /dev/null +++ b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/TableHistograms.java @@ -0,0 +1,119 @@ +/* + * 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 getTableLatencyHistogramsDistribution() { + List list = new ArrayList<>(Arrays.asList(queryLatencyHisto.getRangeHistogramDistribution(), + upsertLatencyHisto.getRangeHistogramDistribution(), deleteLatencyHisto.getRangeHistogramDistribution(), + pointLookupLatencyHisto.getRangeHistogramDistribution(), rangeScanLatencyHisto.getRangeHistogramDistribution())); + return Collections.unmodifiableList(list); + } + + public List getTableSizeHistogramsDistribution() { + List list = new ArrayList<>(Arrays.asList(querySizeHisto.getRangeHistogramDistribution(), + upsertSizeHisto.getRangeHistogramDistribution(), deleteSizeHisto.getRangeHistogramDistribution(), + pointLookupSizeHisto.getRangeHistogramDistribution(), rangeScanSizeHisto.getRangeHistogramDistribution())); + return Collections.unmodifiableList(list); + } +} \ No newline at end of file diff --git a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/TableMetricsManager.java b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/TableMetricsManager.java index f3c626b827b..2c8d8c577de 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/TableMetricsManager.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/TableMetricsManager.java @@ -249,7 +249,7 @@ private TableClientMetrics getTableClientMetrics(String tableName) { if (tInstance == null) { LOGGER.info(String.format("Phoenix Table metrics creating object for table: %s", tableName)); - tInstance = new TableClientMetrics(tableName); + tInstance = new TableClientMetrics(tableName, options.getConfiguration()); if (isMetricPublisherEnabled && mPublisher != null) { mPublisher.registerMetrics(tInstance); } @@ -291,4 +291,178 @@ public void clearTableLevelMetrics() { public void clear() { TableMetricsManager.clearTableLevelMetricsMethod(); } + + public static Map> getSizeHistogramsForAllTables() { + Map> map = new HashMap<>(); + for (Map.Entry entry: tableClientMetricsMapping.entrySet()) { + TableHistograms tableHistograms = entry.getValue().getTableHistograms(); + map.put(entry.getKey(), tableHistograms.getTableSizeHistogramsDistribution()); + } + return map; + } + + public static Map> getLatencyHistogramsForAllTables() { + Map> map = new HashMap<>(); + for (Map.Entry entry: tableClientMetricsMapping.entrySet()) { + TableHistograms tableHistograms = entry.getValue().getTableHistograms(); + map.put(entry.getKey(), tableHistograms.getTableLatencyHistogramsDistribution()); + } + return map; + } + + public static LatencyHistogram getUpsertLatencyHistogramForTable(String tableName) { + TableClientMetrics metrics = getInstance().getTableClientMetrics(tableName); + if (metrics == null) { + LOGGER.trace("Table level client metrics are disabled for table: " + tableName); + return null; + } + return metrics.getTableHistograms().getUpsertLatencyHisto(); + } + + public static SizeHistogram getUpsertSizeHistogramForTable(String tableName) { + TableClientMetrics metrics = getInstance().getTableClientMetrics(tableName); + if (metrics == null) { + LOGGER.trace("Table level client metrics are disabled for table: " + tableName); + return null; + } + return metrics.getTableHistograms().getUpsertSizeHisto(); + } + + public static LatencyHistogram getDeleteLatencyHistogramForTable(String tableName) { + TableClientMetrics metrics = getInstance().getTableClientMetrics(tableName); + if (metrics == null) { + LOGGER.trace("Table level client metrics are disabled for table: " + tableName); + return null; + } + return metrics.getTableHistograms().getDeleteLatencyHisto(); + } + + public static SizeHistogram getDeleteSizeHistogramForTable(String tableName) { + TableClientMetrics metrics = getInstance().getTableClientMetrics(tableName); + if (metrics == null) { + LOGGER.trace("Table level client metrics are disabled for table: " + tableName); + return null; + } + return metrics.getTableHistograms().getDeleteSizeHisto(); + } + + public static LatencyHistogram getQueryLatencyHistogramForTable(String tableName) { + TableClientMetrics metrics = getInstance().getTableClientMetrics(tableName); + if (metrics == null) { + LOGGER.trace("Table level client metrics are disabled for table: " + tableName); + return null; + } + return metrics.getTableHistograms().getQueryLatencyHisto(); + } + + public static SizeHistogram getQuerySizeHistogramForTable(String tableName) { + TableClientMetrics metrics = getInstance().getTableClientMetrics(tableName); + if (metrics == null) { + LOGGER.trace("Table level client metrics are disabled for table: " + tableName); + return null; + } + return metrics.getTableHistograms().getQuerySizeHisto(); + } + + public static LatencyHistogram getPointLookupLatencyHistogramForTable(String tableName) { + TableClientMetrics metrics = getInstance().getTableClientMetrics(tableName); + if (metrics == null) { + LOGGER.trace("Table level client metrics are disabled for table: " + tableName); + return null; + } + return metrics.getTableHistograms().getPointLookupLatencyHisto(); + } + + public static SizeHistogram getPointLookupSizeHistogramForTable(String tableName) { + TableClientMetrics metrics = getInstance().getTableClientMetrics(tableName); + if (metrics == null) { + LOGGER.trace("Table level client metrics are disabled for table: " + tableName); + return null; + } + return metrics.getTableHistograms().getPointLookupSizeHisto(); + } + + public static LatencyHistogram getRangeScanLatencyHistogramForTable(String tableName) { + TableClientMetrics metrics = getInstance().getTableClientMetrics(tableName); + if (metrics == null) { + LOGGER.trace("Table level client metrics are disabled for table: " + tableName); + return null; + } + return metrics.getTableHistograms().getRangeScanLatencyHisto(); + } + + public static SizeHistogram getRangeScanSizeHistogramForTable(String tableName) { + TableClientMetrics metrics = getInstance().getTableClientMetrics(tableName); + if (metrics == null) { + LOGGER.trace("Table level client metrics are disabled for table: " + tableName); + return null; + } + return metrics.getTableHistograms().getRangeScanSizeHisto(); + } + + public static void updateHistogramMetricsForQueryLatency(String tableName, long elapsedTime, + boolean isPointLookup) { + TableClientMetrics tableMetrics = getInstance().getTableClientMetrics(tableName); + if (tableMetrics == null) { + LOGGER.trace("Table level client metrics are disabled for table: " + tableName); + return; + } + LOGGER.trace("Updating latency histograms for select query: tableName: " + tableName + + " isPointLookup: " + isPointLookup + " elapsedTime: " + elapsedTime); + tableMetrics.getTableHistograms().getQueryLatencyHisto().add(elapsedTime); + if (isPointLookup) { + tableMetrics.getTableHistograms().getPointLookupLatencyHisto().add(elapsedTime); + } else { + tableMetrics.getTableHistograms().getRangeScanLatencyHisto().add(elapsedTime); + } + } + + public static void updateHistogramMetricsForQueryScanBytes(long scanBytes, + String tableName, boolean isPointLookup) { + TableClientMetrics tableMetrics = getInstance().getTableClientMetrics(tableName); + if (tableMetrics == null) { + LOGGER.trace("Table level client metrics are disabled for table: " + tableName); + return; + } + tableMetrics.getTableHistograms().getQuerySizeHisto().add(scanBytes); + if (isPointLookup) { + tableMetrics.getTableHistograms().getPointLookupSizeHisto().add(scanBytes); + } else { + tableMetrics.getTableHistograms().getRangeScanSizeHisto().add(scanBytes); + } + } + + public static void updateSizeHistogramMetricsForMutations(String tableName, long mutationBytes, + boolean isUpsert) { + TableClientMetrics tableMetrics = getInstance().getTableClientMetrics(tableName); + if (tableMetrics == null) { + LOGGER.trace("Table level client metrics are disabled for table: " + tableName); + return; + } + + LOGGER.trace("Updating size histograms for mutations: tableName: " + tableName + + " isUpsert: " + isUpsert + " mutation bytes: " + mutationBytes); + + if (isUpsert) { + tableMetrics.getTableHistograms().getUpsertSizeHisto().add(mutationBytes); + } else { + tableMetrics.getTableHistograms().getDeleteSizeHisto().add(mutationBytes); + } + } + + public static void updateLatencyHistogramForMutations(String tableName, long elapsedTime, + boolean isUpsert) { + TableClientMetrics tableMetrics = getInstance().getTableClientMetrics(tableName); + if (tableMetrics == null) { + LOGGER.trace("Table level client metrics are disabled for table: " + tableName); + return; + } + LOGGER.trace("Updating latency histograms for mutations: tableName: " + tableName + + " isUpsert: " + isUpsert + " elapsedTime: " + elapsedTime); + if (isUpsert) { + tableMetrics.getTableHistograms().getUpsertLatencyHisto().add(elapsedTime); + } else { + tableMetrics.getTableHistograms().getDeleteLatencyHisto().add(elapsedTime); + } + } } diff --git a/phoenix-core/src/main/java/org/apache/phoenix/query/QueryServices.java b/phoenix-core/src/main/java/org/apache/phoenix/query/QueryServices.java index f97e5e0e75e..8b93079f073 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/query/QueryServices.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/query/QueryServices.java @@ -372,6 +372,11 @@ public interface QueryServices extends SQLCloseable { public static final String PENDING_MUTATIONS_DDL_THROW_ATTRIB = "phoenix.pending.mutations.before.ddl.throw"; + // The range of bins for latency metrics for histogram. + public static final String PHOENIX_HISTOGRAM_LATENCY_RANGES = "phoenix.histogram.latency.ranges"; + // The range of bins for size metrics for histogram. + public static final String PHOENIX_HISTOGRAM_SIZE_RANGES = "phoenix.histogram.size.ranges"; + /** * Parameter to indicate the source of operation attribute. * It can include metadata about the customer, service, etc. diff --git a/phoenix-core/src/test/java/org/apache/phoenix/monitoring/LatencyHistogramTest.java b/phoenix-core/src/test/java/org/apache/phoenix/monitoring/LatencyHistogramTest.java new file mode 100644 index 00000000000..e49d5c1a528 --- /dev/null +++ b/phoenix-core/src/test/java/org/apache/phoenix/monitoring/LatencyHistogramTest.java @@ -0,0 +1,94 @@ +/* + * 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.HashMap; +import java.util.Map; +import org.apache.hadoop.conf.Configuration; +import org.apache.phoenix.query.QueryServices; +import org.junit.Assert; +import org.junit.Test; + +/** + Test for {@link LatencyHistogram} + **/ +public class LatencyHistogramTest { + + @Test + public void testLatencyHistogramRangeOverride() { + String histoName = "PhoenixGetLatencyHisto"; + Configuration conf = new Configuration(); + conf.set(QueryServices.PHOENIX_HISTOGRAM_LATENCY_RANGES, "2, 5, 8"); + LatencyHistogram histogram = new LatencyHistogram(histoName, + "histogram for GET operation latency", conf); + Assert.assertEquals(histoName, histogram.getName()); + long[] ranges = histogram.getRanges(); + Assert.assertNotNull(ranges); + Assert.assertEquals(3, ranges.length); + Assert.assertEquals(2, ranges[0]); + Assert.assertEquals(5, ranges[1]); + Assert.assertEquals(8, ranges[2]); + } + + @Test + public void testEveryRangeInDefaultRange() { + //1, 3, 10, 30, 100, 300, 1000, 3000, 10000, 30000, 60000, 120000, 300000, 600000 + Configuration conf = new Configuration(); + String histoName = "PhoenixGetLatencyHisto"; + conf.unset(QueryServices.PHOENIX_HISTOGRAM_LATENCY_RANGES); + LatencyHistogram histogram = new LatencyHistogram(histoName, + "histogram for GET operation latency", conf); + Assert.assertEquals(histoName, histogram.getName()); + Assert.assertEquals(LatencyHistogram.DEFAULT_RANGE, histogram.getRanges()); + + histogram.add(1); + histogram.add(2); + histogram.add(3); + histogram.add(5); + histogram.add(20); + histogram.add(60); + histogram.add(200); + histogram.add(600); + histogram.add(2000); + histogram.add(6000); + histogram.add(20000); + histogram.add(45000); + histogram.add(90000); + histogram.add(200000); + histogram.add(450000); + histogram.add(900000); + + Map distribution = histogram.getRangeHistogramDistribution().getRangeDistributionMap(); + Map expectedMap = new HashMap<>(); + expectedMap.put("0,1", 1l); + expectedMap.put("1,3", 2l); + expectedMap.put("3,10", 1l); + expectedMap.put("10,30", 1l); + expectedMap.put("30,100", 1l); + expectedMap.put("100,300", 1l); + expectedMap.put("300,1000", 1l); + expectedMap.put("1000,3000", 1l); + expectedMap.put("3000,10000", 1l); + expectedMap.put("10000,30000", 1l); + expectedMap.put("30000,60000", 1l); + expectedMap.put("60000,120000", 1l); + expectedMap.put("120000,300000", 1l); + expectedMap.put("300000,600000", 1l); + Assert.assertEquals(expectedMap, distribution); + } +} \ No newline at end of file diff --git a/phoenix-core/src/test/java/org/apache/phoenix/monitoring/SizeHistogramTest.java b/phoenix-core/src/test/java/org/apache/phoenix/monitoring/SizeHistogramTest.java new file mode 100644 index 00000000000..c623bcabc23 --- /dev/null +++ b/phoenix-core/src/test/java/org/apache/phoenix/monitoring/SizeHistogramTest.java @@ -0,0 +1,79 @@ +/* + * 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.HashMap; +import java.util.Map; +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.metrics2.lib.MutableSizeHistogram; +import org.apache.phoenix.query.QueryServices; +import org.junit.Assert; +import org.junit.Test; + +/** + Test for {@link SizeHistogram} + **/ +public class SizeHistogramTest { + + @Test + public void testSizeHistogramRangeOverride() { + Configuration conf = new Configuration(); + conf.set(QueryServices.PHOENIX_HISTOGRAM_SIZE_RANGES, "1, 100, 1000"); + SizeHistogram histogram = new SizeHistogram("PhoenixReadBytesHisto", + "histogram for read bytes", conf); + long[] ranges = histogram.getRanges(); + Assert.assertNotNull(ranges); + Assert.assertEquals(3, ranges.length); + Assert.assertEquals(1, ranges[0]); + Assert.assertEquals(100, ranges[1]); + Assert.assertEquals(1000, ranges[2]); + } + + @Test + public void testEveryRangeInDefaultRange() { + // {10,100,1000,10000,100000,1000000,10000000,100000000}; + Configuration conf = new Configuration(); + String histoName = "PhoenixReadBytesHisto"; + conf.unset(QueryServices.PHOENIX_HISTOGRAM_SIZE_RANGES); + SizeHistogram histogram = new SizeHistogram(histoName, + "histogram for read bytes", conf); + Assert.assertEquals(histoName, histogram.getName()); + Assert.assertEquals(SizeHistogram.DEFAULT_RANGE, histogram.getRanges()); + + histogram.add(5); + histogram.add(50); + histogram.add(500); + histogram.add(5000); + histogram.add(50000); + histogram.add(500000); + histogram.add(5000000); + histogram.add(50000000); + Map + distribution = histogram.getRangeHistogramDistribution().getRangeDistributionMap(); + Map expectedMap = new HashMap<>(); + expectedMap.put("0,10", 1l); + expectedMap.put("10,100", 1l); + expectedMap.put("100,1000", 1l); + expectedMap.put("1000,10000", 1l); + expectedMap.put("10000,100000", 1l); + expectedMap.put("100000,1000000", 1l); + expectedMap.put("1000000,10000000", 1l); + expectedMap.put("10000000,100000000", 1l); + Assert.assertEquals(expectedMap, distribution); + } +} \ No newline at end of file diff --git a/phoenix-core/src/test/java/org/apache/phoenix/monitoring/TableClientMetricsTest.java b/phoenix-core/src/test/java/org/apache/phoenix/monitoring/TableClientMetricsTest.java index 23f5db5f1a6..3bbafa9fe1c 100644 --- a/phoenix-core/src/test/java/org/apache/phoenix/monitoring/TableClientMetricsTest.java +++ b/phoenix-core/src/test/java/org/apache/phoenix/monitoring/TableClientMetricsTest.java @@ -159,8 +159,9 @@ public boolean verifyTableName() { */ @Test public void testTableClientMetrics() { + Configuration conf = new Configuration(); for (int i = 0; i < tableNames.length; i++) { - TableClientMetrics tableClientMetrics = new TableClientMetrics(tableNames[i]); + TableClientMetrics tableClientMetrics = new TableClientMetrics(tableNames[i], conf); tableMetricsSet.put(tableNames[i], tableClientMetrics); tableClientMetrics.changeMetricValue(MUTATION_BATCH_SIZE, @@ -208,7 +209,7 @@ public void testTableClientMetrics() { public void testTableClientMetricsforTableName() { Configuration conf = new Configuration(); for (int i = 0; i < tableNames.length; i++) { - TableClientMetrics tableClientMetrics = new TableClientMetrics(tableNames[i]); + TableClientMetrics tableClientMetrics = new TableClientMetrics(tableNames[i], conf); tableMetricsSet.put(tableNames[i], tableClientMetrics); } assertTrue(verifyTableName()); diff --git a/phoenix-core/src/test/java/org/apache/phoenix/monitoring/TableHistogramsTest.java b/phoenix-core/src/test/java/org/apache/phoenix/monitoring/TableHistogramsTest.java new file mode 100644 index 00000000000..fb4d740e64d --- /dev/null +++ b/phoenix-core/src/test/java/org/apache/phoenix/monitoring/TableHistogramsTest.java @@ -0,0 +1,46 @@ +/* + * 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.junit.Assert; +import org.junit.Test; + +public class TableHistogramsTest { + + @Test + public void testTableHistograms() { + String table = "TEST_TABLE"; + Configuration conf = new Configuration(); + TableHistograms tableHistograms = new TableHistograms(table, conf); + Assert.assertEquals(table, tableHistograms.getTableName()); + Assert.assertNotNull(tableHistograms.getUpsertLatencyHisto()); + Assert.assertNotNull(tableHistograms.getUpsertSizeHisto()); + Assert.assertNotNull(tableHistograms.getDeleteLatencyHisto()); + Assert.assertNotNull(tableHistograms.getDeleteSizeHisto()); + Assert.assertNotNull(tableHistograms.getQueryLatencyHisto()); + Assert.assertNotNull(tableHistograms.getQuerySizeHisto()); + Assert.assertNotNull(tableHistograms.getPointLookupLatencyHisto()); + Assert.assertNotNull(tableHistograms.getPointLookupSizeHisto()); + Assert.assertNotNull(tableHistograms.getRangeScanLatencyHisto()); + Assert.assertNotNull(tableHistograms.getRangeScanSizeHisto()); + + Assert.assertEquals(5, tableHistograms.getTableLatencyHistogramsDistribution().size()); + Assert.assertEquals(5, tableHistograms.getTableSizeHistogramsDistribution().size()); + } +} \ No newline at end of file diff --git a/phoenix-core/src/test/java/org/apache/phoenix/monitoring/TableMetricsManagerTest.java b/phoenix-core/src/test/java/org/apache/phoenix/monitoring/TableMetricsManagerTest.java index 8b2b18bf834..533b044c42a 100644 --- a/phoenix-core/src/test/java/org/apache/phoenix/monitoring/TableMetricsManagerTest.java +++ b/phoenix-core/src/test/java/org/apache/phoenix/monitoring/TableMetricsManagerTest.java @@ -18,9 +18,13 @@ package org.apache.phoenix.monitoring; +import org.apache.hadoop.conf.Configuration; import org.apache.hbase.thirdparty.com.google.common.collect.Lists; +import org.apache.phoenix.query.QueryServices; import org.apache.phoenix.query.QueryServicesOptions; +import org.junit.Assert; import org.junit.Test; +import org.mockito.Mockito; import java.util.List; import java.util.Map; @@ -204,4 +208,224 @@ public void testTableMetricsForPushMetricsFromConnInstanceMethodWithAllowedTable assertFalse(verifyTableNamesExists(tableNames[2])); } + /* + Tests histogram metrics for upsert mutations. + */ + @Test + public void testHistogramMetricsForUpsertMutations() { + String tableName = "TEST-TABLE"; + Configuration conf = new Configuration(); + conf.set(QueryServices.PHOENIX_HISTOGRAM_LATENCY_RANGES, "2,5,8"); + conf.set(QueryServices.PHOENIX_HISTOGRAM_SIZE_RANGES, "10, 100, 1000"); + + QueryServicesOptions mockOptions = Mockito.mock(QueryServicesOptions.class); + Mockito.doReturn(true).when(mockOptions).isTableLevelMetricsEnabled(); + Mockito.doReturn(tableName).when(mockOptions).getAllowedListTableNames(); + Mockito.doReturn(conf).when(mockOptions).getConfiguration(); + TableMetricsManager tableMetricsManager = new TableMetricsManager(mockOptions); + TableMetricsManager.setInstance(tableMetricsManager); + + TableMetricsManager.updateLatencyHistogramForMutations(tableName, 1, true); + MutationMetricQueue.MutationMetric metric = new MutationMetricQueue.MutationMetric( + 0L, 5L, 0L, 0L, 0L, + 0L, 1L, 0L, 5L, 0L, 0L, 0L, 0L, 0L); + TableMetricsManager.updateSizeHistogramMetricsForMutations(tableName, metric.getMutationsSizeBytes().getValue(), true); + + TableMetricsManager.updateLatencyHistogramForMutations(tableName, 2, true); + metric = new MutationMetricQueue.MutationMetric(0L, 10L, 0L, 0L, 0L, + 0L, 1L, 0L, 10L, 0L, 0L, 0L, 0L, 0L); + TableMetricsManager.updateSizeHistogramMetricsForMutations(tableName, metric.getMutationsSizeBytes().getValue(), true); + + TableMetricsManager.updateLatencyHistogramForMutations(tableName, 4, true); + metric = new MutationMetricQueue.MutationMetric(0L, 50L, 0L, 0L, 0L, + 0L, 1L, 0L, 50L, 0L, 0L, 0L, 0L, 0L); + TableMetricsManager.updateSizeHistogramMetricsForMutations(tableName, metric.getMutationsSizeBytes().getValue(), true); + + TableMetricsManager.updateLatencyHistogramForMutations(tableName, 5, true); + metric = new MutationMetricQueue.MutationMetric(0L, 100L, 0L, 0L, 0L, + 0L, 1L, 0L, 100L, 0L, 0L, 0L, 0L, 0L); + TableMetricsManager.updateSizeHistogramMetricsForMutations(tableName, metric.getMutationsSizeBytes().getValue(), true); + + TableMetricsManager.updateLatencyHistogramForMutations(tableName, 6, true); + metric = new MutationMetricQueue.MutationMetric(0L, 500L, 0L, 0L, 0L, + 0L, 1L, 0L, 500L, 0L, 0L, 0L, 0L, 0L); + TableMetricsManager.updateSizeHistogramMetricsForMutations(tableName, metric.getMutationsSizeBytes().getValue(), true); + + TableMetricsManager.updateLatencyHistogramForMutations(tableName, 8, true); + metric = new MutationMetricQueue.MutationMetric(0L, 1000L, 0L, 0L, 0L, + 0L, 1L, 0L, 1000L, 0L, 0L, 0L, 0L, 0L); + TableMetricsManager.updateSizeHistogramMetricsForMutations(tableName, metric.getMutationsSizeBytes().getValue(), true); + + // Generate distribution map from histogram snapshots. + LatencyHistogram latencyHistogram = + TableMetricsManager.getUpsertLatencyHistogramForTable(tableName); + SizeHistogram sizeHistogram = TableMetricsManager.getUpsertSizeHistogramForTable(tableName); + + Map latencyMap = latencyHistogram.getRangeHistogramDistribution().getRangeDistributionMap(); + Map sizeMap = sizeHistogram.getRangeHistogramDistribution().getRangeDistributionMap(); + for (Long count: latencyMap.values()) { + Assert.assertEquals(new Long(2), count); + } + for (Long count: sizeMap.values()) { + Assert.assertEquals(new Long(2), count); + } + } + + /* + Tests histogram metrics for delete mutations. + */ + @Test + public void testHistogramMetricsForDeleteMutations() { + String tableName = "TEST-TABLE"; + Configuration conf = new Configuration(); + conf.set(QueryServices.PHOENIX_HISTOGRAM_LATENCY_RANGES, "2,5,8"); + conf.set(QueryServices.PHOENIX_HISTOGRAM_SIZE_RANGES, "10, 100, 1000"); + + QueryServicesOptions mockOptions = Mockito.mock(QueryServicesOptions.class); + Mockito.doReturn(true).when(mockOptions).isTableLevelMetricsEnabled(); + Mockito.doReturn(tableName).when(mockOptions).getAllowedListTableNames(); + Mockito.doReturn(conf).when(mockOptions).getConfiguration(); + TableMetricsManager tableMetricsManager = new TableMetricsManager(mockOptions); + TableMetricsManager.setInstance(tableMetricsManager); + + TableMetricsManager.updateLatencyHistogramForMutations(tableName, 1, false); + MutationMetricQueue.MutationMetric metric = new MutationMetricQueue.MutationMetric( + 0L, 0L, 5L, 0L, 0L, + 0L, 0L, 1L, 5L, 0L, 0L, 0L, 0L, 0L); + TableMetricsManager.updateSizeHistogramMetricsForMutations(tableName, metric.getMutationsSizeBytes().getValue(), false); + + TableMetricsManager.updateLatencyHistogramForMutations(tableName, 2, false); + metric = new MutationMetricQueue.MutationMetric(0L, 0L, 10L, 0L, 0L, + 0L, 0L, 1L, 10L, 0L, 0L, 0L, 0L, 0L); + TableMetricsManager.updateSizeHistogramMetricsForMutations(tableName, metric.getMutationsSizeBytes().getValue(), false); + + TableMetricsManager.updateLatencyHistogramForMutations(tableName, 4, false); + metric = new MutationMetricQueue.MutationMetric(0L, 0L, 50L, 0L, 0L, + 0L, 0L, 1L, 50L, 0L, 0L, 0L, 0L, 0L); + TableMetricsManager.updateSizeHistogramMetricsForMutations(tableName, metric.getMutationsSizeBytes().getValue(), false); + + TableMetricsManager.updateLatencyHistogramForMutations(tableName, 5,false); + metric = new MutationMetricQueue.MutationMetric(0L, 0L, 100L, 0L, 0L, + 0L, 0L, 1L, 100L, 0L, 0L, 0L, 0L, 0L); + TableMetricsManager.updateSizeHistogramMetricsForMutations(tableName, metric.getMutationsSizeBytes().getValue(), false); + + TableMetricsManager.updateLatencyHistogramForMutations(tableName, 6,false); + metric = new MutationMetricQueue.MutationMetric(0L, 0L, 500L, 0L, 0L, + 0L, 0L, 1L, 500L, 0L, 0L, 0L, 0L, 0L); + TableMetricsManager.updateSizeHistogramMetricsForMutations(tableName, metric.getMutationsSizeBytes().getValue(), false); + + TableMetricsManager.updateLatencyHistogramForMutations(tableName, 8, false); + metric = new MutationMetricQueue.MutationMetric(0L, 0L, 1000L, 0L, 0L, + 0L, 0L, 1L, 1000L, 0L, 0L, 0L, 0L, 0L); + TableMetricsManager.updateSizeHistogramMetricsForMutations(tableName, metric.getMutationsSizeBytes().getValue(), false); + + // Generate distribution map from histogram snapshots. + LatencyHistogram latencyHistogram = + TableMetricsManager.getDeleteLatencyHistogramForTable(tableName); + SizeHistogram sizeHistogram = TableMetricsManager.getDeleteSizeHistogramForTable(tableName); + + Map latencyMap = latencyHistogram.getRangeHistogramDistribution().getRangeDistributionMap(); + Map sizeMap = sizeHistogram.getRangeHistogramDistribution().getRangeDistributionMap(); + for (Long count: latencyMap.values()) { + Assert.assertEquals(new Long(2), count); + } + for (Long count: sizeMap.values()) { + Assert.assertEquals(new Long(2), count); + } + } + + /* + Tests histogram metrics for select query, point lookup query and range scan query. + */ + @Test + public void testHistogramMetricsForQuery() { + String tableName = "TEST-TABLE"; + Configuration conf = new Configuration(); + conf.set(QueryServices.PHOENIX_HISTOGRAM_LATENCY_RANGES, "2,5,8"); + conf.set(QueryServices.PHOENIX_HISTOGRAM_SIZE_RANGES, "10, 100, 1000"); + + QueryServicesOptions mockOptions = Mockito.mock(QueryServicesOptions.class); + Mockito.doReturn(true).when(mockOptions).isTableLevelMetricsEnabled(); + Mockito.doReturn(tableName).when(mockOptions).getAllowedListTableNames(); + Mockito.doReturn(conf).when(mockOptions).getConfiguration(); + TableMetricsManager tableMetricsManager = new TableMetricsManager(mockOptions); + TableMetricsManager.setInstance(tableMetricsManager); + + //Generate 2 read metrics in each bucket, one with point lookup and other with range scan. + TableMetricsManager.updateHistogramMetricsForQueryLatency(tableName, 1, true); + TableMetricsManager.updateHistogramMetricsForQueryScanBytes(5l, tableName, true); + + TableMetricsManager.updateHistogramMetricsForQueryLatency(tableName, 2, false); + TableMetricsManager.updateHistogramMetricsForQueryScanBytes(10l, tableName, false); + + TableMetricsManager.updateHistogramMetricsForQueryLatency(tableName, 4, true); + TableMetricsManager.updateHistogramMetricsForQueryScanBytes(50l, tableName, true); + + TableMetricsManager.updateHistogramMetricsForQueryLatency(tableName, 5, false); + TableMetricsManager.updateHistogramMetricsForQueryScanBytes(100l, tableName, false); + + TableMetricsManager.updateHistogramMetricsForQueryLatency(tableName, 7, true); + TableMetricsManager.updateHistogramMetricsForQueryScanBytes(500l, tableName, true); + + TableMetricsManager.updateHistogramMetricsForQueryLatency(tableName, 8, false); + TableMetricsManager.updateHistogramMetricsForQueryScanBytes(1000l, tableName, false); + + // Generate distribution map from histogram snapshots. + LatencyHistogram latencyHistogram = + TableMetricsManager.getQueryLatencyHistogramForTable(tableName); + SizeHistogram sizeHistogram = TableMetricsManager.getQuerySizeHistogramForTable(tableName); + + Map latencyMap = latencyHistogram.getRangeHistogramDistribution().getRangeDistributionMap(); + Map sizeMap = sizeHistogram.getRangeHistogramDistribution().getRangeDistributionMap(); + for (Long count: latencyMap.values()) { + Assert.assertEquals(new Long(2), count); + } + for (Long count: sizeMap.values()) { + Assert.assertEquals(new Long(2), count); + } + + // Verify there is 1 entry in each bucket for point lookup query. + LatencyHistogram pointLookupLtHisto = + TableMetricsManager.getPointLookupLatencyHistogramForTable(tableName); + SizeHistogram pointLookupSizeHisto = + TableMetricsManager.getPointLookupSizeHistogramForTable(tableName); + + Map pointLookupLtMap = pointLookupLtHisto.getRangeHistogramDistribution().getRangeDistributionMap(); + Map pointLookupSizeMap = pointLookupSizeHisto.getRangeHistogramDistribution().getRangeDistributionMap(); + for (Long count: pointLookupLtMap.values()) { + Assert.assertEquals(new Long(1), count); + } + for (Long count: pointLookupSizeMap.values()) { + Assert.assertEquals(new Long(1), count); + } + + // Verify there is 1 entry in each bucket for range scan query. + LatencyHistogram rangeScanLtHisto = + TableMetricsManager.getRangeScanLatencyHistogramForTable(tableName); + SizeHistogram rangeScanSizeHisto = + TableMetricsManager.getRangeScanSizeHistogramForTable(tableName); + + Map rangeScanLtMap = rangeScanLtHisto.getRangeHistogramDistribution().getRangeDistributionMap(); + Map rangeScanSizeMap = rangeScanSizeHisto.getRangeHistogramDistribution().getRangeDistributionMap(); + for (Long count: rangeScanLtMap.values()) { + Assert.assertEquals(new Long(1), count); + } + for (Long count: rangeScanSizeMap.values()) { + Assert.assertEquals(new Long(1), count); + } + } + + @Test + public void testTableMetricsNull() { + String tableName = "TEST-TABLE"; + String badTableName = "NOT-ALLOWED-TABLE"; + + QueryServicesOptions mockOptions = Mockito.mock(QueryServicesOptions.class); + Mockito.doReturn(true).when(mockOptions).isTableLevelMetricsEnabled(); + Mockito.doReturn(tableName).when(mockOptions).getAllowedListTableNames(); + + TableMetricsManager tableMetricsManager = new TableMetricsManager(mockOptions); + TableMetricsManager.setInstance(tableMetricsManager); + Assert.assertNull(TableMetricsManager.getQueryLatencyHistogramForTable(badTableName)); + } } \ No newline at end of file diff --git a/pom.xml b/pom.xml index 6baaae79be7..c58241855dc 100644 --- a/pom.xml +++ b/pom.xml @@ -162,6 +162,7 @@ 0.700 0.600 + 2.1.12 6.3.1 0.6.1 @@ -944,6 +945,11 @@ hbase-shaded-protobuf ${hbase.thirdparty.version} + + org.hdrhistogram + HdrHistogram + ${hdrhistogram.version} + From 7ea0d2a5a299af786136407793f65ddf8376d166 Mon Sep 17 00:00:00 2001 From: vmeka2020 Date: Fri, 14 May 2021 01:06:14 -0700 Subject: [PATCH 02/11] Adding Functions in PhoenixRuntime --- .../java/org/apache/phoenix/util/PhoenixRuntime.java | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/phoenix-core/src/main/java/org/apache/phoenix/util/PhoenixRuntime.java b/phoenix-core/src/main/java/org/apache/phoenix/util/PhoenixRuntime.java index 0a258808c87..103c7f77d2f 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/util/PhoenixRuntime.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/util/PhoenixRuntime.java @@ -48,6 +48,7 @@ import javax.annotation.Nullable; import org.apache.hbase.thirdparty.com.google.common.annotations.VisibleForTesting; +import org.apache.phoenix.monitoring.HistogramDistribution; import org.apache.phoenix.monitoring.PhoenixTableMetric; import org.apache.phoenix.monitoring.TableMetricsManager; import org.apache.phoenix.thirdparty.org.apache.commons.cli.CommandLine; @@ -1390,6 +1391,14 @@ public static Map> getPhoenixTableClientMetrics( return TableMetricsManager.getTableMetricsMethod(); } + public static Map> getLatencyHistograms() { + return TableMetricsManager.getLatencyHistogramsForAllTables(); + } + + public static Map> getSizeHistograms() { + return TableMetricsManager.getSizeHistogramsForAllTables(); + } + /** * This is only used in testcases to reset the tableLevel Metrics data */ From 4d1c807b382a3d6571ed9828875112fcc4aaa460 Mon Sep 17 00:00:00 2001 From: vmeka2020 Date: Fri, 14 May 2021 10:04:45 -0700 Subject: [PATCH 03/11] fixing testcases --- .../apache/phoenix/monitoring/NoOpTableMetricsManager.java | 4 ++++ .../org/apache/phoenix/monitoring/TableMetricsManager.java | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/NoOpTableMetricsManager.java b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/NoOpTableMetricsManager.java index 23248d09e41..7de4f5d542b 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/NoOpTableMetricsManager.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/NoOpTableMetricsManager.java @@ -50,4 +50,8 @@ private NoOpTableMetricsManager() { @Override public Map> getTableLevelMetrics() { return Collections.emptyMap(); } + + @Override public TableClientMetrics getTableClientMetrics(String tableName) { + return null; + } } diff --git a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/TableMetricsManager.java b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/TableMetricsManager.java index 2c8d8c577de..65e5b1487e3 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/TableMetricsManager.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/TableMetricsManager.java @@ -230,7 +230,7 @@ public void updateMetrics(String tableName, MetricType type, long value) { * @param tableName * @return TableClientMetrics object */ - private TableClientMetrics getTableClientMetrics(String tableName) { + public TableClientMetrics getTableClientMetrics(String tableName) { if (Strings.isNullOrEmpty(tableName)) { LOGGER.debug("Phoenix Table metrics TableName cannot be null or empty"); From c9b59783f3789eecdb8ed513407c09ede834006e Mon Sep 17 00:00:00 2001 From: vmeka2020 Date: Wed, 19 May 2021 12:03:49 -0700 Subject: [PATCH 04/11] addressing review comments --- phoenix-core/pom.xml | 1 + .../apache/phoenix/monitoring/HistogramDistribution.java | 1 + .../phoenix/monitoring/HistogramDistributionImpl.java | 1 + .../org/apache/phoenix/monitoring/LatencyHistogram.java | 1 + .../phoenix/monitoring/MetricPublisherSupplierFactory.java | 1 + .../apache/phoenix/monitoring/NoOpTableMetricsManager.java | 1 + .../org/apache/phoenix/monitoring/PhoenixTableMetric.java | 1 + .../apache/phoenix/monitoring/PhoenixTableMetricImpl.java | 1 + .../java/org/apache/phoenix/monitoring/RangeHistogram.java | 1 + .../java/org/apache/phoenix/monitoring/SizeHistogram.java | 1 + .../org/apache/phoenix/monitoring/TableClientMetrics.java | 1 + .../java/org/apache/phoenix/monitoring/TableHistograms.java | 1 + .../org/apache/phoenix/monitoring/TableMetricsManager.java | 1 + .../org/apache/phoenix/monitoring/LatencyHistogramTest.java | 1 + .../phoenix/monitoring/PhoenixTableMetricImplTest.java | 1 + .../org/apache/phoenix/monitoring/SizeHistogramTest.java | 1 + .../apache/phoenix/monitoring/TableClientMetricsTest.java | 1 + .../org/apache/phoenix/monitoring/TableHistogramsTest.java | 1 + .../apache/phoenix/monitoring/TableMetricsManagerTest.java | 1 + pom.xml | 6 ------ 20 files changed, 19 insertions(+), 6 deletions(-) diff --git a/phoenix-core/pom.xml b/phoenix-core/pom.xml index b2a8feb1939..f17de9a054a 100644 --- a/phoenix-core/pom.xml +++ b/phoenix-core/pom.xml @@ -510,6 +510,7 @@ org.hdrhistogram HdrHistogram + 2.1.12 diff --git a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/HistogramDistribution.java b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/HistogramDistribution.java index 7c28b45a975..4e8039c27f5 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/HistogramDistribution.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/HistogramDistribution.java @@ -29,4 +29,5 @@ public interface HistogramDistribution { public String getHistoName(); public Map getRangeDistributionMap(); + } diff --git a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/HistogramDistributionImpl.java b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/HistogramDistributionImpl.java index eb8b63c8456..8041f4395a3 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/HistogramDistributionImpl.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/HistogramDistributionImpl.java @@ -70,4 +70,5 @@ public String getHistoName() { public Map getRangeDistributionMap() { return rangeDistribution; } + } diff --git a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/LatencyHistogram.java b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/LatencyHistogram.java index 144372e43f8..108b48d1eb5 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/LatencyHistogram.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/LatencyHistogram.java @@ -41,4 +41,5 @@ private static long[] initializeRanges(Configuration conf) { QueryServices.PHOENIX_HISTOGRAM_LATENCY_RANGES); return ranges != null ? ranges : DEFAULT_RANGE; } + } \ No newline at end of file diff --git a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/MetricPublisherSupplierFactory.java b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/MetricPublisherSupplierFactory.java index 85c2ee9c295..dee2345a330 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/MetricPublisherSupplierFactory.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/MetricPublisherSupplierFactory.java @@ -31,4 +31,5 @@ public interface MetricPublisherSupplierFactory extends MetricsRegistry { * Interface for UnRegistering Publisher Method */ void unregisterMetricProvider(); + } diff --git a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/NoOpTableMetricsManager.java b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/NoOpTableMetricsManager.java index 7de4f5d542b..5194518f8b5 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/NoOpTableMetricsManager.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/NoOpTableMetricsManager.java @@ -54,4 +54,5 @@ private NoOpTableMetricsManager() { @Override public TableClientMetrics getTableClientMetrics(String tableName) { return null; } + } diff --git a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/PhoenixTableMetric.java b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/PhoenixTableMetric.java index aa8d31d3531..ddf880fd066 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/PhoenixTableMetric.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/PhoenixTableMetric.java @@ -34,4 +34,5 @@ public interface PhoenixTableMetric extends Metric { * @return Sum of the values of the metric sampled since the last {@link #reset()} call. */ public long getTotalSum(); + } diff --git a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/PhoenixTableMetricImpl.java b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/PhoenixTableMetricImpl.java index 2fbf6a7789f..91bade1bd70 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/PhoenixTableMetricImpl.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/PhoenixTableMetricImpl.java @@ -74,4 +74,5 @@ public PhoenixTableMetricImpl(MetricType type) { metric.decrement(); numberOfSamples.incrementAndGet(); } + } diff --git a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/RangeHistogram.java b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/RangeHistogram.java index e935726b325..f5b64cc1e32 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/RangeHistogram.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/RangeHistogram.java @@ -110,4 +110,5 @@ private Map generateDistributionMap(Histogram snapshot) { } return map; } + } \ No newline at end of file diff --git a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/SizeHistogram.java b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/SizeHistogram.java index 2af4ea31551..0c03b3cb5c6 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/SizeHistogram.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/SizeHistogram.java @@ -40,4 +40,5 @@ private static long[] initializeRanges(Configuration conf) { QueryServices.PHOENIX_HISTOGRAM_SIZE_RANGES); return ranges != null ? ranges : DEFAULT_RANGE; } + } \ No newline at end of file diff --git a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/TableClientMetrics.java b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/TableClientMetrics.java index 6bf619b6711..d434593e38b 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/TableClientMetrics.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/TableClientMetrics.java @@ -192,4 +192,5 @@ public Map getMetricRegistry() { public TableHistograms getTableHistograms() { return tableHistograms; } + } \ No newline at end of file diff --git a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/TableHistograms.java b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/TableHistograms.java index 1de0030d3de..deef9b264e8 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/TableHistograms.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/TableHistograms.java @@ -116,4 +116,5 @@ public List getTableSizeHistogramsDistribution() { pointLookupSizeHisto.getRangeHistogramDistribution(), rangeScanSizeHisto.getRangeHistogramDistribution())); return Collections.unmodifiableList(list); } + } \ No newline at end of file diff --git a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/TableMetricsManager.java b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/TableMetricsManager.java index 65e5b1487e3..12fb39fa052 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/TableMetricsManager.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/TableMetricsManager.java @@ -465,4 +465,5 @@ public static void updateLatencyHistogramForMutations(String tableName, long ela tableMetrics.getTableHistograms().getDeleteLatencyHisto().add(elapsedTime); } } + } diff --git a/phoenix-core/src/test/java/org/apache/phoenix/monitoring/LatencyHistogramTest.java b/phoenix-core/src/test/java/org/apache/phoenix/monitoring/LatencyHistogramTest.java index e49d5c1a528..e65185e0cea 100644 --- a/phoenix-core/src/test/java/org/apache/phoenix/monitoring/LatencyHistogramTest.java +++ b/phoenix-core/src/test/java/org/apache/phoenix/monitoring/LatencyHistogramTest.java @@ -91,4 +91,5 @@ public void testEveryRangeInDefaultRange() { expectedMap.put("300000,600000", 1l); Assert.assertEquals(expectedMap, distribution); } + } \ No newline at end of file diff --git a/phoenix-core/src/test/java/org/apache/phoenix/monitoring/PhoenixTableMetricImplTest.java b/phoenix-core/src/test/java/org/apache/phoenix/monitoring/PhoenixTableMetricImplTest.java index 87eac178580..28611e7f0f2 100644 --- a/phoenix-core/src/test/java/org/apache/phoenix/monitoring/PhoenixTableMetricImplTest.java +++ b/phoenix-core/src/test/java/org/apache/phoenix/monitoring/PhoenixTableMetricImplTest.java @@ -95,4 +95,5 @@ public class PhoenixTableMetricImplTest { metric.change(10); assertEquals(10, metric.getValue()); } + } diff --git a/phoenix-core/src/test/java/org/apache/phoenix/monitoring/SizeHistogramTest.java b/phoenix-core/src/test/java/org/apache/phoenix/monitoring/SizeHistogramTest.java index c623bcabc23..616d8dd7028 100644 --- a/phoenix-core/src/test/java/org/apache/phoenix/monitoring/SizeHistogramTest.java +++ b/phoenix-core/src/test/java/org/apache/phoenix/monitoring/SizeHistogramTest.java @@ -76,4 +76,5 @@ public void testEveryRangeInDefaultRange() { expectedMap.put("10000000,100000000", 1l); Assert.assertEquals(expectedMap, distribution); } + } \ No newline at end of file diff --git a/phoenix-core/src/test/java/org/apache/phoenix/monitoring/TableClientMetricsTest.java b/phoenix-core/src/test/java/org/apache/phoenix/monitoring/TableClientMetricsTest.java index 3bbafa9fe1c..52e7793a8ae 100644 --- a/phoenix-core/src/test/java/org/apache/phoenix/monitoring/TableClientMetricsTest.java +++ b/phoenix-core/src/test/java/org/apache/phoenix/monitoring/TableClientMetricsTest.java @@ -214,4 +214,5 @@ public void testTableClientMetricsforTableName() { } assertTrue(verifyTableName()); } + } diff --git a/phoenix-core/src/test/java/org/apache/phoenix/monitoring/TableHistogramsTest.java b/phoenix-core/src/test/java/org/apache/phoenix/monitoring/TableHistogramsTest.java index fb4d740e64d..2d0e52c6fff 100644 --- a/phoenix-core/src/test/java/org/apache/phoenix/monitoring/TableHistogramsTest.java +++ b/phoenix-core/src/test/java/org/apache/phoenix/monitoring/TableHistogramsTest.java @@ -43,4 +43,5 @@ public void testTableHistograms() { Assert.assertEquals(5, tableHistograms.getTableLatencyHistogramsDistribution().size()); Assert.assertEquals(5, tableHistograms.getTableSizeHistogramsDistribution().size()); } + } \ No newline at end of file diff --git a/phoenix-core/src/test/java/org/apache/phoenix/monitoring/TableMetricsManagerTest.java b/phoenix-core/src/test/java/org/apache/phoenix/monitoring/TableMetricsManagerTest.java index 533b044c42a..5b3287185d9 100644 --- a/phoenix-core/src/test/java/org/apache/phoenix/monitoring/TableMetricsManagerTest.java +++ b/phoenix-core/src/test/java/org/apache/phoenix/monitoring/TableMetricsManagerTest.java @@ -428,4 +428,5 @@ public void testTableMetricsNull() { TableMetricsManager.setInstance(tableMetricsManager); Assert.assertNull(TableMetricsManager.getQueryLatencyHistogramForTable(badTableName)); } + } \ No newline at end of file diff --git a/pom.xml b/pom.xml index c58241855dc..6baaae79be7 100644 --- a/pom.xml +++ b/pom.xml @@ -162,7 +162,6 @@ 0.700 0.600 - 2.1.12 6.3.1 0.6.1 @@ -945,11 +944,6 @@ hbase-shaded-protobuf ${hbase.thirdparty.version} - - org.hdrhistogram - HdrHistogram - ${hdrhistogram.version} - From 7c1c5544a1c3d16dc0b40fd19404a270bca1f0b6 Mon Sep 17 00:00:00 2001 From: vmeka2020 Date: Thu, 7 Oct 2021 14:55:00 -0700 Subject: [PATCH 05/11] Addressing review comments --- .../PhoenixTableLevelMetricsIT.java | 5 - .../monitoring/HistogramDistributionImpl.java | 16 +- .../monitoring/PhoenixTableMetricImpl.java | 1 - .../phoenix/monitoring/RangeHistogram.java | 11 +- .../phoenix/monitoring/TableHistograms.java | 29 +-- .../monitoring/TableMetricsManager.java | 181 +++++++++--------- 6 files changed, 111 insertions(+), 132 deletions(-) diff --git a/phoenix-core/src/it/java/org/apache/phoenix/monitoring/PhoenixTableLevelMetricsIT.java b/phoenix-core/src/it/java/org/apache/phoenix/monitoring/PhoenixTableLevelMetricsIT.java index 4d1dff41757..9073b0080fd 100644 --- a/phoenix-core/src/it/java/org/apache/phoenix/monitoring/PhoenixTableLevelMetricsIT.java +++ b/phoenix-core/src/it/java/org/apache/phoenix/monitoring/PhoenixTableLevelMetricsIT.java @@ -462,8 +462,6 @@ private void assertHistogramMetricsForMutations(String tableName, boolean isUpse private static void assertMetricValue(Metric m, MetricType checkType, long compareValue, CompareOp op) { if (m.getMetricType().equals(checkType)) { - System.out.println("THe Type:"+m.getMetricType().toString() + "expected value:"+m.getValue() + - "\t compare value:"+compareValue); switch (op) { case EQ: assertEquals(compareValue, m.getValue()); @@ -1165,7 +1163,6 @@ private static void assertMetricValue(Metric m, MetricType checkType, long compa writeMutMetrics, conn); } } - @Test public void testDeleteCommitTimeSlowRS() throws Throwable { String tableName = generateUniqueName(); int numRows = 15; @@ -1310,8 +1307,6 @@ public void testHistogramMetricsForQueries() throws Exception { // Reset table metrics as well as global metrics PhoenixRuntime.clearTableLevelMetrics(); PhoenixMetricsIT.resetGlobalMetrics(); - DelayedOrFailingRegionServer.setDelayEnabled(true); - DelayedOrFailingRegionServer.setDelayScan(30); try (Connection conn = getConnFromTestDriver(); Statement statement = conn.createStatement()) { String select = "SELECT * FROM " + tableName; diff --git a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/HistogramDistributionImpl.java b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/HistogramDistributionImpl.java index 8041f4395a3..cf42e8ef0cc 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/HistogramDistributionImpl.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/HistogramDistributionImpl.java @@ -26,23 +26,11 @@ public class HistogramDistributionImpl implements HistogramDistribution { private long count; private Map rangeDistribution; - public HistogramDistributionImpl(String histoName) { + public HistogramDistributionImpl(String histoName, long min, long max, long count, Map distributionMap ) { this.histoName = histoName; - } - - public void setMin(long min) { - this.min = min; - } - - public void setMax(long max) { + this.min = min; this.max = max; - } - - public void setCount(long count) { this.count = count; - } - - public void setRangeDistributionMap(Map distributionMap) { this.rangeDistribution = distributionMap; } diff --git a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/PhoenixTableMetricImpl.java b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/PhoenixTableMetricImpl.java index 91bade1bd70..2fbf6a7789f 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/PhoenixTableMetricImpl.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/PhoenixTableMetricImpl.java @@ -74,5 +74,4 @@ public PhoenixTableMetricImpl(MetricType type) { metric.decrement(); numberOfSamples.incrementAndGet(); } - } diff --git a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/RangeHistogram.java b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/RangeHistogram.java index f5b64cc1e32..d44402d7345 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/RangeHistogram.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/RangeHistogram.java @@ -62,7 +62,7 @@ public RangeHistogram(long[] ranges, String name, String description) { 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."); + LOGGER.warn("Histogram recording higher value than maximum. Ignoring it."); return; } histogram.recordValue(value); @@ -87,11 +87,10 @@ public String getDesc() { 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)); + HistogramDistributionImpl + distribution = + new HistogramDistributionImpl(name, snapshot.getMinValue(), snapshot.getMaxValue(), + snapshot.getTotalCount(), generateDistributionMap(snapshot)); return distribution; } diff --git a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/TableHistograms.java b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/TableHistograms.java index deef9b264e8..90fb563bf2d 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/TableHistograms.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/TableHistograms.java @@ -17,11 +17,9 @@ */ 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; +import org.apache.phoenix.thirdparty.com.google.common.collect.ImmutableList; public class TableHistograms { private String tableName; @@ -104,17 +102,26 @@ public SizeHistogram getDeleteSizeHisto() { } public List getTableLatencyHistogramsDistribution() { - List list = new ArrayList<>(Arrays.asList(queryLatencyHisto.getRangeHistogramDistribution(), - upsertLatencyHisto.getRangeHistogramDistribution(), deleteLatencyHisto.getRangeHistogramDistribution(), - pointLookupLatencyHisto.getRangeHistogramDistribution(), rangeScanLatencyHisto.getRangeHistogramDistribution())); - return Collections.unmodifiableList(list); + ImmutableList + list = + ImmutableList.of(queryLatencyHisto.getRangeHistogramDistribution(), + upsertLatencyHisto.getRangeHistogramDistribution(), + deleteLatencyHisto.getRangeHistogramDistribution(), + pointLookupLatencyHisto.getRangeHistogramDistribution(), + rangeScanLatencyHisto.getRangeHistogramDistribution()); + return list; } public List getTableSizeHistogramsDistribution() { - List list = new ArrayList<>(Arrays.asList(querySizeHisto.getRangeHistogramDistribution(), - upsertSizeHisto.getRangeHistogramDistribution(), deleteSizeHisto.getRangeHistogramDistribution(), - pointLookupSizeHisto.getRangeHistogramDistribution(), rangeScanSizeHisto.getRangeHistogramDistribution())); - return Collections.unmodifiableList(list); + ImmutableList + list = + ImmutableList.of(querySizeHisto.getRangeHistogramDistribution(), + upsertSizeHisto.getRangeHistogramDistribution(), + deleteSizeHisto.getRangeHistogramDistribution(), + pointLookupSizeHisto.getRangeHistogramDistribution(), + rangeScanSizeHisto.getRangeHistogramDistribution()); + return list; + } } \ No newline at end of file diff --git a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/TableMetricsManager.java b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/TableMetricsManager.java index 12fb39fa052..0afa246b0e1 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/TableMetricsManager.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/TableMetricsManager.java @@ -185,7 +185,7 @@ public static void clearTableLevelMetricsMethod() { public void pushMetricsFromConnInstance(Map> map) { if (map == null) { - LOGGER.debug("Phoenix table level metrics input map cannott be null"); + LOGGER.debug("Phoenix table level metrics input map cannot be null"); return; } @@ -296,12 +296,12 @@ public static Map> getSizeHistogramsForAllTa Map> map = new HashMap<>(); for (Map.Entry entry: tableClientMetricsMapping.entrySet()) { TableHistograms tableHistograms = entry.getValue().getTableHistograms(); - map.put(entry.getKey(), tableHistograms.getTableSizeHistogramsDistribution()); } return map; } public static Map> getLatencyHistogramsForAllTables() { + Map> map = new HashMap<>(); for (Map.Entry entry: tableClientMetricsMapping.entrySet()) { TableHistograms tableHistograms = entry.getValue().getTableHistograms(); @@ -311,158 +311,149 @@ public static Map> getLatencyHistogramsForAl } public static LatencyHistogram getUpsertLatencyHistogramForTable(String tableName) { - TableClientMetrics metrics = getInstance().getTableClientMetrics(tableName); - if (metrics == null) { - LOGGER.trace("Table level client metrics are disabled for table: " + tableName); - return null; + TableClientMetrics tableMetrics; + if( (tableMetrics = getTableClientMetricsInstance(tableName)) != null ) { + return tableMetrics.getTableHistograms().getUpsertLatencyHisto(); } - return metrics.getTableHistograms().getUpsertLatencyHisto(); + return null; } public static SizeHistogram getUpsertSizeHistogramForTable(String tableName) { - TableClientMetrics metrics = getInstance().getTableClientMetrics(tableName); - if (metrics == null) { - LOGGER.trace("Table level client metrics are disabled for table: " + tableName); - return null; + TableClientMetrics tableMetrics; + if( (tableMetrics = getTableClientMetricsInstance(tableName)) != null ) { + return tableMetrics.getTableHistograms().getUpsertSizeHisto(); } - return metrics.getTableHistograms().getUpsertSizeHisto(); + return null; } public static LatencyHistogram getDeleteLatencyHistogramForTable(String tableName) { - TableClientMetrics metrics = getInstance().getTableClientMetrics(tableName); - if (metrics == null) { - LOGGER.trace("Table level client metrics are disabled for table: " + tableName); - return null; + TableClientMetrics tableMetrics; + if( (tableMetrics = getTableClientMetricsInstance(tableName)) != null ) { + return tableMetrics.getTableHistograms().getDeleteLatencyHisto(); } - return metrics.getTableHistograms().getDeleteLatencyHisto(); + return null; } public static SizeHistogram getDeleteSizeHistogramForTable(String tableName) { - TableClientMetrics metrics = getInstance().getTableClientMetrics(tableName); - if (metrics == null) { - LOGGER.trace("Table level client metrics are disabled for table: " + tableName); - return null; + TableClientMetrics tableMetrics; + if( (tableMetrics = getTableClientMetricsInstance(tableName)) != null ) { + return tableMetrics.getTableHistograms().getDeleteSizeHisto(); } - return metrics.getTableHistograms().getDeleteSizeHisto(); + return null; } public static LatencyHistogram getQueryLatencyHistogramForTable(String tableName) { - TableClientMetrics metrics = getInstance().getTableClientMetrics(tableName); - if (metrics == null) { - LOGGER.trace("Table level client metrics are disabled for table: " + tableName); - return null; + TableClientMetrics tableMetrics; + if( (tableMetrics = getTableClientMetricsInstance(tableName)) != null ) { + return tableMetrics.getTableHistograms().getQueryLatencyHisto(); } - return metrics.getTableHistograms().getQueryLatencyHisto(); + return null; } public static SizeHistogram getQuerySizeHistogramForTable(String tableName) { - TableClientMetrics metrics = getInstance().getTableClientMetrics(tableName); - if (metrics == null) { - LOGGER.trace("Table level client metrics are disabled for table: " + tableName); - return null; + TableClientMetrics tableMetrics; + if( (tableMetrics = getTableClientMetricsInstance(tableName)) != null ) { + return tableMetrics.getTableHistograms().getQuerySizeHisto(); } - return metrics.getTableHistograms().getQuerySizeHisto(); + return null; } public static LatencyHistogram getPointLookupLatencyHistogramForTable(String tableName) { - TableClientMetrics metrics = getInstance().getTableClientMetrics(tableName); - if (metrics == null) { - LOGGER.trace("Table level client metrics are disabled for table: " + tableName); - return null; + TableClientMetrics tableMetrics; + if( (tableMetrics = getTableClientMetricsInstance(tableName)) != null ) { + return tableMetrics.getTableHistograms().getPointLookupLatencyHisto(); } - return metrics.getTableHistograms().getPointLookupLatencyHisto(); + return null; } public static SizeHistogram getPointLookupSizeHistogramForTable(String tableName) { - TableClientMetrics metrics = getInstance().getTableClientMetrics(tableName); - if (metrics == null) { - LOGGER.trace("Table level client metrics are disabled for table: " + tableName); - return null; + TableClientMetrics tableMetrics; + if( (tableMetrics = getTableClientMetricsInstance(tableName)) != null ) { + return tableMetrics.getTableHistograms().getPointLookupSizeHisto(); } - return metrics.getTableHistograms().getPointLookupSizeHisto(); + return null; } public static LatencyHistogram getRangeScanLatencyHistogramForTable(String tableName) { - TableClientMetrics metrics = getInstance().getTableClientMetrics(tableName); - if (metrics == null) { - LOGGER.trace("Table level client metrics are disabled for table: " + tableName); - return null; + TableClientMetrics tableMetrics; + if( (tableMetrics = getTableClientMetricsInstance(tableName)) != null ) { + return tableMetrics.getTableHistograms().getRangeScanLatencyHisto(); } - return metrics.getTableHistograms().getRangeScanLatencyHisto(); + return null; } public static SizeHistogram getRangeScanSizeHistogramForTable(String tableName) { - TableClientMetrics metrics = getInstance().getTableClientMetrics(tableName); - if (metrics == null) { - LOGGER.trace("Table level client metrics are disabled for table: " + tableName); - return null; + TableClientMetrics tableMetrics; + if( (tableMetrics = getTableClientMetricsInstance(tableName)) != null ) { + return tableMetrics.getTableHistograms().getRangeScanSizeHisto(); } - return metrics.getTableHistograms().getRangeScanSizeHisto(); + return null; } public static void updateHistogramMetricsForQueryLatency(String tableName, long elapsedTime, boolean isPointLookup) { - TableClientMetrics tableMetrics = getInstance().getTableClientMetrics(tableName); - if (tableMetrics == null) { - LOGGER.trace("Table level client metrics are disabled for table: " + tableName); - return; - } - LOGGER.trace("Updating latency histograms for select query: tableName: " + tableName + - " isPointLookup: " + isPointLookup + " elapsedTime: " + elapsedTime); - tableMetrics.getTableHistograms().getQueryLatencyHisto().add(elapsedTime); - if (isPointLookup) { - tableMetrics.getTableHistograms().getPointLookupLatencyHisto().add(elapsedTime); - } else { - tableMetrics.getTableHistograms().getRangeScanLatencyHisto().add(elapsedTime); + TableClientMetrics tableMetrics; + if( (tableMetrics = getTableClientMetricsInstance(tableName)) != null ) { + LOGGER.trace("Updating latency histograms for select query: tableName: " + tableName + + " isPointLookup: " + isPointLookup + " elapsedTime: " + elapsedTime); + tableMetrics.getTableHistograms().getQueryLatencyHisto().add(elapsedTime); + if (isPointLookup) { + tableMetrics.getTableHistograms().getPointLookupLatencyHisto().add(elapsedTime); + } else { + tableMetrics.getTableHistograms().getRangeScanLatencyHisto().add(elapsedTime); + } } + } public static void updateHistogramMetricsForQueryScanBytes(long scanBytes, String tableName, boolean isPointLookup) { - TableClientMetrics tableMetrics = getInstance().getTableClientMetrics(tableName); - if (tableMetrics == null) { - LOGGER.trace("Table level client metrics are disabled for table: " + tableName); - return; - } - tableMetrics.getTableHistograms().getQuerySizeHisto().add(scanBytes); - if (isPointLookup) { - tableMetrics.getTableHistograms().getPointLookupSizeHisto().add(scanBytes); - } else { - tableMetrics.getTableHistograms().getRangeScanSizeHisto().add(scanBytes); + TableClientMetrics tableMetrics; + if( (tableMetrics = getTableClientMetricsInstance(tableName)) != null ) { + tableMetrics.getTableHistograms().getQuerySizeHisto().add(scanBytes); + if (isPointLookup) { + tableMetrics.getTableHistograms().getPointLookupSizeHisto().add(scanBytes); + } else { + tableMetrics.getTableHistograms().getRangeScanSizeHisto().add(scanBytes); + } } } public static void updateSizeHistogramMetricsForMutations(String tableName, long mutationBytes, boolean isUpsert) { + TableClientMetrics tableMetrics; + if( (tableMetrics = getTableClientMetricsInstance(tableName)) != null ) { + LOGGER.trace("Updating size histograms for mutations: tableName: " + tableName + + " isUpsert: " + isUpsert + " mutation bytes: " + mutationBytes); + + if (isUpsert) { + tableMetrics.getTableHistograms().getUpsertSizeHisto().add(mutationBytes); + } else { + tableMetrics.getTableHistograms().getDeleteSizeHisto().add(mutationBytes); + } + } + } + + private static TableClientMetrics getTableClientMetricsInstance(String tableName) { TableClientMetrics tableMetrics = getInstance().getTableClientMetrics(tableName); if (tableMetrics == null) { LOGGER.trace("Table level client metrics are disabled for table: " + tableName); - return; - } - - LOGGER.trace("Updating size histograms for mutations: tableName: " + tableName + - " isUpsert: " + isUpsert + " mutation bytes: " + mutationBytes); - - if (isUpsert) { - tableMetrics.getTableHistograms().getUpsertSizeHisto().add(mutationBytes); - } else { - tableMetrics.getTableHistograms().getDeleteSizeHisto().add(mutationBytes); + return null; } + return tableMetrics; } public static void updateLatencyHistogramForMutations(String tableName, long elapsedTime, boolean isUpsert) { - TableClientMetrics tableMetrics = getInstance().getTableClientMetrics(tableName); - if (tableMetrics == null) { - LOGGER.trace("Table level client metrics are disabled for table: " + tableName); - return; - } - LOGGER.trace("Updating latency histograms for mutations: tableName: " + tableName + - " isUpsert: " + isUpsert + " elapsedTime: " + elapsedTime); - if (isUpsert) { - tableMetrics.getTableHistograms().getUpsertLatencyHisto().add(elapsedTime); - } else { - tableMetrics.getTableHistograms().getDeleteLatencyHisto().add(elapsedTime); + TableClientMetrics tableMetrics; + if( (tableMetrics = getTableClientMetricsInstance(tableName)) != null ) { + LOGGER.trace("Updating latency histograms for mutations: tableName: " + tableName + + " isUpsert: " + isUpsert + " elapsedTime: " + elapsedTime); + if (isUpsert) { + tableMetrics.getTableHistograms().getUpsertLatencyHisto().add(elapsedTime); + } else { + tableMetrics.getTableHistograms().getDeleteLatencyHisto().add(elapsedTime); + } } } From 8e983f1e4787b058c5bfc80d431d60bf0a8b9d28 Mon Sep 17 00:00:00 2001 From: vmeka2020 Date: Thu, 7 Oct 2021 15:30:07 -0700 Subject: [PATCH 06/11] addressing review comments --- .../java/org/apache/phoenix/monitoring/LatencyHistogram.java | 1 + .../main/java/org/apache/phoenix/monitoring/SizeHistogram.java | 1 + .../java/org/apache/phoenix/monitoring/TableMetricsManager.java | 1 - 3 files changed, 2 insertions(+), 1 deletion(-) diff --git a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/LatencyHistogram.java b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/LatencyHistogram.java index 108b48d1eb5..1120a03d291 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/LatencyHistogram.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/LatencyHistogram.java @@ -29,6 +29,7 @@ */ public class LatencyHistogram extends RangeHistogram { + //default range of time buckets in milli seconds. public final static long[] DEFAULT_RANGE = { 1, 3, 10, 30, 100, 300, 1000, 3000, 10000, 30000, 60000, 120000, 300000, 600000}; diff --git a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/SizeHistogram.java b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/SizeHistogram.java index 0c03b3cb5c6..8ab62d7e5ee 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/SizeHistogram.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/SizeHistogram.java @@ -29,6 +29,7 @@ */ public class SizeHistogram extends RangeHistogram { + //default range of bins for size Histograms 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); diff --git a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/TableMetricsManager.java b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/TableMetricsManager.java index 0afa246b0e1..d0b3b4c838b 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/TableMetricsManager.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/TableMetricsManager.java @@ -403,7 +403,6 @@ public static void updateHistogramMetricsForQueryLatency(String tableName, long tableMetrics.getTableHistograms().getRangeScanLatencyHisto().add(elapsedTime); } } - } public static void updateHistogramMetricsForQueryScanBytes(long scanBytes, From 3ee43223b1599cfb333ccdacc9d8eab404ead469 Mon Sep 17 00:00:00 2001 From: vmeka2020 Date: Thu, 7 Oct 2021 20:48:02 -0700 Subject: [PATCH 07/11] addressing review comments --- .../monitoring/HistogramDistributionImpl.java | 2 +- .../phoenix/monitoring/TableHistograms.java | 27 +++++++------------ 2 files changed, 11 insertions(+), 18 deletions(-) diff --git a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/HistogramDistributionImpl.java b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/HistogramDistributionImpl.java index cf42e8ef0cc..ffe4fb7ba98 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/HistogramDistributionImpl.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/HistogramDistributionImpl.java @@ -26,7 +26,7 @@ public class HistogramDistributionImpl implements HistogramDistribution { private long count; private Map rangeDistribution; - public HistogramDistributionImpl(String histoName, long min, long max, long count, Map distributionMap ) { + public HistogramDistributionImpl(final String histoName, final long min, final long max, final long count, final Map distributionMap ) { this.histoName = histoName; this.min = min; this.max = max; diff --git a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/TableHistograms.java b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/TableHistograms.java index 90fb563bf2d..5e364d6cd1b 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/TableHistograms.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/TableHistograms.java @@ -102,26 +102,19 @@ public SizeHistogram getDeleteSizeHisto() { } public List getTableLatencyHistogramsDistribution() { - ImmutableList - list = - ImmutableList.of(queryLatencyHisto.getRangeHistogramDistribution(), - upsertLatencyHisto.getRangeHistogramDistribution(), - deleteLatencyHisto.getRangeHistogramDistribution(), - pointLookupLatencyHisto.getRangeHistogramDistribution(), - rangeScanLatencyHisto.getRangeHistogramDistribution()); - return list; + return ImmutableList.of(queryLatencyHisto.getRangeHistogramDistribution(), + upsertLatencyHisto.getRangeHistogramDistribution(), + deleteLatencyHisto.getRangeHistogramDistribution(), + pointLookupLatencyHisto.getRangeHistogramDistribution(), + rangeScanLatencyHisto.getRangeHistogramDistribution()); } public List getTableSizeHistogramsDistribution() { - ImmutableList - list = - ImmutableList.of(querySizeHisto.getRangeHistogramDistribution(), - upsertSizeHisto.getRangeHistogramDistribution(), - deleteSizeHisto.getRangeHistogramDistribution(), - pointLookupSizeHisto.getRangeHistogramDistribution(), - rangeScanSizeHisto.getRangeHistogramDistribution()); - return list; - + return ImmutableList.of(querySizeHisto.getRangeHistogramDistribution(), + upsertSizeHisto.getRangeHistogramDistribution(), + deleteSizeHisto.getRangeHistogramDistribution(), + pointLookupSizeHisto.getRangeHistogramDistribution(), + rangeScanSizeHisto.getRangeHistogramDistribution()); } } \ No newline at end of file From 99a2e5940b863b4c49ac9f32af63b3956c0068c8 Mon Sep 17 00:00:00 2001 From: vmeka2020 Date: Thu, 7 Oct 2021 21:22:51 -0700 Subject: [PATCH 08/11] rebasing master --- .../PhoenixTableLevelMetricsIT.java | 9 +++++-- .../monitoring/TableMetricsManagerTest.java | 24 +++++++++---------- 2 files changed, 19 insertions(+), 14 deletions(-) diff --git a/phoenix-core/src/it/java/org/apache/phoenix/monitoring/PhoenixTableLevelMetricsIT.java b/phoenix-core/src/it/java/org/apache/phoenix/monitoring/PhoenixTableLevelMetricsIT.java index 9073b0080fd..a0c5ed1c824 100644 --- a/phoenix-core/src/it/java/org/apache/phoenix/monitoring/PhoenixTableLevelMetricsIT.java +++ b/phoenix-core/src/it/java/org/apache/phoenix/monitoring/PhoenixTableLevelMetricsIT.java @@ -462,6 +462,8 @@ private void assertHistogramMetricsForMutations(String tableName, boolean isUpse private static void assertMetricValue(Metric m, MetricType checkType, long compareValue, CompareOp op) { if (m.getMetricType().equals(checkType)) { + System.out.println("THe Type:"+m.getMetricType().toString() + "expected value:"+m.getValue() + + "\t compare value:"+compareValue); switch (op) { case EQ: assertEquals(compareValue, m.getValue()); @@ -1163,6 +1165,7 @@ private static void assertMetricValue(Metric m, MetricType checkType, long compa writeMutMetrics, conn); } } + @Test public void testDeleteCommitTimeSlowRS() throws Throwable { String tableName = generateUniqueName(); int numRows = 15; @@ -1203,7 +1206,7 @@ private static void assertMetricValue(Metric m, MetricType checkType, long compa Connection conn = null; Throwable exception = null; int numAtomicUpserts = 4; - try {x + try { conn = getConnFromTestDriver(); String ddl = "create table " + tableName + "(pk varchar primary key, counter1 bigint)"; conn.createStatement().execute(ddl); @@ -1240,6 +1243,7 @@ private static void assertMetricValue(Metric m, MetricType checkType, long compa assertEquals(numAtomicUpserts, getMetricFromTableMetrics(tableName, ATOMIC_UPSERT_SQL_COUNTER)); assertTrue(getMetricFromTableMetrics(tableName, ATOMIC_UPSERT_COMMIT_TIME) > 0); } + } @Test public void testHistogramMetricsForMutations() throws Exception { @@ -1307,6 +1311,8 @@ public void testHistogramMetricsForQueries() throws Exception { // Reset table metrics as well as global metrics PhoenixRuntime.clearTableLevelMetrics(); PhoenixMetricsIT.resetGlobalMetrics(); + DelayedOrFailingRegionServer.setDelayEnabled(true); + DelayedOrFailingRegionServer.setDelayScan(30); try (Connection conn = getConnFromTestDriver(); Statement statement = conn.createStatement()) { String select = "SELECT * FROM " + tableName; @@ -1379,7 +1385,6 @@ private void assertHistogramMetricsForQueries(String tableName, LatencyHistogram Long scanBytes = GLOBAL_SCAN_BYTES.getMetric().getValue(); long maxSzValue = sizeHistogram.getHistogram().getMaxValue(); Assert.assertTrue(sizeHistogram.getHistogram().valuesAreEquivalent(scanBytes, maxSzValue)); ->>>>>>> PHOENIX-5838 Add Histograms for Table level Metrics } private Connection getConnFromTestDriver() throws SQLException { diff --git a/phoenix-core/src/test/java/org/apache/phoenix/monitoring/TableMetricsManagerTest.java b/phoenix-core/src/test/java/org/apache/phoenix/monitoring/TableMetricsManagerTest.java index 5b3287185d9..1cd90f7f4f3 100644 --- a/phoenix-core/src/test/java/org/apache/phoenix/monitoring/TableMetricsManagerTest.java +++ b/phoenix-core/src/test/java/org/apache/phoenix/monitoring/TableMetricsManagerTest.java @@ -228,32 +228,32 @@ public void testHistogramMetricsForUpsertMutations() { TableMetricsManager.updateLatencyHistogramForMutations(tableName, 1, true); MutationMetricQueue.MutationMetric metric = new MutationMetricQueue.MutationMetric( 0L, 5L, 0L, 0L, 0L, - 0L, 1L, 0L, 5L, 0L, 0L, 0L, 0L, 0L); + 0L, 1L, 0L, 5L, 0L, 0L, 0L, 0L, 0L,0L); TableMetricsManager.updateSizeHistogramMetricsForMutations(tableName, metric.getMutationsSizeBytes().getValue(), true); TableMetricsManager.updateLatencyHistogramForMutations(tableName, 2, true); metric = new MutationMetricQueue.MutationMetric(0L, 10L, 0L, 0L, 0L, - 0L, 1L, 0L, 10L, 0L, 0L, 0L, 0L, 0L); + 0L, 1L, 0L, 10L, 0L, 0L, 0L, 0L, 0L,0L); TableMetricsManager.updateSizeHistogramMetricsForMutations(tableName, metric.getMutationsSizeBytes().getValue(), true); TableMetricsManager.updateLatencyHistogramForMutations(tableName, 4, true); metric = new MutationMetricQueue.MutationMetric(0L, 50L, 0L, 0L, 0L, - 0L, 1L, 0L, 50L, 0L, 0L, 0L, 0L, 0L); + 0L, 1L, 0L, 50L, 0L, 0L, 0L, 0L, 0L,0L); TableMetricsManager.updateSizeHistogramMetricsForMutations(tableName, metric.getMutationsSizeBytes().getValue(), true); TableMetricsManager.updateLatencyHistogramForMutations(tableName, 5, true); metric = new MutationMetricQueue.MutationMetric(0L, 100L, 0L, 0L, 0L, - 0L, 1L, 0L, 100L, 0L, 0L, 0L, 0L, 0L); + 0L, 1L, 0L, 100L, 0L, 0L, 0L, 0L, 0L,0L); TableMetricsManager.updateSizeHistogramMetricsForMutations(tableName, metric.getMutationsSizeBytes().getValue(), true); TableMetricsManager.updateLatencyHistogramForMutations(tableName, 6, true); metric = new MutationMetricQueue.MutationMetric(0L, 500L, 0L, 0L, 0L, - 0L, 1L, 0L, 500L, 0L, 0L, 0L, 0L, 0L); + 0L, 1L, 0L, 500L, 0L, 0L, 0L, 0L, 0L,0L); TableMetricsManager.updateSizeHistogramMetricsForMutations(tableName, metric.getMutationsSizeBytes().getValue(), true); TableMetricsManager.updateLatencyHistogramForMutations(tableName, 8, true); metric = new MutationMetricQueue.MutationMetric(0L, 1000L, 0L, 0L, 0L, - 0L, 1L, 0L, 1000L, 0L, 0L, 0L, 0L, 0L); + 0L, 1L, 0L, 1000L, 0L, 0L, 0L, 0L, 0L,0L); TableMetricsManager.updateSizeHistogramMetricsForMutations(tableName, metric.getMutationsSizeBytes().getValue(), true); // Generate distribution map from histogram snapshots. @@ -291,32 +291,32 @@ public void testHistogramMetricsForDeleteMutations() { TableMetricsManager.updateLatencyHistogramForMutations(tableName, 1, false); MutationMetricQueue.MutationMetric metric = new MutationMetricQueue.MutationMetric( 0L, 0L, 5L, 0L, 0L, - 0L, 0L, 1L, 5L, 0L, 0L, 0L, 0L, 0L); + 0L, 0L, 1L, 5L, 0L, 0L, 0L, 0L, 0L, 0L); TableMetricsManager.updateSizeHistogramMetricsForMutations(tableName, metric.getMutationsSizeBytes().getValue(), false); TableMetricsManager.updateLatencyHistogramForMutations(tableName, 2, false); metric = new MutationMetricQueue.MutationMetric(0L, 0L, 10L, 0L, 0L, - 0L, 0L, 1L, 10L, 0L, 0L, 0L, 0L, 0L); + 0L, 0L, 1L, 10L, 0L, 0L, 0L, 0L, 0L, 0L); TableMetricsManager.updateSizeHistogramMetricsForMutations(tableName, metric.getMutationsSizeBytes().getValue(), false); TableMetricsManager.updateLatencyHistogramForMutations(tableName, 4, false); metric = new MutationMetricQueue.MutationMetric(0L, 0L, 50L, 0L, 0L, - 0L, 0L, 1L, 50L, 0L, 0L, 0L, 0L, 0L); + 0L, 0L, 1L, 50L, 0L, 0L, 0L, 0L, 0L, 0L); TableMetricsManager.updateSizeHistogramMetricsForMutations(tableName, metric.getMutationsSizeBytes().getValue(), false); TableMetricsManager.updateLatencyHistogramForMutations(tableName, 5,false); metric = new MutationMetricQueue.MutationMetric(0L, 0L, 100L, 0L, 0L, - 0L, 0L, 1L, 100L, 0L, 0L, 0L, 0L, 0L); + 0L, 0L, 1L, 100L, 0L, 0L, 0L, 0L, 0L,0L); TableMetricsManager.updateSizeHistogramMetricsForMutations(tableName, metric.getMutationsSizeBytes().getValue(), false); TableMetricsManager.updateLatencyHistogramForMutations(tableName, 6,false); metric = new MutationMetricQueue.MutationMetric(0L, 0L, 500L, 0L, 0L, - 0L, 0L, 1L, 500L, 0L, 0L, 0L, 0L, 0L); + 0L, 0L, 1L, 500L, 0L, 0L, 0L, 0L, 0L, 0L); TableMetricsManager.updateSizeHistogramMetricsForMutations(tableName, metric.getMutationsSizeBytes().getValue(), false); TableMetricsManager.updateLatencyHistogramForMutations(tableName, 8, false); metric = new MutationMetricQueue.MutationMetric(0L, 0L, 1000L, 0L, 0L, - 0L, 0L, 1L, 1000L, 0L, 0L, 0L, 0L, 0L); + 0L, 0L, 1L, 1000L, 0L, 0L, 0L, 0L, 0L,0L); TableMetricsManager.updateSizeHistogramMetricsForMutations(tableName, metric.getMutationsSizeBytes().getValue(), false); // Generate distribution map from histogram snapshots. From 47d56438d4bfa5e0ba1c3e031e223d7f6a6d1216 Mon Sep 17 00:00:00 2001 From: vmeka2020 Date: Thu, 7 Oct 2021 23:21:05 -0700 Subject: [PATCH 09/11] fixing the unit testcase --- .../apache/phoenix/execute/MutationState.java | 2 +- .../monitoring/MutationMetricQueue.java | 12 +-- .../monitoring/TableMetricsManagerTest.java | 74 ++++++++++--------- 3 files changed, 45 insertions(+), 43 deletions(-) diff --git a/phoenix-core/src/main/java/org/apache/phoenix/execute/MutationState.java b/phoenix-core/src/main/java/org/apache/phoenix/execute/MutationState.java index d09326aab61..ed50c8fe163 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/execute/MutationState.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/execute/MutationState.java @@ -1501,7 +1501,7 @@ public List getMutationList() { } // Update size and latency histogram metrics. TableMetricsManager.updateSizeHistogramMetricsForMutations(htableNameStr, - committedMutationsMetric.getMutationsSizeBytes().getValue(), allUpsertsMutations); + committedMutationsMetric.getTotalMutationsSizeBytes().getValue(), allUpsertsMutations); Long latency = timeInExecuteMutationMap.get(htableNameStr); if (latency == null) { latency = 0l; diff --git a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/MutationMetricQueue.java b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/MutationMetricQueue.java index d22702f6c93..5a129c09145 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/MutationMetricQueue.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/MutationMetricQueue.java @@ -88,7 +88,7 @@ public Map> aggregate() { publishedMetricsForTable.put(metric.getNumOfIndexCommitFailedMutations().getMetricType(), metric.getNumOfIndexCommitFailedMutations().getValue()); publishedMetricsForTable.put(metric.getUpsertMutationSqlCounterSuccess().getMetricType(), metric.getUpsertMutationSqlCounterSuccess().getValue()); publishedMetricsForTable.put(metric.getDeleteMutationSqlCounterSuccess().getMetricType(), metric.getDeleteMutationSqlCounterSuccess().getValue()); - publishedMetricsForTable.put(metric.getMutationsSizeBytes().getMetricType(), metric.getMutationsSizeBytes().getValue()); + publishedMetricsForTable.put(metric.getTotalMutationsSizeBytes().getMetricType(), metric.getTotalMutationsSizeBytes().getValue()); publishedMetricsForTable.put(metric.getUpsertBatchFailedSize().getMetricType(), metric.getUpsertBatchFailedSize().getValue()); publishedMetricsForTable.put(metric.getUpsertBatchFailedCounter().getMetricType(), metric.getUpsertBatchFailedCounter().getValue()); publishedMetricsForTable.put(metric.getDeleteBatchFailedSize().getMetricType(), metric.getDeleteBatchFailedSize().getValue()); @@ -107,7 +107,7 @@ public void clearMetrics() { */ public static class MutationMetric { private final CombinableMetric numMutations = new CombinableMetricImpl(MUTATION_BATCH_SIZE); - private final CombinableMetric mutationsSizeBytes = new CombinableMetricImpl(MUTATION_BYTES); + private final CombinableMetric totalMutationsSizeBytes = new CombinableMetricImpl(MUTATION_BYTES); private final CombinableMetric totalCommitTimeForMutations = new CombinableMetricImpl(MUTATION_COMMIT_TIME); private final CombinableMetric numFailedMutations = new CombinableMetricImpl(MUTATION_BATCH_FAILED_SIZE); private final CombinableMetric totalCommitTimeForUpserts = new CombinableMetricImpl(UPSERT_COMMIT_TIME); @@ -144,7 +144,7 @@ public MutationMetric(long numMutations, long upsertMutationsSizeBytes, this.numOfIndexCommitFailMutations.change(numOfPhase3Failed); this.upsertMutationsSizeBytes.change(upsertMutationsSizeBytes); this.deleteMutationsSizeBytes.change(deleteMutationsSizeBytes); - this.mutationsSizeBytes.change(totalMutationBytes); + this.totalMutationsSizeBytes.change(totalMutationBytes); this.upsertMutationSqlCounterSuccess.change(upsertMutationSqlCounterSuccess); this.deleteMutationSqlCounterSuccess.change(deleteMutationSqlCounterSuccess); this.upsertBatchFailedSize.change(upsertBatchFailedSize); @@ -171,8 +171,8 @@ public CombinableMetric getNumMutations() { return numMutations; } - public CombinableMetric getMutationsSizeBytes() { - return mutationsSizeBytes; + public CombinableMetric getTotalMutationsSizeBytes() { + return totalMutationsSizeBytes; } public CombinableMetric getNumFailedMutations() { @@ -225,7 +225,7 @@ public void combineMetric(MutationMetric other) { this.numOfIndexCommitFailMutations.combine(other.numOfIndexCommitFailMutations); this.upsertMutationsSizeBytes.combine(other.upsertMutationsSizeBytes); this.deleteMutationsSizeBytes.combine(other.deleteMutationsSizeBytes); - this.mutationsSizeBytes.combine(other.mutationsSizeBytes); + this.totalMutationsSizeBytes.combine(other.totalMutationsSizeBytes); this.upsertMutationSqlCounterSuccess.combine(other.upsertMutationSqlCounterSuccess); this.deleteMutationSqlCounterSuccess.combine(other.deleteMutationSqlCounterSuccess); this.upsertBatchFailedSize.combine(other.upsertBatchFailedSize); diff --git a/phoenix-core/src/test/java/org/apache/phoenix/monitoring/TableMetricsManagerTest.java b/phoenix-core/src/test/java/org/apache/phoenix/monitoring/TableMetricsManagerTest.java index 1cd90f7f4f3..73adb8cbbf1 100644 --- a/phoenix-core/src/test/java/org/apache/phoenix/monitoring/TableMetricsManagerTest.java +++ b/phoenix-core/src/test/java/org/apache/phoenix/monitoring/TableMetricsManagerTest.java @@ -227,34 +227,35 @@ public void testHistogramMetricsForUpsertMutations() { TableMetricsManager.updateLatencyHistogramForMutations(tableName, 1, true); MutationMetricQueue.MutationMetric metric = new MutationMetricQueue.MutationMetric( - 0L, 5L, 0L, 0L, 0L, - 0L, 1L, 0L, 5L, 0L, 0L, 0L, 0L, 0L,0L); - TableMetricsManager.updateSizeHistogramMetricsForMutations(tableName, metric.getMutationsSizeBytes().getValue(), true); + 0L, 5L, 0L, 0L, 0L,0L, + 0L, 1L, 0L, 5L, 0L, 0L, 0L, 0L, 0L); + TableMetricsManager.updateSizeHistogramMetricsForMutations(tableName, metric.getTotalMutationsSizeBytes().getValue(), true); TableMetricsManager.updateLatencyHistogramForMutations(tableName, 2, true); - metric = new MutationMetricQueue.MutationMetric(0L, 10L, 0L, 0L, 0L, - 0L, 1L, 0L, 10L, 0L, 0L, 0L, 0L, 0L,0L); - TableMetricsManager.updateSizeHistogramMetricsForMutations(tableName, metric.getMutationsSizeBytes().getValue(), true); + metric = new MutationMetricQueue.MutationMetric(0L, 10L, 0L, 0L, 0L,0L, + 0L, 1L, 0L, 10L, 0L, 0L, 0L, 0L, 0L); + TableMetricsManager.updateSizeHistogramMetricsForMutations(tableName, metric.getTotalMutationsSizeBytes().getValue(), true); TableMetricsManager.updateLatencyHistogramForMutations(tableName, 4, true); - metric = new MutationMetricQueue.MutationMetric(0L, 50L, 0L, 0L, 0L, - 0L, 1L, 0L, 50L, 0L, 0L, 0L, 0L, 0L,0L); - TableMetricsManager.updateSizeHistogramMetricsForMutations(tableName, metric.getMutationsSizeBytes().getValue(), true); + metric = new MutationMetricQueue.MutationMetric(0L, 50L, 0L, 0L, 0L,0L, + 0L, 1L, 0L, 50L, 0L, 0L, 0L, 0L, 0L); + TableMetricsManager.updateSizeHistogramMetricsForMutations(tableName, metric.getTotalMutationsSizeBytes().getValue(), true); TableMetricsManager.updateLatencyHistogramForMutations(tableName, 5, true); - metric = new MutationMetricQueue.MutationMetric(0L, 100L, 0L, 0L, 0L, - 0L, 1L, 0L, 100L, 0L, 0L, 0L, 0L, 0L,0L); - TableMetricsManager.updateSizeHistogramMetricsForMutations(tableName, metric.getMutationsSizeBytes().getValue(), true); + metric = new MutationMetricQueue.MutationMetric(0L, 100L, 0L, 0L, 0L,0L, + 0L, 1L, 0L, 100L, 0L, 0L, 0L, 0L, 0L); + TableMetricsManager.updateSizeHistogramMetricsForMutations(tableName, metric.getTotalMutationsSizeBytes().getValue(), true); TableMetricsManager.updateLatencyHistogramForMutations(tableName, 6, true); - metric = new MutationMetricQueue.MutationMetric(0L, 500L, 0L, 0L, 0L, - 0L, 1L, 0L, 500L, 0L, 0L, 0L, 0L, 0L,0L); - TableMetricsManager.updateSizeHistogramMetricsForMutations(tableName, metric.getMutationsSizeBytes().getValue(), true); + metric = new MutationMetricQueue.MutationMetric(0L, 500L, 0L, 0L, 0L,0L, + 0L, 1L, 0L, 500L, 0L, 0L, 0L, 0L, 0L); + TableMetricsManager.updateSizeHistogramMetricsForMutations(tableName, metric.getTotalMutationsSizeBytes().getValue(), true); TableMetricsManager.updateLatencyHistogramForMutations(tableName, 8, true); - metric = new MutationMetricQueue.MutationMetric(0L, 1000L, 0L, 0L, 0L, - 0L, 1L, 0L, 1000L, 0L, 0L, 0L, 0L, 0L,0L); - TableMetricsManager.updateSizeHistogramMetricsForMutations(tableName, metric.getMutationsSizeBytes().getValue(), true); + metric = new MutationMetricQueue.MutationMetric(0L, 1000L, 0L, 0L, 0L,0L, + 0L, 1L, 0L, 1000L, 0L, 0L, 0L, 0L, 0L); + TableMetricsManager.updateSizeHistogramMetricsForMutations(tableName, metric.getTotalMutationsSizeBytes().getValue(), true); + // Generate distribution map from histogram snapshots. LatencyHistogram latencyHistogram = @@ -290,34 +291,35 @@ public void testHistogramMetricsForDeleteMutations() { TableMetricsManager.updateLatencyHistogramForMutations(tableName, 1, false); MutationMetricQueue.MutationMetric metric = new MutationMetricQueue.MutationMetric( - 0L, 0L, 5L, 0L, 0L, - 0L, 0L, 1L, 5L, 0L, 0L, 0L, 0L, 0L, 0L); - TableMetricsManager.updateSizeHistogramMetricsForMutations(tableName, metric.getMutationsSizeBytes().getValue(), false); + 0L, 0L, 5L, 0L, 0L, 0L, + 0L, 0L, 1L, 5L, 0L, 0L, 0L, 0L, 0L); + TableMetricsManager.updateSizeHistogramMetricsForMutations(tableName, metric.getTotalMutationsSizeBytes().getValue(), false); TableMetricsManager.updateLatencyHistogramForMutations(tableName, 2, false); - metric = new MutationMetricQueue.MutationMetric(0L, 0L, 10L, 0L, 0L, - 0L, 0L, 1L, 10L, 0L, 0L, 0L, 0L, 0L, 0L); - TableMetricsManager.updateSizeHistogramMetricsForMutations(tableName, metric.getMutationsSizeBytes().getValue(), false); + metric = new MutationMetricQueue.MutationMetric(0L, 0L, 10L, 0L, 0L, 0L, + 0L, 0L, 1L, 10L, 0L, 0L, 0L, 0L, 0L); + TableMetricsManager.updateSizeHistogramMetricsForMutations(tableName, metric.getTotalMutationsSizeBytes().getValue(), false); TableMetricsManager.updateLatencyHistogramForMutations(tableName, 4, false); - metric = new MutationMetricQueue.MutationMetric(0L, 0L, 50L, 0L, 0L, - 0L, 0L, 1L, 50L, 0L, 0L, 0L, 0L, 0L, 0L); - TableMetricsManager.updateSizeHistogramMetricsForMutations(tableName, metric.getMutationsSizeBytes().getValue(), false); + metric = new MutationMetricQueue.MutationMetric(0L, 0L, 50L, 0L, 0L, 0L, + 0L, 0L, 1L, 50L, 0L, 0L, 0L, 0L, 0L); + TableMetricsManager.updateSizeHistogramMetricsForMutations(tableName, metric.getTotalMutationsSizeBytes().getValue(), false); TableMetricsManager.updateLatencyHistogramForMutations(tableName, 5,false); - metric = new MutationMetricQueue.MutationMetric(0L, 0L, 100L, 0L, 0L, - 0L, 0L, 1L, 100L, 0L, 0L, 0L, 0L, 0L,0L); - TableMetricsManager.updateSizeHistogramMetricsForMutations(tableName, metric.getMutationsSizeBytes().getValue(), false); + metric = new MutationMetricQueue.MutationMetric(0L, 0L, 100L, 0L, 0L, 0L, + 0L, 0L, 1L, 100L, 0L, 0L, 0L, 0L, 0L); + TableMetricsManager.updateSizeHistogramMetricsForMutations(tableName, metric.getTotalMutationsSizeBytes().getValue(), false); TableMetricsManager.updateLatencyHistogramForMutations(tableName, 6,false); - metric = new MutationMetricQueue.MutationMetric(0L, 0L, 500L, 0L, 0L, - 0L, 0L, 1L, 500L, 0L, 0L, 0L, 0L, 0L, 0L); - TableMetricsManager.updateSizeHistogramMetricsForMutations(tableName, metric.getMutationsSizeBytes().getValue(), false); + metric = new MutationMetricQueue.MutationMetric(0L, 0L, 500L, 0L, 0L, 0L, + 0L, 0L, 1L, 500L, 0L, 0L, 0L, 0L, 0L); + TableMetricsManager.updateSizeHistogramMetricsForMutations(tableName, metric.getTotalMutationsSizeBytes().getValue(), false); TableMetricsManager.updateLatencyHistogramForMutations(tableName, 8, false); - metric = new MutationMetricQueue.MutationMetric(0L, 0L, 1000L, 0L, 0L, - 0L, 0L, 1L, 1000L, 0L, 0L, 0L, 0L, 0L,0L); - TableMetricsManager.updateSizeHistogramMetricsForMutations(tableName, metric.getMutationsSizeBytes().getValue(), false); + metric = new MutationMetricQueue.MutationMetric(0L, 0L, 1000L, 0L, 0L, 0L, + 0L, 0L, 1L, 1000L, 0L, 0L, 0L, 0L, 0L); + TableMetricsManager.updateSizeHistogramMetricsForMutations(tableName, metric.getTotalMutationsSizeBytes().getValue(), false); + // Generate distribution map from histogram snapshots. LatencyHistogram latencyHistogram = From ef2032f81dd332a1386d9a179d9bca992b4bd9d1 Mon Sep 17 00:00:00 2001 From: vmeka2020 Date: Sun, 10 Oct 2021 22:35:57 -0700 Subject: [PATCH 10/11] Addressing checkstyle comments --- .../monitoring/HistogramDistributionImpl.java | 12 ++-- .../monitoring/PhoenixTableMetricImpl.java | 1 + .../phoenix/monitoring/SizeHistogram.java | 4 +- .../phoenix/monitoring/TableHistograms.java | 1 + .../monitoring/TableMetricsManager.java | 60 ++++++++++--------- .../apache/phoenix/util/PhoenixRuntime.java | 12 ++-- 6 files changed, 48 insertions(+), 42 deletions(-) diff --git a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/HistogramDistributionImpl.java b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/HistogramDistributionImpl.java index ffe4fb7ba98..6c6c27d2a26 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/HistogramDistributionImpl.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/HistogramDistributionImpl.java @@ -20,13 +20,13 @@ import java.util.Map; public class HistogramDistributionImpl implements HistogramDistribution { - private String histoName; - private long min; - private long max; - private long count; - private Map rangeDistribution; + private final String histoName; + private final long min; + private final long max; + private final long count; + private final Map rangeDistribution; - public HistogramDistributionImpl(final String histoName, final long min, final long max, final long count, final Map distributionMap ) { + public HistogramDistributionImpl(String histoName, long min, long max, long count, Map distributionMap ) { this.histoName = histoName; this.min = min; this.max = max; diff --git a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/PhoenixTableMetricImpl.java b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/PhoenixTableMetricImpl.java index 2fbf6a7789f..91bade1bd70 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/PhoenixTableMetricImpl.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/PhoenixTableMetricImpl.java @@ -74,4 +74,5 @@ public PhoenixTableMetricImpl(MetricType type) { metric.decrement(); numberOfSamples.incrementAndGet(); } + } diff --git a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/SizeHistogram.java b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/SizeHistogram.java index 8ab62d7e5ee..690f83a0b5d 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/SizeHistogram.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/SizeHistogram.java @@ -30,7 +30,9 @@ public class SizeHistogram extends RangeHistogram { //default range of bins for size Histograms - public static long[] DEFAULT_RANGE = {10,100,1000,10000,100000,1000000,10000000,100000000}; + 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); diff --git a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/TableHistograms.java b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/TableHistograms.java index 5e364d6cd1b..1ef29f5da6b 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/TableHistograms.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/TableHistograms.java @@ -19,6 +19,7 @@ import java.util.List; import org.apache.hadoop.conf.Configuration; + import org.apache.phoenix.thirdparty.com.google.common.collect.ImmutableList; public class TableHistograms { diff --git a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/TableMetricsManager.java b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/TableMetricsManager.java index d0b3b4c838b..9b0184e2808 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/TableMetricsManager.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/TableMetricsManager.java @@ -49,14 +49,15 @@ public class TableMetricsManager { private static final Set allowedListOfTableNames = new HashSet<>(); private static volatile boolean isTableLevelMetricsEnabled; private static volatile boolean isMetricPublisherEnabled; - private static volatile ConcurrentMap tableClientMetricsMapping = null; + private static volatile ConcurrentMap + tableClientMetricsMapping = + null; // Singleton object private static volatile TableMetricsManager tableMetricsManager = null; private static volatile MetricPublisherSupplierFactory mPublisher = null; private static volatile QueryServicesOptions options = null; - @SuppressWarnings(value="ST_WRITE_TO_STATIC_FROM_INSTANCE_METHOD", - justification="This is how we implement the singleton pattern") + @SuppressWarnings(value = "ST_WRITE_TO_STATIC_FROM_INSTANCE_METHOD", justification = "This is how we implement the singleton pattern") public TableMetricsManager(QueryServicesOptions ops) { options = ops; isTableLevelMetricsEnabled = options.isTableLevelMetricsEnabled(); @@ -92,8 +93,9 @@ private static TableMetricsManager getInstance() { if (localRef == null) { QueryServicesOptions options = QueryServicesOptions.withDefaults(); if (!options.isTableLevelMetricsEnabled()) { - localRef = tableMetricsManager = - NoOpTableMetricsManager.noOpsTableMetricManager; + localRef = + tableMetricsManager = + NoOpTableMetricsManager.noOpsTableMetricManager; return localRef; } localRef = tableMetricsManager = new TableMetricsManager(options); @@ -294,7 +296,7 @@ public void clear() { public static Map> getSizeHistogramsForAllTables() { Map> map = new HashMap<>(); - for (Map.Entry entry: tableClientMetricsMapping.entrySet()) { + for (Map.Entry entry : tableClientMetricsMapping.entrySet()) { TableHistograms tableHistograms = entry.getValue().getTableHistograms(); } return map; @@ -303,7 +305,7 @@ public static Map> getSizeHistogramsForAllTa public static Map> getLatencyHistogramsForAllTables() { Map> map = new HashMap<>(); - for (Map.Entry entry: tableClientMetricsMapping.entrySet()) { + for (Map.Entry entry : tableClientMetricsMapping.entrySet()) { TableHistograms tableHistograms = entry.getValue().getTableHistograms(); map.put(entry.getKey(), tableHistograms.getTableLatencyHistogramsDistribution()); } @@ -312,7 +314,7 @@ public static Map> getLatencyHistogramsForAl public static LatencyHistogram getUpsertLatencyHistogramForTable(String tableName) { TableClientMetrics tableMetrics; - if( (tableMetrics = getTableClientMetricsInstance(tableName)) != null ) { + if ((tableMetrics = getTableClientMetricsInstance(tableName)) != null) { return tableMetrics.getTableHistograms().getUpsertLatencyHisto(); } return null; @@ -320,7 +322,7 @@ public static LatencyHistogram getUpsertLatencyHistogramForTable(String tableNam public static SizeHistogram getUpsertSizeHistogramForTable(String tableName) { TableClientMetrics tableMetrics; - if( (tableMetrics = getTableClientMetricsInstance(tableName)) != null ) { + if ((tableMetrics = getTableClientMetricsInstance(tableName)) != null) { return tableMetrics.getTableHistograms().getUpsertSizeHisto(); } return null; @@ -328,7 +330,7 @@ public static SizeHistogram getUpsertSizeHistogramForTable(String tableName) { public static LatencyHistogram getDeleteLatencyHistogramForTable(String tableName) { TableClientMetrics tableMetrics; - if( (tableMetrics = getTableClientMetricsInstance(tableName)) != null ) { + if ((tableMetrics = getTableClientMetricsInstance(tableName)) != null) { return tableMetrics.getTableHistograms().getDeleteLatencyHisto(); } return null; @@ -336,7 +338,7 @@ public static LatencyHistogram getDeleteLatencyHistogramForTable(String tableNam public static SizeHistogram getDeleteSizeHistogramForTable(String tableName) { TableClientMetrics tableMetrics; - if( (tableMetrics = getTableClientMetricsInstance(tableName)) != null ) { + if ((tableMetrics = getTableClientMetricsInstance(tableName)) != null) { return tableMetrics.getTableHistograms().getDeleteSizeHisto(); } return null; @@ -344,7 +346,7 @@ public static SizeHistogram getDeleteSizeHistogramForTable(String tableName) { public static LatencyHistogram getQueryLatencyHistogramForTable(String tableName) { TableClientMetrics tableMetrics; - if( (tableMetrics = getTableClientMetricsInstance(tableName)) != null ) { + if ((tableMetrics = getTableClientMetricsInstance(tableName)) != null) { return tableMetrics.getTableHistograms().getQueryLatencyHisto(); } return null; @@ -352,7 +354,7 @@ public static LatencyHistogram getQueryLatencyHistogramForTable(String tableName public static SizeHistogram getQuerySizeHistogramForTable(String tableName) { TableClientMetrics tableMetrics; - if( (tableMetrics = getTableClientMetricsInstance(tableName)) != null ) { + if ((tableMetrics = getTableClientMetricsInstance(tableName)) != null) { return tableMetrics.getTableHistograms().getQuerySizeHisto(); } return null; @@ -360,7 +362,7 @@ public static SizeHistogram getQuerySizeHistogramForTable(String tableName) { public static LatencyHistogram getPointLookupLatencyHistogramForTable(String tableName) { TableClientMetrics tableMetrics; - if( (tableMetrics = getTableClientMetricsInstance(tableName)) != null ) { + if ((tableMetrics = getTableClientMetricsInstance(tableName)) != null) { return tableMetrics.getTableHistograms().getPointLookupLatencyHisto(); } return null; @@ -368,7 +370,7 @@ public static LatencyHistogram getPointLookupLatencyHistogramForTable(String tab public static SizeHistogram getPointLookupSizeHistogramForTable(String tableName) { TableClientMetrics tableMetrics; - if( (tableMetrics = getTableClientMetricsInstance(tableName)) != null ) { + if ((tableMetrics = getTableClientMetricsInstance(tableName)) != null) { return tableMetrics.getTableHistograms().getPointLookupSizeHisto(); } return null; @@ -376,7 +378,7 @@ public static SizeHistogram getPointLookupSizeHistogramForTable(String tableName public static LatencyHistogram getRangeScanLatencyHistogramForTable(String tableName) { TableClientMetrics tableMetrics; - if( (tableMetrics = getTableClientMetricsInstance(tableName)) != null ) { + if ((tableMetrics = getTableClientMetricsInstance(tableName)) != null) { return tableMetrics.getTableHistograms().getRangeScanLatencyHisto(); } return null; @@ -384,7 +386,7 @@ public static LatencyHistogram getRangeScanLatencyHistogramForTable(String table public static SizeHistogram getRangeScanSizeHistogramForTable(String tableName) { TableClientMetrics tableMetrics; - if( (tableMetrics = getTableClientMetricsInstance(tableName)) != null ) { + if ((tableMetrics = getTableClientMetricsInstance(tableName)) != null) { return tableMetrics.getTableHistograms().getRangeScanSizeHisto(); } return null; @@ -393,9 +395,9 @@ public static SizeHistogram getRangeScanSizeHistogramForTable(String tableName) public static void updateHistogramMetricsForQueryLatency(String tableName, long elapsedTime, boolean isPointLookup) { TableClientMetrics tableMetrics; - if( (tableMetrics = getTableClientMetricsInstance(tableName)) != null ) { - LOGGER.trace("Updating latency histograms for select query: tableName: " + tableName + - " isPointLookup: " + isPointLookup + " elapsedTime: " + elapsedTime); + if ((tableMetrics = getTableClientMetricsInstance(tableName)) != null) { + LOGGER.trace("Updating latency histograms for select query: tableName: " + tableName + + " isPointLookup: " + isPointLookup + " elapsedTime: " + elapsedTime); tableMetrics.getTableHistograms().getQueryLatencyHisto().add(elapsedTime); if (isPointLookup) { tableMetrics.getTableHistograms().getPointLookupLatencyHisto().add(elapsedTime); @@ -405,10 +407,10 @@ public static void updateHistogramMetricsForQueryLatency(String tableName, long } } - public static void updateHistogramMetricsForQueryScanBytes(long scanBytes, - String tableName, boolean isPointLookup) { + public static void updateHistogramMetricsForQueryScanBytes(long scanBytes, String tableName, + boolean isPointLookup) { TableClientMetrics tableMetrics; - if( (tableMetrics = getTableClientMetricsInstance(tableName)) != null ) { + if ((tableMetrics = getTableClientMetricsInstance(tableName)) != null) { tableMetrics.getTableHistograms().getQuerySizeHisto().add(scanBytes); if (isPointLookup) { tableMetrics.getTableHistograms().getPointLookupSizeHisto().add(scanBytes); @@ -421,9 +423,9 @@ public static void updateHistogramMetricsForQueryScanBytes(long scanBytes, public static void updateSizeHistogramMetricsForMutations(String tableName, long mutationBytes, boolean isUpsert) { TableClientMetrics tableMetrics; - if( (tableMetrics = getTableClientMetricsInstance(tableName)) != null ) { - LOGGER.trace("Updating size histograms for mutations: tableName: " + tableName + - " isUpsert: " + isUpsert + " mutation bytes: " + mutationBytes); + if ((tableMetrics = getTableClientMetricsInstance(tableName)) != null) { + LOGGER.trace("Updating size histograms for mutations: tableName: " + tableName + + " isUpsert: " + isUpsert + " mutation bytes: " + mutationBytes); if (isUpsert) { tableMetrics.getTableHistograms().getUpsertSizeHisto().add(mutationBytes); @@ -445,9 +447,9 @@ private static TableClientMetrics getTableClientMetricsInstance(String tableName public static void updateLatencyHistogramForMutations(String tableName, long elapsedTime, boolean isUpsert) { TableClientMetrics tableMetrics; - if( (tableMetrics = getTableClientMetricsInstance(tableName)) != null ) { - LOGGER.trace("Updating latency histograms for mutations: tableName: " + tableName + - " isUpsert: " + isUpsert + " elapsedTime: " + elapsedTime); + if ((tableMetrics = getTableClientMetricsInstance(tableName)) != null) { + LOGGER.trace("Updating latency histograms for mutations: tableName: " + tableName + + " isUpsert: " + isUpsert + " elapsedTime: " + elapsedTime); if (isUpsert) { tableMetrics.getTableHistograms().getUpsertLatencyHisto().add(elapsedTime); } else { diff --git a/phoenix-core/src/main/java/org/apache/phoenix/util/PhoenixRuntime.java b/phoenix-core/src/main/java/org/apache/phoenix/util/PhoenixRuntime.java index 103c7f77d2f..9d57c806b42 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/util/PhoenixRuntime.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/util/PhoenixRuntime.java @@ -17,8 +17,6 @@ */ package org.apache.phoenix.util; -import static org.apache.phoenix.thirdparty.com.google.common.base.Preconditions.checkNotNull; -import static org.apache.phoenix.thirdparty.com.google.common.base.Preconditions.checkArgument; import static org.apache.phoenix.schema.types.PDataType.ARRAY_TYPE_SUFFIX; import java.io.File; @@ -47,10 +45,6 @@ import javax.annotation.Nullable; -import org.apache.hbase.thirdparty.com.google.common.annotations.VisibleForTesting; -import org.apache.phoenix.monitoring.HistogramDistribution; -import org.apache.phoenix.monitoring.PhoenixTableMetric; -import org.apache.phoenix.monitoring.TableMetricsManager; import org.apache.phoenix.thirdparty.org.apache.commons.cli.CommandLine; import org.apache.phoenix.thirdparty.org.apache.commons.cli.CommandLineParser; import org.apache.phoenix.thirdparty.org.apache.commons.cli.DefaultParser; @@ -99,6 +93,9 @@ import org.apache.phoenix.schema.TableNotFoundException; import org.apache.phoenix.schema.ValueBitSet; import org.apache.phoenix.schema.types.PDataType; +import org.apache.phoenix.monitoring.HistogramDistribution; +import org.apache.phoenix.monitoring.PhoenixTableMetric; +import org.apache.phoenix.monitoring.TableMetricsManager; import org.apache.phoenix.thirdparty.com.google.common.base.Function; import org.apache.phoenix.thirdparty.com.google.common.base.Joiner; @@ -106,6 +103,9 @@ import org.apache.phoenix.thirdparty.com.google.common.collect.ImmutableList; import org.apache.phoenix.thirdparty.com.google.common.collect.Lists; import org.apache.phoenix.thirdparty.com.google.common.collect.Maps; +import org.apache.hbase.thirdparty.com.google.common.annotations.VisibleForTesting; +import static org.apache.phoenix.thirdparty.com.google.common.base.Preconditions.checkNotNull; +import static org.apache.phoenix.thirdparty.com.google.common.base.Preconditions.checkArgument; /** * From 291093d46b698388faabd1ea6c7f191060e709cb Mon Sep 17 00:00:00 2001 From: vmeka2020 Date: Mon, 11 Oct 2021 12:01:16 -0700 Subject: [PATCH 11/11] Addressing spot check errors --- .../apache/phoenix/monitoring/HistogramDistributionImpl.java | 1 + .../java/org/apache/phoenix/monitoring/LatencyHistogram.java | 2 +- .../main/java/org/apache/phoenix/monitoring/RangeHistogram.java | 2 +- .../main/java/org/apache/phoenix/monitoring/SizeHistogram.java | 2 +- 4 files changed, 4 insertions(+), 3 deletions(-) diff --git a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/HistogramDistributionImpl.java b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/HistogramDistributionImpl.java index 6c6c27d2a26..90711561a42 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/HistogramDistributionImpl.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/HistogramDistributionImpl.java @@ -55,6 +55,7 @@ public String getHistoName() { } @Override + //The caller making the list immutable public Map getRangeDistributionMap() { return rangeDistribution; } diff --git a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/LatencyHistogram.java b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/LatencyHistogram.java index 1120a03d291..dbceb9189bb 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/LatencyHistogram.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/LatencyHistogram.java @@ -30,7 +30,7 @@ public class LatencyHistogram extends RangeHistogram { //default range of time buckets in milli seconds. - public final static long[] DEFAULT_RANGE = + protected 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) { diff --git a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/RangeHistogram.java b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/RangeHistogram.java index d44402d7345..29110b90993 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/RangeHistogram.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/RangeHistogram.java @@ -39,7 +39,7 @@ public class RangeHistogram { public RangeHistogram(long[] ranges, String name, String description) { Preconditions.checkNotNull(ranges); Preconditions.checkArgument(ranges.length != 0); - this.ranges = ranges; + this.ranges = ranges; // the ranges are static or either provided by user this.name = name; this.desc = description; /* diff --git a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/SizeHistogram.java b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/SizeHistogram.java index 690f83a0b5d..11c410ae740 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/SizeHistogram.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/SizeHistogram.java @@ -30,7 +30,7 @@ public class SizeHistogram extends RangeHistogram { //default range of bins for size Histograms - public static long[] + protected static long[] DEFAULT_RANGE = { 10, 100, 1000, 10000, 100000, 1000000, 10000000, 100000000 }; public SizeHistogram(String name, String description, Configuration conf) {