You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@phoenix.apache.org by go...@apache.org on 2020/05/28 00:04:39 UTC

[phoenix] branch master updated: PHOENIX-5921 Shorten verification phase fail log line and add a metric for that

This is an automated email from the ASF dual-hosted git repository.

gokcen pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/phoenix.git


The following commit(s) were added to refs/heads/master by this push:
     new dcc88af  PHOENIX-5921 Shorten verification phase fail log line and add a metric for that
dcc88af is described below

commit dcc88af8acc2ba8df10d2e9d498ab3646fdf0a78
Author: Gokcen Iskender <gi...@salesforce.com>
AuthorDate: Tue May 26 14:16:17 2020 -0700

    PHOENIX-5921 Shorten verification phase fail log line and add a metric for that
---
 .../phoenix/monitoring/BasePhoenixMetricsIT.java     |  5 +++--
 .../apache/phoenix/monitoring/PhoenixMetricsIT.java  |  6 +++++-
 .../org/apache/phoenix/execute/MutationState.java    | 20 ++++++++++++--------
 .../phoenix/monitoring/GlobalClientMetrics.java      |  2 ++
 .../org/apache/phoenix/monitoring/MetricType.java    |  1 +
 .../phoenix/monitoring/MutationMetricQueue.java      | 13 ++++++++++++-
 6 files changed, 35 insertions(+), 12 deletions(-)

diff --git a/phoenix-core/src/it/java/org/apache/phoenix/monitoring/BasePhoenixMetricsIT.java b/phoenix-core/src/it/java/org/apache/phoenix/monitoring/BasePhoenixMetricsIT.java
index 993aef3..02dff9c 100644
--- a/phoenix-core/src/it/java/org/apache/phoenix/monitoring/BasePhoenixMetricsIT.java
+++ b/phoenix-core/src/it/java/org/apache/phoenix/monitoring/BasePhoenixMetricsIT.java
@@ -19,7 +19,6 @@ package org.apache.phoenix.monitoring;
 
 import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
-import org.apache.phoenix.coprocessor.TaskRegionObserver;
 import org.apache.phoenix.end2end.BaseUniqueNamesOwnClusterIT;
 import org.apache.phoenix.jdbc.PhoenixDriver;
 import org.apache.phoenix.query.QueryServices;
@@ -110,7 +109,7 @@ public class BasePhoenixMetricsIT extends BaseUniqueNamesOwnClusterIT {
             String t = entry.getKey();
             assertEquals("Table name didn't match for mutation metrics", tableName, t);
             Map<MetricType, Long> p = entry.getValue();
-            assertEquals("There should have been four metrics", 4, p.size());
+            assertEquals("There should have been five metrics", 5, p.size());
             for (Map.Entry<MetricType, Long> metric : p.entrySet()) {
                 MetricType metricType = metric.getKey();
                 long metricValue = metric.getValue();
@@ -122,6 +121,8 @@ public class BasePhoenixMetricsIT extends BaseUniqueNamesOwnClusterIT {
                     assertTrue("Mutation bytes size should be greater than zero", metricValue > 0);
                 } else if (metricType.equals(MetricType.MUTATION_BATCH_FAILED_SIZE)) {
                     assertEquals("Zero failed mutations expected", 0, metricValue);
+                } else if (metricType.equals(MetricType.INDEX_COMMIT_FAILURE_SIZE)) {
+                    assertEquals("Zero failed phase 3 mutations expected", 0, metricValue);
                 }
             }
         }
diff --git a/phoenix-core/src/it/java/org/apache/phoenix/monitoring/PhoenixMetricsIT.java b/phoenix-core/src/it/java/org/apache/phoenix/monitoring/PhoenixMetricsIT.java
index ac21865..53fcf92 100644
--- a/phoenix-core/src/it/java/org/apache/phoenix/monitoring/PhoenixMetricsIT.java
+++ b/phoenix-core/src/it/java/org/apache/phoenix/monitoring/PhoenixMetricsIT.java
@@ -19,6 +19,7 @@ import static org.apache.phoenix.monitoring.GlobalClientMetrics.GLOBAL_MUTATION_
 import static org.apache.phoenix.monitoring.GlobalClientMetrics.GLOBAL_MUTATION_BATCH_SIZE;
 import static org.apache.phoenix.monitoring.GlobalClientMetrics.GLOBAL_MUTATION_BYTES;
 import static org.apache.phoenix.monitoring.GlobalClientMetrics.GLOBAL_MUTATION_COMMIT_TIME;
+import static org.apache.phoenix.monitoring.GlobalClientMetrics.GLOBAL_MUTATION_INDEX_COMMIT_FAILURE_COUNT;
 import static org.apache.phoenix.monitoring.GlobalClientMetrics.GLOBAL_MUTATION_SQL_COUNTER;
 import static org.apache.phoenix.monitoring.GlobalClientMetrics.GLOBAL_NUM_PARALLEL_SCANS;
 import static org.apache.phoenix.monitoring.GlobalClientMetrics.GLOBAL_OPEN_PHOENIX_CONNECTIONS;
@@ -123,6 +124,7 @@ public class PhoenixMetricsIT extends BasePhoenixMetricsIT {
         assertEquals(0, GLOBAL_MUTATION_BATCH_SIZE.getMetric().getValue());
         assertEquals(0, GLOBAL_MUTATION_BYTES.getMetric().getValue());
         assertEquals(0, GLOBAL_MUTATION_BATCH_FAILED_COUNT.getMetric().getValue());
+        assertEquals(0, GLOBAL_MUTATION_INDEX_COMMIT_FAILURE_COUNT.getMetric().getValue());
 
         assertTrue(GLOBAL_SCAN_BYTES.getMetric().getValue() > 0);
         assertTrue(GLOBAL_QUERY_TIME.getMetric().getValue() > 0);
@@ -152,6 +154,7 @@ public class PhoenixMetricsIT extends BasePhoenixMetricsIT {
         assertEquals(0, GLOBAL_FAILED_QUERY_COUNTER.getMetric().getValue());
         assertEquals(0, GLOBAL_SPOOL_FILE_COUNTER.getMetric().getValue());
         assertEquals(0, GLOBAL_MUTATION_BATCH_FAILED_COUNT.getMetric().getValue());
+        assertEquals(0, GLOBAL_MUTATION_INDEX_COMMIT_FAILURE_COUNT.getMetric().getValue());
 
         assertTrue(verifyMetricsFromSink());
     }
@@ -182,6 +185,7 @@ public class PhoenixMetricsIT extends BasePhoenixMetricsIT {
         assertEquals(0, GLOBAL_FAILED_QUERY_COUNTER.getMetric().getValue());
         assertEquals(0, GLOBAL_SPOOL_FILE_COUNTER.getMetric().getValue());
         assertEquals(0, GLOBAL_MUTATION_BATCH_FAILED_COUNT.getMetric().getValue());
+        assertEquals(0, GLOBAL_MUTATION_INDEX_COMMIT_FAILURE_COUNT.getMetric().getValue());
 
         assertTrue(GLOBAL_HBASE_COUNT_RPC_CALLS.getMetric().getValue() > 0);
         assertTrue(GLOBAL_HBASE_COUNT_MILLS_BETWEEN_NEXTS.getMetric().getValue() > 0);
@@ -329,7 +333,7 @@ public class PhoenixMetricsIT extends BasePhoenixMetricsIT {
             String t = entry.getKey();
             assertEquals("Table names didn't match!", tableName, t);
             Map<MetricType, Long> p = entry.getValue();
-            assertEquals("There should have been four metrics", 4, p.size());
+            assertEquals("There should have been five metrics", 5, p.size());
             boolean mutationBatchSizePresent = false;
             boolean mutationCommitTimePresent = false;
             boolean mutationBytesPresent = false;
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 c72522a..960edd0 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
@@ -22,6 +22,7 @@ import static org.apache.phoenix.monitoring.GlobalClientMetrics.GLOBAL_MUTATION_
 import static org.apache.phoenix.monitoring.GlobalClientMetrics.GLOBAL_MUTATION_BATCH_SIZE;
 import static org.apache.phoenix.monitoring.GlobalClientMetrics.GLOBAL_MUTATION_BYTES;
 import static org.apache.phoenix.monitoring.GlobalClientMetrics.GLOBAL_MUTATION_COMMIT_TIME;
+import static org.apache.phoenix.monitoring.GlobalClientMetrics.GLOBAL_MUTATION_INDEX_COMMIT_FAILURE_COUNT;
 import static org.apache.phoenix.query.QueryServices.WILDCARD_QUERY_DYNAMIC_COLS_ATTRIB;
 import static org.apache.phoenix.query.QueryServicesOptions.DEFAULT_WILDCARD_QUERY_DYNAMIC_COLS_ATTRIB;
 
@@ -987,25 +988,23 @@ public class MutationState implements SQLCloseable {
                     verifiedOrDeletedIndexMutations);
 
             // Phase 1: Send index mutations with the empty column value = "unverified"
-            sendMutations(unverifiedIndexMutations.entrySet().iterator(), span, indexMetaDataPtr);
+            sendMutations(unverifiedIndexMutations.entrySet().iterator(), span, indexMetaDataPtr, false);
 
             // Phase 2: Send data table and other indexes
-            sendMutations(physicalTableMutationMap.entrySet().iterator(), span, indexMetaDataPtr);
+            sendMutations(physicalTableMutationMap.entrySet().iterator(), span, indexMetaDataPtr, false);
 
             // Phase 3: Send put index mutations with the empty column value = "verified" and/or delete index mutations
             try {
-                sendMutations(verifiedOrDeletedIndexMutations.entrySet().iterator(), span, indexMetaDataPtr);
+                sendMutations(verifiedOrDeletedIndexMutations.entrySet().iterator(), span, indexMetaDataPtr, true);
             } catch (SQLException ex) {
-                // TODO: add a metric here
                 LOGGER.warn(
-                        "Ignoring exception that happened during setting index verified value to verified=TRUE "
-                                + verifiedOrDeletedIndexMutations.toString(),
+                        "Ignoring exception that happened during setting index verified value to verified=TRUE ",
                         ex);
             }
         }
     }
 
-    private void sendMutations(Iterator<Entry<TableInfo, List<Mutation>>> mutationsIterator, Span span, ImmutableBytesWritable indexMetaDataPtr)
+    private void sendMutations(Iterator<Entry<TableInfo, List<Mutation>>> mutationsIterator, Span span, ImmutableBytesWritable indexMetaDataPtr, boolean isVerifiedPhase)
             throws SQLException {
         while (mutationsIterator.hasNext()) {
             Entry<TableInfo, List<Mutation>> pair = mutationsIterator.next();
@@ -1025,6 +1024,7 @@ public class MutationState implements SQLCloseable {
             long mutationSizeBytes = 0;
             long mutationCommitTime = 0;
             long numFailedMutations = 0;
+            long numFailedPhase3Mutations = 0;
 
             long startTime = 0;
             boolean shouldRetryIndexedMutation = false;
@@ -1196,9 +1196,13 @@ public class MutationState implements SQLCloseable {
                     sqlE = new CommitException(e, uncommittedStatementIndexes, serverTimestamp);
                     numFailedMutations = uncommittedStatementIndexes.length;
                     GLOBAL_MUTATION_BATCH_FAILED_COUNT.update(numFailedMutations);
+                    if (isVerifiedPhase) {
+                        numFailedPhase3Mutations = numFailedMutations;
+                        GLOBAL_MUTATION_INDEX_COMMIT_FAILURE_COUNT.update(numFailedPhase3Mutations);
+                    }
                 } finally {
                     MutationMetric mutationsMetric = new MutationMetric(numMutations, mutationSizeBytes,
-                            mutationCommitTime, numFailedMutations);
+                            mutationCommitTime, numFailedMutations, numFailedPhase3Mutations);
                     mutationMetricQueue.addMetricsForTable(Bytes.toString(htableName), mutationsMetric);
                     try {
                         if (cache != null) cache.close();
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/GlobalClientMetrics.java b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/GlobalClientMetrics.java
index 810278d..16996e8 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/GlobalClientMetrics.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/GlobalClientMetrics.java
@@ -27,6 +27,7 @@ import static org.apache.phoenix.monitoring.MetricType.MUTATION_COMMIT_TIME;
 import static org.apache.phoenix.monitoring.MetricType.MUTATION_SQL_COUNTER;
 import static org.apache.phoenix.monitoring.MetricType.NUM_PARALLEL_SCANS;
 import static org.apache.phoenix.monitoring.MetricType.OPEN_PHOENIX_CONNECTIONS_COUNTER;
+import static org.apache.phoenix.monitoring.MetricType.INDEX_COMMIT_FAILURE_SIZE;
 import static org.apache.phoenix.monitoring.MetricType.QUERY_FAILED_COUNTER;
 import static org.apache.phoenix.monitoring.MetricType.QUERY_SERVICES_COUNTER;
 import static org.apache.phoenix.monitoring.MetricType.QUERY_TIME;
@@ -80,6 +81,7 @@ public enum GlobalClientMetrics {
     GLOBAL_MUTATION_BYTES(MUTATION_BYTES),
     GLOBAL_MUTATION_COMMIT_TIME(MUTATION_COMMIT_TIME),
     GLOBAL_MUTATION_BATCH_FAILED_COUNT(MUTATION_BATCH_FAILED_SIZE),
+    GLOBAL_MUTATION_INDEX_COMMIT_FAILURE_COUNT(INDEX_COMMIT_FAILURE_SIZE),
     GLOBAL_QUERY_TIME(QUERY_TIME),
     GLOBAL_NUM_PARALLEL_SCANS(NUM_PARALLEL_SCANS),
     GLOBAL_SCAN_BYTES(SCAN_BYTES),
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/MetricType.java b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/MetricType.java
index 8e1de66..1396a89 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/monitoring/MetricType.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/monitoring/MetricType.java
@@ -34,6 +34,7 @@ public enum MetricType {
     MUTATION_COMMIT_TIME("mt", "Time it took to commit a batch of mutations",LogLevel.OFF, PLong.INSTANCE),
     MUTATION_BATCH_FAILED_SIZE("mfs", "Number of mutations that failed to be committed",LogLevel.OFF, PLong.INSTANCE),
     MUTATION_SQL_COUNTER("msc", "Counter for number of mutation sql statements",LogLevel.OFF, PLong.INSTANCE),
+    INDEX_COMMIT_FAILURE_SIZE("p3s", "Number of mutations that failed in phase 3", LogLevel.OFF, PLong.INSTANCE),
     // query (read) related metrics
     QUERY_TIME("qt", "Query times",LogLevel.OFF, PLong.INSTANCE),
     QUERY_TIMEOUT_COUNTER("qo", "Number of times query timed out",LogLevel.DEBUG, PLong.INSTANCE),
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 08cf239..8387b2c 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
@@ -21,6 +21,7 @@ import static org.apache.phoenix.monitoring.MetricType.MUTATION_BATCH_FAILED_SIZ
 import static org.apache.phoenix.monitoring.MetricType.MUTATION_BATCH_SIZE;
 import static org.apache.phoenix.monitoring.MetricType.MUTATION_BYTES;
 import static org.apache.phoenix.monitoring.MetricType.MUTATION_COMMIT_TIME;
+import static org.apache.phoenix.monitoring.MetricType.INDEX_COMMIT_FAILURE_SIZE;
 
 import java.util.Collections;
 import java.util.HashMap;
@@ -70,6 +71,7 @@ public class MutationMetricQueue {
             publishedMetricsForTable.put(metric.getMutationsSizeBytes().getMetricType(), metric.getMutationsSizeBytes().getValue());
             publishedMetricsForTable.put(metric.getCommitTimeForMutations().getMetricType(), metric.getCommitTimeForMutations().getValue());
             publishedMetricsForTable.put(metric.getNumFailedMutations().getMetricType(), metric.getNumFailedMutations().getValue());
+            publishedMetricsForTable.put(metric.getNumOfIndexCommitFailedMutations().getMetricType(), metric.getNumOfIndexCommitFailedMutations().getValue());
         }
         return publishedMetrics;
     }
@@ -86,11 +88,15 @@ public class MutationMetricQueue {
         private final CombinableMetric mutationsSizeBytes = 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 numOfIndexCommitFailMutations = new CombinableMetricImpl(
+                INDEX_COMMIT_FAILURE_SIZE);
 
-        public MutationMetric(long numMutations, long mutationsSizeBytes, long commitTimeForMutations, long numFailedMutations) {            this.numMutations.change(numMutations);
+        public MutationMetric(long numMutations, long mutationsSizeBytes, long commitTimeForMutations, long numFailedMutations, long numOfPhase3Failed) {
+            this.numMutations.change(numMutations);
             this.mutationsSizeBytes.change(mutationsSizeBytes);
             this.totalCommitTimeForMutations.change(commitTimeForMutations);
             this.numFailedMutations.change(numFailedMutations);
+            this.numOfIndexCommitFailMutations.change(numOfPhase3Failed);
         }
 
         public CombinableMetric getCommitTimeForMutations() {
@@ -109,11 +115,16 @@ public class MutationMetricQueue {
             return numFailedMutations;
         }
 
+        public CombinableMetric getNumOfIndexCommitFailedMutations() {
+            return numOfIndexCommitFailMutations;
+        }
+
         public void combineMetric(MutationMetric other) {
             this.numMutations.combine(other.numMutations);
             this.mutationsSizeBytes.combine(other.mutationsSizeBytes);
             this.totalCommitTimeForMutations.combine(other.totalCommitTimeForMutations);
             this.numFailedMutations.combine(other.numFailedMutations);
+            this.numOfIndexCommitFailMutations.combine(other.numOfIndexCommitFailMutations);
         }
 
     }