nsgupta1 commented on a change in pull request #1231:
URL: https://github.com/apache/phoenix/pull/1231#discussion_r633925984



##########
File path: 
phoenix-core/src/main/java/org/apache/phoenix/monitoring/SizeHistogram.java
##########
@@ -0,0 +1,43 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you maynot use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.phoenix.monitoring;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.phoenix.mapreduce.util.PhoenixConfigurationUtil;
+import org.apache.phoenix.query.QueryServices;
+
+/**
+ * Histogram for calculating sizes (for eg: bytes read, bytes scanned). We 
read ranges using
+ * config property {@link QueryServices#PHOENIX_HISTOGRAM_SIZE_RANGES}. If 
this property is not set
+ * then it will default to {@link 
org.apache.hadoop.metrics2.lib.MutableSizeHistogram#RANGES}
+ * values.
+ */
+public class SizeHistogram extends RangeHistogram {
+
+    public static long[] DEFAULT_RANGE = 
{10,100,1000,10000,100000,1000000,10000000,100000000};
+    public SizeHistogram(String name, String description, Configuration conf) {
+        super(initializeRanges(conf), name, description);
+        initializeRanges(conf);
+    }
+
+    private static long[] initializeRanges(Configuration conf) {
+        long[] ranges = PhoenixConfigurationUtil.getLongs(conf,
+                QueryServices.PHOENIX_HISTOGRAM_SIZE_RANGES);
+        return ranges != null ? ranges : DEFAULT_RANGE;
+    }
+}

Review comment:
       Add a new line after this.

##########
File path: 
phoenix-core/src/main/java/org/apache/phoenix/monitoring/TableClientMetrics.java
##########
@@ -179,4 +182,7 @@ public String getTableName() {
         return metricRegister;
     }
 
+    public TableHistograms getTableHistograms() {
+        return tableHistograms;
+    }
 }

Review comment:
       Add a new line after this.

##########
File path: 
phoenix-core/src/main/java/org/apache/phoenix/monitoring/RangeHistogram.java
##########
@@ -0,0 +1,112 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you maynot use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.phoenix.monitoring;
+
+import com.google.common.base.Preconditions;
+import java.util.HashMap;
+import java.util.Map;
+import org.HdrHistogram.ConcurrentHistogram;
+import org.HdrHistogram.Histogram;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/*
+    Creates a histogram with the specified range.
+ */
+public class RangeHistogram {
+    private Histogram histogram;
+    private long[] ranges;
+    private String name;
+    private String desc;
+    private static final Logger LOGGER = 
LoggerFactory.getLogger(RangeHistogram.class);
+
+    public RangeHistogram(long[] ranges, String name, String description) {
+        Preconditions.checkNotNull(ranges);
+        Preconditions.checkArgument(ranges.length != 0);
+        this.ranges = ranges;
+        this.name = name;
+        this.desc = description;
+        /*
+            Below is the memory footprint per precision as of hdrhistogram 
version 2.1.12
+            Histogram#getEstimatedFootprintInBytes provide a (conservatively 
high) estimate
+            of the Histogram's total footprint in bytes.
+            |-----------------------------------------|
+            |PRECISION |  ERROR RATE |  SIZE IN BYTES |
+            |    1     |     10%     |       3,584    |
+            |    2     |     1%      |      22,016    |
+            |    3     |     0.1%    |     147,968    |
+            |    4     |     0.01%   |   1,835,520    |
+            |    5     |     0.001%  |  11,534,848    |
+            |-----------------------------------------|
+         */
+        // highestTrackable value is the last value in the provided range.
+        this.histogram = new 
ConcurrentHistogram(this.ranges[this.ranges.length-1], 2);
+    }
+
+    public void add(long value) {
+        if (value > histogram.getHighestTrackableValue()) {
+            // Ignoring recording value more than maximum trackable value.
+            LOGGER.debug("Histogram recording higher value than maximum. 
Ignoring it.");
+            return;
+        }
+        histogram.recordValue(value);
+    }
+
+    public Histogram getHistogram() {
+        return histogram;
+    }
+
+    public long[] getRanges() {
+        return ranges;
+    }
+
+    public String getName() {
+        return name;
+    }
+
+    public String getDesc() {
+        return desc;
+    }
+
+    public HistogramDistribution getRangeHistogramDistribution() {
+        // Generate distribution from the snapshot.
+        Histogram snapshot = histogram.copy();
+        HistogramDistributionImpl distribution = new 
HistogramDistributionImpl(name);
+        distribution.setMin(snapshot.getMinValue());
+        distribution.setMax(snapshot.getMaxValue());
+        distribution.setCount(snapshot.getTotalCount());
+        
distribution.setRangeDistributionMap(generateDistributionMap(snapshot));
+        return distribution;
+    }
+
+    private Map<String, Long> generateDistributionMap(Histogram snapshot) {
+        long priorRange = 0;
+        Map<String, Long> map = new HashMap<>();
+        for (int i = 0; i < ranges.length; i++) {
+            // We get the next non equivalent range to avoid double counting.
+            // getCountBetweenValues is inclusive of both values but since we 
are getting
+            // next non equivalent value from the lower bound it will be more 
than priorRange.
+            long nextNonEquivalentRange = 
histogram.nextNonEquivalentValue(priorRange);
+            // lower exclusive upper inclusive
+            long val = snapshot.getCountBetweenValues(nextNonEquivalentRange, 
ranges[i]);
+            map.put(priorRange + "," + ranges[i], val);
+            priorRange = ranges[i];
+        }
+        return map;
+    }
+}

Review comment:
       Add a new line after this.

##########
File path: 
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<String, Long>
+                distribution = 
histogram.getRangeHistogramDistribution().getRangeDistributionMap();
+        Map<String, Long> 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);
+    }
+}

Review comment:
       Add a new line after this.

##########
File path: 
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<HistogramDistribution> getTableLatencyHistogramsDistribution() 
{
+        List<HistogramDistribution> list = new 
ArrayList<>(Arrays.asList(queryLatencyHisto.getRangeHistogramDistribution(),
+                upsertLatencyHisto.getRangeHistogramDistribution(), 
deleteLatencyHisto.getRangeHistogramDistribution(),
+                pointLookupLatencyHisto.getRangeHistogramDistribution(), 
rangeScanLatencyHisto.getRangeHistogramDistribution()));
+        return Collections.unmodifiableList(list);
+    }
+
+    public List<HistogramDistribution> getTableSizeHistogramsDistribution() {
+        List<HistogramDistribution> list = new 
ArrayList<>(Arrays.asList(querySizeHisto.getRangeHistogramDistribution(),
+                upsertSizeHisto.getRangeHistogramDistribution(), 
deleteSizeHisto.getRangeHistogramDistribution(),
+                pointLookupSizeHisto.getRangeHistogramDistribution(), 
rangeScanSizeHisto.getRangeHistogramDistribution()));
+        return Collections.unmodifiableList(list);
+    }
+}

Review comment:
       Add a new line after this.

##########
File path: 
phoenix-core/src/test/java/org/apache/phoenix/monitoring/TableMetricsManagerTest.java
##########
@@ -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<String, Long> latencyMap = 
latencyHistogram.getRangeHistogramDistribution().getRangeDistributionMap();
+        Map<String, Long> 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<String, Long> latencyMap = 
latencyHistogram.getRangeHistogramDistribution().getRangeDistributionMap();
+        Map<String, Long> 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<String, Long> latencyMap = 
latencyHistogram.getRangeHistogramDistribution().getRangeDistributionMap();
+        Map<String, Long> 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<String, Long> pointLookupLtMap = 
pointLookupLtHisto.getRangeHistogramDistribution().getRangeDistributionMap();
+        Map<String, Long> 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<String, Long> rangeScanLtMap = 
rangeScanLtHisto.getRangeHistogramDistribution().getRangeDistributionMap();
+        Map<String, Long> 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));
+    }
 }

Review comment:
       Add a new line after this.

##########
File path: 
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<String, Long> distribution = 
histogram.getRangeHistogramDistribution().getRangeDistributionMap();
+        Map<String, Long> 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);
+    }
+}

Review comment:
       Add a new line after this.

##########
File path: 
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());
+    }
+}

Review comment:
       Add a new line after this.

##########
File path: 
phoenix-core/src/main/java/org/apache/phoenix/monitoring/LatencyHistogram.java
##########
@@ -0,0 +1,44 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you maynot use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicablelaw or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.phoenix.monitoring;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.phoenix.mapreduce.util.PhoenixConfigurationUtil;
+import org.apache.phoenix.query.QueryServices;
+
+/**
+ * Histogram for calculating latencies. We read ranges using
+ * config property {@link QueryServices#PHOENIX_HISTOGRAM_LATENCY_RANGES}.
+ * If this property is not set then it will default to
+ * {@link org.apache.hadoop.metrics2.lib.MutableTimeHistogram#RANGES} values.
+ */
+public class LatencyHistogram extends RangeHistogram {
+
+    public final static long[] DEFAULT_RANGE =
+            { 1, 3, 10, 30, 100, 300, 1000, 3000, 10000, 30000, 60000, 120000, 
300000, 600000};
+
+    public LatencyHistogram(String name, String description, Configuration 
conf) {
+        super(initializeRanges(conf), name, description);
+    }
+
+    private static long[] initializeRanges(Configuration conf) {
+        long[] ranges = PhoenixConfigurationUtil.getLongs(conf,
+                QueryServices.PHOENIX_HISTOGRAM_LATENCY_RANGES);
+        return  ranges != null ? ranges : DEFAULT_RANGE;
+    }
+}

Review comment:
       Add a new line after this.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to