You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@phoenix.apache.org by gj...@apache.org on 2021/11/15 17:12:57 UTC

[phoenix] branch 4.x updated: PHOENIX-6589 - Add metrics for schema registry export (#1347)

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

gjacoby pushed a commit to branch 4.x
in repository https://gitbox.apache.org/repos/asf/phoenix.git


The following commit(s) were added to refs/heads/4.x by this push:
     new 0f312d9  PHOENIX-6589 - Add metrics for schema registry export (#1347)
0f312d9 is described below

commit 0f312d9215cb242162899edd4ae834943f21fd1b
Author: Geoffrey Jacoby <gj...@salesforce.com>
AuthorDate: Mon Nov 15 12:12:49 2021 -0500

    PHOENIX-6589 - Add metrics for schema registry export (#1347)
---
 .../apache/phoenix/monitoring/IndexMetricsIT.java  |   6 +-
 .../phoenix/monitoring/MetadataMetricsIT.java      |  84 +++++++++++++++++
 .../phoenix/coprocessor/MetaDataEndpointImpl.java  |  15 ++++
 .../index/metrics/MetricsIndexerSourceFactory.java |   2 +-
 .../schema/metrics/MetricsMetadataSource.java      | 100 +++++++++++++++++++++
 .../metrics/MetricsMetadataSourceFactory.java      |  41 +++++++++
 .../schema/metrics/MetricsMetadataSourceImpl.java  |  97 ++++++++++++++++++++
 7 files changed, 341 insertions(+), 4 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 36c8dd9..d6ea814 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
@@ -191,17 +191,17 @@ public class IndexMetricsIT extends ParallelStatsDisabledIT {
             registry, ageOfUnverifiedRow);
     }
 
-    private void verifyHistogram(String counterName, DynamicMetricsRegistry registry) {
+    public static void verifyHistogram(String counterName, DynamicMetricsRegistry registry) {
         verifyHistogram(counterName, registry, TIME_VAL);
     }
 
-    private void verifyHistogram(String counterName,
+    public static void verifyHistogram(String counterName,
             DynamicMetricsRegistry registry, long max) {
         MutableHistogram histogram = registry.getHistogram(counterName);
         assertEquals(max, histogram.getMax());
     }
 
-    private void verifyCounter(String counterName, DynamicMetricsRegistry registry) {
+    public static void verifyCounter(String counterName, DynamicMetricsRegistry registry) {
         MutableFastCounter counter = registry.getCounter(counterName, 0);
         assertEquals(1, counter.value());
     }
diff --git a/phoenix-core/src/it/java/org/apache/phoenix/monitoring/MetadataMetricsIT.java b/phoenix-core/src/it/java/org/apache/phoenix/monitoring/MetadataMetricsIT.java
new file mode 100644
index 0000000..9e72a56
--- /dev/null
+++ b/phoenix-core/src/it/java/org/apache/phoenix/monitoring/MetadataMetricsIT.java
@@ -0,0 +1,84 @@
+/*
+ * 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 org.apache.hadoop.metrics2.lib.DynamicMetricsRegistry;
+import org.apache.phoenix.end2end.ParallelStatsDisabledIT;
+import org.apache.phoenix.end2end.ParallelStatsDisabledTest;
+import org.apache.phoenix.query.QueryServices;
+import org.apache.phoenix.schema.metrics.MetricsMetadataSource;
+import org.apache.phoenix.schema.metrics.MetricsMetadataSourceImpl;
+import org.apache.phoenix.thirdparty.com.google.common.collect.Maps;
+import org.apache.phoenix.util.ReadOnlyProps;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import java.util.Map;
+
+@Category(ParallelStatsDisabledTest.class)
+public class MetadataMetricsIT extends ParallelStatsDisabledIT {
+
+    @BeforeClass
+    public static void setup() throws Exception {
+        Map<String, String> props = Maps.newHashMapWithExpectedSize(3);
+        props.put(QueryServices.TASK_HANDLING_INITIAL_DELAY_MS_ATTRIB, Long.toString(Long.MAX_VALUE));
+        // disable renewing leases as this will force spooling to happen.
+        props.put(QueryServices.RENEW_LEASE_ENABLED, String.valueOf(false));
+        setUpTestDriver(new ReadOnlyProps(props.entrySet().iterator()));
+    }
+
+    @Test
+    public void testCreateMetrics() {
+        MetricsMetadataSourceImpl metadataSource = new MetricsMetadataSourceImpl();
+        DynamicMetricsRegistry registry = metadataSource.getMetricsRegistry();
+
+        metadataSource.incrementCreateExportCount();
+        IndexMetricsIT.verifyCounter(MetricsMetadataSource.CREATE_EXPORT_COUNT, registry);
+
+        metadataSource.incrementCreateExportFailureCount();
+        IndexMetricsIT.verifyCounter(MetricsMetadataSource.CREATE_EXPORT_FAILURE_COUNT, registry);
+
+        long time = 10L;
+        metadataSource.updateCreateExportTime(time);
+        IndexMetricsIT.verifyHistogram(MetricsMetadataSource.CREATE_EXPORT_TIME, registry, time);
+
+        metadataSource.updateCreateExportFailureTime(time);
+        IndexMetricsIT.verifyHistogram(MetricsMetadataSource.CREATE_EXPORT_FAILURE_TIME, registry, time);
+    }
+
+    @Test
+    public void testAlterMetrics() {
+        MetricsMetadataSourceImpl metadataSource = new MetricsMetadataSourceImpl();
+        DynamicMetricsRegistry registry = metadataSource.getMetricsRegistry();
+
+        metadataSource.incrementAlterExportCount();
+        IndexMetricsIT.verifyCounter(MetricsMetadataSource.ALTER_EXPORT_COUNT, registry);
+
+        metadataSource.incrementAlterExportFailureCount();
+        IndexMetricsIT.verifyCounter(MetricsMetadataSource.ALTER_EXPORT_FAILURE_COUNT, registry);
+
+        long time = 10L;
+        metadataSource.updateAlterExportTime(time);
+        IndexMetricsIT.verifyHistogram(MetricsMetadataSource.ALTER_EXPORT_TIME, registry, time);
+
+        metadataSource.updateAlterExportFailureTime(time);
+        IndexMetricsIT.verifyHistogram(MetricsMetadataSource.ALTER_EXPORT_FAILURE_TIME, registry, time);
+    }
+}
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/MetaDataEndpointImpl.java b/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/MetaDataEndpointImpl.java
index dae26ff..37c6fa8 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/MetaDataEndpointImpl.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/MetaDataEndpointImpl.java
@@ -223,6 +223,8 @@ import org.apache.phoenix.schema.export.SchemaRegistryRepository;
 import org.apache.phoenix.schema.export.SchemaRegistryRepositoryFactory;
 import org.apache.phoenix.schema.export.SchemaWriter;
 import org.apache.phoenix.schema.export.SchemaWriterFactory;
+import org.apache.phoenix.schema.metrics.MetricsMetadataSource;
+import org.apache.phoenix.schema.metrics.MetricsMetadataSourceFactory;
 import org.apache.phoenix.schema.task.SystemTaskParams;
 import org.apache.phoenix.schema.task.Task;
 import org.apache.phoenix.schema.types.PBinary;
@@ -571,6 +573,8 @@ TABLE_FAMILY_BYTES, TABLE_SEQ_NUM_BYTES);
     // before 4.15, so that we can rollback the upgrade to 4.15 if required
     private boolean allowSplittableSystemCatalogRollback;
 
+    private MetricsMetadataSource metricsSource;
+
     /**
      * Stores a reference to the coprocessor environment provided by the
      * {@link org.apache.hadoop.hbase.regionserver.RegionCoprocessorHost} from the region where this
@@ -607,6 +611,7 @@ TABLE_FAMILY_BYTES, TABLE_SEQ_NUM_BYTES);
         // Start the phoenix trace collection
         Tracing.addTraceMetricsSource();
         Metrics.ensureConfigured();
+        metricsSource = MetricsMetadataSourceFactory.getMetadataMetricsSource();
     }
 
     @Override
@@ -2329,9 +2334,14 @@ TABLE_FAMILY_BYTES, TABLE_SEQ_NUM_BYTES);
                     //and if we're doing change detection on this table or view, notify the
                     //external schema registry and get its schema id
                     if (isChangeDetectionEnabled) {
+                        long startTime = EnvironmentEdgeManager.currentTimeMillis();
                         try {
                             exportSchema(tableMetadata, tableKey, clientTimeStamp, clientVersion, null);
+                            metricsSource.incrementCreateExportCount();
+                            metricsSource.updateCreateExportTime(EnvironmentEdgeManager.currentTimeMillis() - startTime);
                         } catch (IOException ie){
+                            metricsSource.incrementCreateExportFailureCount();
+                            metricsSource.updateCreateExportFailureTime(EnvironmentEdgeManager.currentTimeMillis() - startTime);
                             //If we fail to write to the schema registry, fail the entire
                             //CREATE TABLE or VIEW operation so we stay consistent
                             LOGGER.error("Error writing schema to external schema registry", ie);
@@ -3201,10 +3211,15 @@ TABLE_FAMILY_BYTES, TABLE_SEQ_NUM_BYTES);
                 }
 
                 if (table.isChangeDetectionEnabled() || MetaDataUtil.getChangeDetectionEnabled(tableMetadata)) {
+                    long startTime = EnvironmentEdgeManager.currentTimeMillis();
                     try {
                         exportSchema(tableMetadata, key, clientTimeStamp, clientVersion, table);
+                        metricsSource.incrementAlterExportCount();
+                        metricsSource.updateAlterExportTime(EnvironmentEdgeManager.currentTimeMillis() - startTime);
                     } catch (Exception e) {
                         LOGGER.error("Error writing to schema registry", e);
+                        metricsSource.incrementAlterExportFailureCount();
+                        metricsSource.updateAlterExportFailureTime(EnvironmentEdgeManager.currentTimeMillis() - startTime);
                         result = new MetaDataMutationResult(MutationCode.ERROR_WRITING_TO_SCHEMA_REGISTRY,
                             EnvironmentEdgeManager.currentTimeMillis(), table);
                         return result;
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/hbase/index/metrics/MetricsIndexerSourceFactory.java b/phoenix-core/src/main/java/org/apache/phoenix/hbase/index/metrics/MetricsIndexerSourceFactory.java
index b105290..baf27f4 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/hbase/index/metrics/MetricsIndexerSourceFactory.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/hbase/index/metrics/MetricsIndexerSourceFactory.java
@@ -21,7 +21,7 @@ package org.apache.phoenix.hbase.index.metrics;
  */
 public class MetricsIndexerSourceFactory {
   private static final MetricsIndexerSourceFactory INSTANCE = new MetricsIndexerSourceFactory();
-  private MetricsIndexerSource indexerSource;
+  private volatile MetricsIndexerSource indexerSource;
   private GlobalIndexCheckerSource globalIndexCheckerSource;
 
   private MetricsIndexerSourceFactory() {}
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/schema/metrics/MetricsMetadataSource.java b/phoenix-core/src/main/java/org/apache/phoenix/schema/metrics/MetricsMetadataSource.java
new file mode 100644
index 0000000..bab1c05
--- /dev/null
+++ b/phoenix-core/src/main/java/org/apache/phoenix/schema/metrics/MetricsMetadataSource.java
@@ -0,0 +1,100 @@
+/*
+ * 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.schema.metrics;
+
+public interface MetricsMetadataSource {
+    // Metrics2 and JMX constants
+    String METRICS_NAME = "PhoenixMetadata";
+    String METRICS_CONTEXT = "phoenix";
+    String METRICS_DESCRIPTION = "Metrics about the Phoenix MetadataEndpoint";
+    String METRICS_JMX_CONTEXT = "RegionServer,sub=" + METRICS_NAME;
+
+    String CREATE_EXPORT_COUNT = "createExportCount";
+    String CREATE_EXPORT_COUNT_DESC = "Count of CREATE DDL statements exported to schema registry";
+
+    String CREATE_EXPORT_FAILURE_COUNT = "createExportFailureCount";
+    String CREATE_EXPORT_FAILURE_COUNT_DESC = "Count of create DDL that failed on export "
+        + "to schema registry";
+
+    String CREATE_EXPORT_TIME = "createExportTime";
+    String CREATE_EXPORT_TIME_DESC = "Time taken while exporting CREATE DDL statements to schema registry";
+
+    String CREATE_EXPORT_FAILURE_TIME = "createExportFailureTime";
+    String CREATE_EXPORT_FAILURE_TIME_DESC = "Time taken while failing to export "
+        + "CREATE DDL to schema registry";
+
+    String ALTER_EXPORT_COUNT = "alterExportCount";
+    String ALTER_EXPORT_COUNT_DESC = "Count of ALTER DDL statements exported to schema registry";
+
+    String ALTER_EXPORT_FAILURE_COUNT = "alterExportFailureCount";
+    String ALTER_EXPORT_FAILURE_COUNT_DESC = "Count of ALTER DDL that failed on export "
+        + "to schema registry";
+
+    String ALTER_EXPORT_TIME = "alterExportTime";
+    String ALTER_EXPORT_TIME_DESC = "Time taken while exporting ALTER DDL statements to schema registry";
+
+    String ALTER_EXPORT_FAILURE_TIME = "alterExportFailureTime";
+    String ALTER_EXPORT_FAILURE_TIME_DESC = "Time taken while failing to export "
+        + "ALTER DDL to schema registry";
+
+    /**
+     * Updates the count of successful requests to the schema registry for CREATE statements
+     */
+    void incrementCreateExportCount();
+
+    /**
+     * Updates the histogram of time taken to update the schema registry for CREATE statements
+     * @param t Time taken
+     */
+    void updateCreateExportTime(long t);
+
+    /**
+     * Updates the count of unsuccessful requests to the schema registry for CREATE statements
+     */
+    void incrementCreateExportFailureCount();
+
+    /**
+     * Updates the histogram of time taken trying and failing to
+     * update the schema registry for CREATE statements
+     * @param t time taken
+     */
+    void updateCreateExportFailureTime(long t);
+
+    /**
+     * Updates the count of successful requests to the schema registry for ALTER statements
+     */
+    void incrementAlterExportCount();
+
+    /**
+     * Updates the histogram of time taken updating the schema registry for ALTER statements
+     * @param t time takne
+     */
+    void updateAlterExportTime(long t);
+
+    /**
+     * Updates the count of unsuccessful requests to the schema registry for ALTER statements
+     */
+    void incrementAlterExportFailureCount();
+
+    /**
+     * Updates the histogram of time taken trying and failing to update the schema registry for
+     * ALTER statements
+     * @param t time taken
+     */
+    void updateAlterExportFailureTime(long t);
+}
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/schema/metrics/MetricsMetadataSourceFactory.java b/phoenix-core/src/main/java/org/apache/phoenix/schema/metrics/MetricsMetadataSourceFactory.java
new file mode 100644
index 0000000..29e914f
--- /dev/null
+++ b/phoenix-core/src/main/java/org/apache/phoenix/schema/metrics/MetricsMetadataSourceFactory.java
@@ -0,0 +1,41 @@
+/*
+ * 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.schema.metrics;
+
+/**
+ * Factory class for creating {@link MetricsMetadataSource} instances
+ */
+public class MetricsMetadataSourceFactory {
+    private static final MetricsMetadataSourceFactory INSTANCE = new MetricsMetadataSourceFactory();
+
+    private volatile MetricsMetadataSource metricsMetadataSource;
+
+    private MetricsMetadataSourceFactory() {}
+
+    public static MetricsMetadataSourceFactory getInstance() {
+        return INSTANCE;
+    }
+
+    public static synchronized MetricsMetadataSource getMetadataMetricsSource() {
+        if (INSTANCE.metricsMetadataSource == null) {
+            INSTANCE.metricsMetadataSource = new MetricsMetadataSourceImpl();
+        }
+        return INSTANCE.metricsMetadataSource;
+    }
+
+}
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/schema/metrics/MetricsMetadataSourceImpl.java b/phoenix-core/src/main/java/org/apache/phoenix/schema/metrics/MetricsMetadataSourceImpl.java
new file mode 100644
index 0000000..8d0e024
--- /dev/null
+++ b/phoenix-core/src/main/java/org/apache/phoenix/schema/metrics/MetricsMetadataSourceImpl.java
@@ -0,0 +1,97 @@
+/*
+ * 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.schema.metrics;
+
+import org.apache.hadoop.hbase.metrics.BaseSourceImpl;
+import org.apache.hadoop.metrics2.MetricHistogram;
+import org.apache.hadoop.metrics2.lib.MutableFastCounter;
+
+public class MetricsMetadataSourceImpl extends BaseSourceImpl implements MetricsMetadataSource {
+
+    private final MutableFastCounter createExportCount;
+    private final MetricHistogram createExportTimeHisto;
+
+    private final MutableFastCounter createExportFailureCount;
+    private final MetricHistogram createExportFailureTimeHisto;
+
+    private final MutableFastCounter alterExportCount;
+    private final MetricHistogram alterExportTimeHisto;
+
+    private final MutableFastCounter alterExportFailureCount;
+    private final MetricHistogram alterExportFailureTimeHisto;
+
+    public MetricsMetadataSourceImpl() {
+        this(METRICS_NAME, METRICS_DESCRIPTION, METRICS_CONTEXT, METRICS_JMX_CONTEXT);
+    }
+
+    public MetricsMetadataSourceImpl(String metricsName, String metricsDescription,
+        String metricsContext, String metricsJmxContext) {
+        super(metricsName, metricsDescription, metricsContext, metricsJmxContext);
+
+        createExportCount = getMetricsRegistry().newCounter(CREATE_EXPORT_COUNT,
+            CREATE_EXPORT_COUNT_DESC, 0L);
+        createExportTimeHisto = getMetricsRegistry().newHistogram(CREATE_EXPORT_TIME, CREATE_EXPORT_TIME_DESC);
+
+        createExportFailureCount = getMetricsRegistry().newCounter(CREATE_EXPORT_FAILURE_COUNT,
+            CREATE_EXPORT_FAILURE_COUNT_DESC, 0L);
+        createExportFailureTimeHisto = getMetricsRegistry().newHistogram(CREATE_EXPORT_FAILURE_TIME,
+            CREATE_EXPORT_FAILURE_TIME_DESC);
+
+        alterExportCount = getMetricsRegistry().newCounter(ALTER_EXPORT_COUNT,
+            ALTER_EXPORT_COUNT_DESC, 0L);
+        alterExportTimeHisto = getMetricsRegistry().newHistogram(ALTER_EXPORT_TIME, ALTER_EXPORT_TIME_DESC);
+
+        alterExportFailureCount = getMetricsRegistry().newCounter(ALTER_EXPORT_FAILURE_COUNT,
+            ALTER_EXPORT_FAILURE_COUNT_DESC, 0L);
+        alterExportFailureTimeHisto = getMetricsRegistry().newHistogram(ALTER_EXPORT_FAILURE_TIME,
+            ALTER_EXPORT_FAILURE_TIME_DESC);
+
+    }
+
+    @Override public void incrementCreateExportCount() {
+        createExportCount.incr();
+    }
+
+    @Override public void updateCreateExportTime(long t) {
+        createExportTimeHisto.add(t);
+    }
+
+    @Override public void incrementCreateExportFailureCount() {
+        createExportFailureCount.incr();
+    }
+
+    @Override public void updateCreateExportFailureTime(long t) {
+        createExportFailureTimeHisto.add(t);
+    }
+
+    @Override public void incrementAlterExportCount() {
+        alterExportCount.incr();
+    }
+
+    @Override public void updateAlterExportTime(long t) {
+        alterExportTimeHisto.add(t);
+    }
+
+    @Override public void incrementAlterExportFailureCount() {
+        alterExportFailureCount.incr();
+    }
+
+    @Override public void updateAlterExportFailureTime(long t) {
+        alterExportFailureTimeHisto.add(t);
+    }
+}