From 5d9c05b96f784c39f8990a84bc3c642c3d734e30 Mon Sep 17 00:00:00 2001 From: Viraj Jasani Date: Thu, 5 Nov 2020 17:22:22 +0530 Subject: [PATCH] PHOENIX-6184 : Emit ageOfUnverifiedRow metric during read repairs --- .../apache/phoenix/monitoring/IndexMetricsIT.java | 15 ++++++++++++++- .../index/metrics/GlobalIndexCheckerSource.java | 11 +++++++++++ .../metrics/GlobalIndexCheckerSourceImpl.java | 15 +++++++++++++++ .../apache/phoenix/index/GlobalIndexChecker.java | 11 +++++++---- 4 files changed, 47 insertions(+), 5 deletions(-) diff --git a/phoenix-core/src/it/java/org/apache/phoenix/monitoring/IndexMetricsIT.java b/phoenix-core/src/it/java/org/apache/phoenix/monitoring/IndexMetricsIT.java index 8524926033c..95693aa86e3 100644 --- a/phoenix-core/src/it/java/org/apache/phoenix/monitoring/IndexMetricsIT.java +++ b/phoenix-core/src/it/java/org/apache/phoenix/monitoring/IndexMetricsIT.java @@ -27,6 +27,7 @@ import org.apache.phoenix.hbase.index.metrics.MetricsIndexerSource; import org.apache.phoenix.hbase.index.metrics.MetricsIndexerSourceImpl; import org.apache.phoenix.query.QueryServices; +import org.apache.phoenix.util.EnvironmentEdgeManager; import org.apache.phoenix.util.ReadOnlyProps; import org.junit.BeforeClass; import org.junit.Test; @@ -178,11 +179,23 @@ public void testGlobalIndexCheckerHistogramMetrics() { verifyHistogram(GlobalIndexCheckerSource.INDEX_REPAIR_FAILURE_TIME, registry); verifyHistogram(getIndexCounterName(GlobalIndexCheckerSource.INDEX_REPAIR_FAILURE_TIME), registry); + + long ageOfUnverifiedRow = EnvironmentEdgeManager.currentTimeMillis() - TIME_VAL; + metricSource.updateUnverifiedIndexRowAge(INDEX_NAME, ageOfUnverifiedRow); + verifyHistogram(GlobalIndexCheckerSource.UNVERIFIED_INDEX_ROW_AGE, registry, + ageOfUnverifiedRow); + verifyHistogram(getIndexCounterName(GlobalIndexCheckerSource.UNVERIFIED_INDEX_ROW_AGE), + registry, ageOfUnverifiedRow); } private void verifyHistogram(String counterName, DynamicMetricsRegistry registry) { + verifyHistogram(counterName, registry, TIME_VAL); + } + + private void verifyHistogram(String counterName, + DynamicMetricsRegistry registry, long max) { MutableHistogram histogram = registry.getHistogram(counterName); - assertEquals(TIME_VAL, histogram.getMax()); + assertEquals(max, histogram.getMax()); } private void verifyCounter(String counterName, DynamicMetricsRegistry registry) { diff --git a/phoenix-core/src/main/java/org/apache/phoenix/hbase/index/metrics/GlobalIndexCheckerSource.java b/phoenix-core/src/main/java/org/apache/phoenix/hbase/index/metrics/GlobalIndexCheckerSource.java index d38393a03d1..0ef19da6d08 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/hbase/index/metrics/GlobalIndexCheckerSource.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/hbase/index/metrics/GlobalIndexCheckerSource.java @@ -44,6 +44,10 @@ public interface GlobalIndexCheckerSource extends BaseSource { String INDEX_REPAIR_FAILURE_TIME = "indexRepairFailureTime"; String INDEX_REPAIR_FAILURE_TIME_DESC = "Histogram for the time in milliseconds for index row repair failures"; + String UNVERIFIED_INDEX_ROW_AGE = "unverifiedIndexRowAge"; + String UNVERIFIED_INDEX_ROW_AGE_DESC = "Histogram for the age in " + + "milliseconds for unverified row soon after it is repaired"; + /** * Increments the number of index rows inspected for verified status * @param indexName Name of the index @@ -56,6 +60,13 @@ public interface GlobalIndexCheckerSource extends BaseSource { */ void incrementIndexRepairs(String indexName); + /** + * Updates the index age of unverified row histogram + * @param indexName name of the index + * @param time time taken in milliseconds + */ + void updateUnverifiedIndexRowAge(String indexName, long time); + /** * Increments the number of index repair failures * @param indexName Name of the index diff --git a/phoenix-core/src/main/java/org/apache/phoenix/hbase/index/metrics/GlobalIndexCheckerSourceImpl.java b/phoenix-core/src/main/java/org/apache/phoenix/hbase/index/metrics/GlobalIndexCheckerSourceImpl.java index 59373e383e8..f1724bd9f5d 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/hbase/index/metrics/GlobalIndexCheckerSourceImpl.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/hbase/index/metrics/GlobalIndexCheckerSourceImpl.java @@ -31,6 +31,7 @@ public class GlobalIndexCheckerSourceImpl extends BaseSourceImpl implements Glob private final MetricHistogram indexRepairTimeHisto; private final MetricHistogram indexRepairFailureTimeHisto; + private final MetricHistogram unverifiedIndexRowAge; public GlobalIndexCheckerSourceImpl() { this(METRICS_NAME, METRICS_DESCRIPTION, METRICS_CONTEXT, METRICS_JMX_CONTEXT); @@ -48,6 +49,8 @@ public GlobalIndexCheckerSourceImpl(String metricsName, indexRepairTimeHisto = getMetricsRegistry().newHistogram(INDEX_REPAIR_TIME, INDEX_REPAIR_TIME_DESC); indexRepairFailureTimeHisto = getMetricsRegistry().newHistogram(INDEX_REPAIR_FAILURE_TIME, INDEX_REPAIR_FAILURE_TIME_DESC); + unverifiedIndexRowAge = getMetricsRegistry().newHistogram( + UNVERIFIED_INDEX_ROW_AGE, UNVERIFIED_INDEX_ROW_AGE_DESC); } /** @@ -74,6 +77,18 @@ public void incrementIndexRepairFailures(String indexName) { indexRepairFailures.incr(); } + /** + * Updates the index age of unverified row histogram + * @param indexName name of the index + * @param time time taken in milliseconds + */ + public void updateUnverifiedIndexRowAge(final String indexName, + final long time) { + incrementIndexSpecificHistogram(UNVERIFIED_INDEX_ROW_AGE, indexName, + time); + unverifiedIndexRowAge.add(time); + } + /** * Updates the index repair time histogram * diff --git a/phoenix-core/src/main/java/org/apache/phoenix/index/GlobalIndexChecker.java b/phoenix-core/src/main/java/org/apache/phoenix/index/GlobalIndexChecker.java index a8a1130993b..9415f534ec2 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/index/GlobalIndexChecker.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/index/GlobalIndexChecker.java @@ -259,7 +259,8 @@ public long getMvccReadPoint() { } - private void deleteRowIfAgedEnough(byte[] indexRowKey, List row, long ts, boolean specific) throws IOException { + private void deleteRowIfAgedEnough(byte[] indexRowKey, long ts, + boolean specific) throws IOException { if ((EnvironmentEdgeManager.currentTimeMillis() - ts) > ageThreshold) { Delete del; if (specific) { @@ -357,7 +358,7 @@ private void repairIndexRows(byte[] indexRowKey, long ts, List row) throws // This means there does not exist a data table row for the data row key derived from // this unverified index row. So, no index row has been built // Delete the unverified row from index if it is old enough - deleteRowIfAgedEnough(indexRowKey, row, ts, false); + deleteRowIfAgedEnough(indexRowKey, ts, false); // Skip this unverified row (i.e., do not return it to the client). Just retuning empty row is // sufficient to do that row.clear(); @@ -379,7 +380,7 @@ private void repairIndexRows(byte[] indexRowKey, long ts, List row) throws // This means there exists a data table row for the data row key derived from this unverified index row // but the data table row does not point back to the index row. // Delete the unverified row from index if it is old enough - deleteRowIfAgedEnough(indexRowKey, row, ts, false); + deleteRowIfAgedEnough(indexRowKey, ts, false); // Open a new scanner starting from the row after the current row indexScan.withStartRow(indexRowKey, false); scanner = region.getScanner(indexScan); @@ -427,7 +428,7 @@ private void repairIndexRows(byte[] indexRowKey, long ts, List row) throws // There could be back to back such events so we need a loop to go through them do { // First delete the unverified row from index if it is old enough - deleteRowIfAgedEnough(indexRowKey, row, ts, true); + deleteRowIfAgedEnough(indexRowKey, ts, true); // Now we will do a single row scan to retrieve the verified index row built from the data table row. // Note we cannot read all versions in one scan as the max number of row versions for an index table // can be 1. In that case, we will get only one (i.e., the most recent) version instead of all versions @@ -570,6 +571,8 @@ private boolean verifyRowAndRepairIfNecessary(List cellList) throws IOExce try { repairIndexRows(rowKey, ts, cellList); metricsSource.incrementIndexRepairs(indexName); + metricsSource.updateUnverifiedIndexRowAge(indexName, + EnvironmentEdgeManager.currentTimeMillis() - ts); metricsSource.updateIndexRepairTime(indexName, EnvironmentEdgeManager.currentTimeMillis() - repairStart); } catch (IOException e) {