You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hive.apache.org by lp...@apache.org on 2022/02/24 08:00:25 UTC

[hive] branch master updated: HIVE-25973: Fix JsonReporter and JsonFileMetricsReporter writing the same file (#3047) (Viktor Csomor, reviewed by Laszlo Pinter)

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

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


The following commit(s) were added to refs/heads/master by this push:
     new af01324  HIVE-25973: Fix JsonReporter and JsonFileMetricsReporter writing the same file (#3047) (Viktor Csomor, reviewed by Laszlo Pinter)
af01324 is described below

commit af013246100be85675d18e6dcfcea7f202bc8d2c
Author: Viktor Csomor <cs...@gmail.com>
AuthorDate: Thu Feb 24 09:00:09 2022 +0100

    HIVE-25973: Fix JsonReporter and JsonFileMetricsReporter writing the same file (#3047) (Viktor Csomor, reviewed by Laszlo Pinter)
---
 .../ql/txn/compactor/TestCompactionMetrics.java    | 25 +++++-----
 .../hadoop/hive/metastore/HiveMetaStore.java       |  3 --
 .../hive/metastore/metrics/AcidMetricService.java  | 58 ++++++++++------------
 3 files changed, 40 insertions(+), 46 deletions(-)

diff --git a/ql/src/test/org/apache/hadoop/hive/ql/txn/compactor/TestCompactionMetrics.java b/ql/src/test/org/apache/hadoop/hive/ql/txn/compactor/TestCompactionMetrics.java
index 07a3212..bde83a5 100644
--- a/ql/src/test/org/apache/hadoop/hive/ql/txn/compactor/TestCompactionMetrics.java
+++ b/ql/src/test/org/apache/hadoop/hive/ql/txn/compactor/TestCompactionMetrics.java
@@ -450,7 +450,7 @@ public class TestCompactionMetrics  extends CompactorTest {
             System.currentTimeMillis(),true, "4.0.0", "4.0.0",40));
 
     scr.setCompacts(elements);
-    AcidMetricService.updateMetricsFromShowCompact(scr, conf);
+    AcidMetricService.updateMetricsFromShowCompact(scr);
 
     Assert.assertEquals(1,
         Metrics.getOrCreateGauge(MetricsConstants.COMPACTION_STATUS_PREFIX +
@@ -487,7 +487,7 @@ public class TestCompactionMetrics  extends CompactorTest {
 
     ShowCompactResponse scr = new ShowCompactResponse();
     scr.setCompacts(elements);
-    AcidMetricService.updateMetricsFromShowCompact(scr, conf);
+    AcidMetricService.updateMetricsFromShowCompact(scr);
 
     // Check that it is not set
     Assert.assertEquals(0, Metrics.getOrCreateGauge(MetricsConstants.COMPACTION_OLDEST_ENQUEUE_AGE).intValue());
@@ -504,7 +504,7 @@ public class TestCompactionMetrics  extends CompactorTest {
     );
 
     scr.setCompacts(elements);
-    AcidMetricService.updateMetricsFromShowCompact(scr, conf);
+    AcidMetricService.updateMetricsFromShowCompact(scr);
     long diff = (System.currentTimeMillis() - start) / 1000;
 
     // Check that we have at least 1s old compaction age, but not more than expected
@@ -524,7 +524,7 @@ public class TestCompactionMetrics  extends CompactorTest {
     );
 
     scr.setCompacts(elements);
-    AcidMetricService.updateMetricsFromShowCompact(scr, conf);
+    AcidMetricService.updateMetricsFromShowCompact(scr);
     long diff = (System.currentTimeMillis() - start) / 1000;
 
     // Check that we have at least 1s old compaction age, but not more than expected
@@ -544,7 +544,7 @@ public class TestCompactionMetrics  extends CompactorTest {
     );
 
     scr.setCompacts(elements);
-    AcidMetricService.updateMetricsFromShowCompact(scr, conf);
+    AcidMetricService.updateMetricsFromShowCompact(scr);
     long diff = (System.currentTimeMillis() - start) / 1000;
 
     // Check that we have at least 1s old compaction age, but not more than expected
@@ -566,7 +566,7 @@ public class TestCompactionMetrics  extends CompactorTest {
     );
 
     scr.setCompacts(elements);
-    AcidMetricService.updateMetricsFromShowCompact(scr, conf);
+    AcidMetricService.updateMetricsFromShowCompact(scr);
     // Check that the age is older than 10s
     Assert.assertTrue(Metrics.getOrCreateGauge(MetricsConstants.COMPACTION_OLDEST_ENQUEUE_AGE).intValue() > 10);
 
@@ -578,7 +578,7 @@ public class TestCompactionMetrics  extends CompactorTest {
             start - 1_000L)
     );
     scr.setCompacts(elements);
-    AcidMetricService.updateMetricsFromShowCompact(scr, conf);
+    AcidMetricService.updateMetricsFromShowCompact(scr);
 
     // Check that the age is older than 20s
     Assert.assertTrue(Metrics.getOrCreateGauge(MetricsConstants.COMPACTION_OLDEST_ENQUEUE_AGE).intValue() > 20);
@@ -597,7 +597,7 @@ public class TestCompactionMetrics  extends CompactorTest {
     );
 
     scr.setCompacts(elements);
-    AcidMetricService.updateMetricsFromShowCompact(scr, conf);
+    AcidMetricService.updateMetricsFromShowCompact(scr);
     // Check that the age is older than 10s
     Assert.assertTrue(Metrics.getOrCreateGauge(MetricsConstants.COMPACTION_OLDEST_WORKING_AGE).intValue() > 10);
 
@@ -609,7 +609,7 @@ public class TestCompactionMetrics  extends CompactorTest {
             start, false, "4.0.0", "4.0.0", start - 1_000L)
     );
     scr.setCompacts(elements);
-    AcidMetricService.updateMetricsFromShowCompact(scr, conf);
+    AcidMetricService.updateMetricsFromShowCompact(scr);
 
     // Check that the age is older than 20s
     Assert.assertTrue(Metrics.getOrCreateGauge(MetricsConstants.COMPACTION_OLDEST_WORKING_AGE).intValue() > 20);
@@ -628,7 +628,7 @@ public class TestCompactionMetrics  extends CompactorTest {
     );
 
     scr.setCompacts(elements);
-    AcidMetricService.updateMetricsFromShowCompact(scr, conf);
+    AcidMetricService.updateMetricsFromShowCompact(scr);
     // Check that the age is older than 10s
     Assert.assertTrue(Metrics.getOrCreateGauge(MetricsConstants.COMPACTION_OLDEST_CLEANING_AGE).intValue() > 10);
 
@@ -640,7 +640,7 @@ public class TestCompactionMetrics  extends CompactorTest {
             start, false, "4.0.0", "4.0.0", -1L, start - 1_000L)
     );
     scr.setCompacts(elements);
-    AcidMetricService.updateMetricsFromShowCompact(scr, conf);
+    AcidMetricService.updateMetricsFromShowCompact(scr);
 
     // Check that the age is older than 20s
     Assert.assertTrue(Metrics.getOrCreateGauge(MetricsConstants.COMPACTION_OLDEST_CLEANING_AGE).intValue() > 20);
@@ -827,7 +827,8 @@ public class TestCompactionMetrics  extends CompactorTest {
     params.put(hive_metastoreConstants.TABLE_NO_AUTO_COMPACT, "true");
     Table disabledTbl = newTable(dbName, "comp_disabled", false, params);
     burnThroughTransactions(disabledTbl.getDbName(), disabledTbl.getTableName(), 1, null, null);
-    burnThroughTransactions(disabledTbl.getDbName(), disabledTbl.getTableName(), 1, null, new HashSet<>(Arrays.asList(2L)));
+    burnThroughTransactions(disabledTbl.getDbName(), disabledTbl.getTableName(), 1, null, new HashSet<>(
+        Collections.singletonList(2L)));
 
     Table enabledTbl = newTable(dbName, "comp_enabled", false);
     burnThroughTransactions(enabledTbl.getDbName(), enabledTbl.getTableName(), 1, null, null);
diff --git a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
index 98952f6..a5845bf 100644
--- a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
+++ b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
@@ -22,7 +22,6 @@ import org.apache.commons.cli.OptionBuilder;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.hive.common.ZKDeRegisterWatcher;
 import org.apache.hadoop.hive.common.ZooKeeperHiveHelper;
-import org.apache.hadoop.hive.common.metrics.common.MetricsFactory;
 import org.apache.hadoop.hive.metastore.api.Database;
 import org.apache.hadoop.hive.metastore.api.MetaException;
 import org.apache.hadoop.hive.metastore.api.ThriftHiveMetastore;
@@ -284,7 +283,6 @@ public class HiveMetaStore extends ThriftHiveMetastore {
         if (MetastoreConf.getBoolVar(conf, ConfVars.METRICS_ENABLED)) {
           try {
             Metrics.shutdown();
-            MetricsFactory.close();
           } catch (Exception e) {
             LOG.error("error in Metrics deinit: " + e.getClass().getName() + " "
                 + e.getMessage(), e);
@@ -306,7 +304,6 @@ public class HiveMetaStore extends ThriftHiveMetastore {
       if (MetastoreConf.getBoolVar(conf, ConfVars.METRICS_ENABLED)) {
         try {
           Metrics.initialize(conf);
-          MetricsFactory.init(conf);
         } catch (Exception e) {
           // log exception, but ignore inability to start
           LOG.error("error in Metrics init: " + e.getClass().getName() + " "
diff --git a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/metrics/AcidMetricService.java b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/metrics/AcidMetricService.java
index 1d57ee8..567fb92 100644
--- a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/metrics/AcidMetricService.java
+++ b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/metrics/AcidMetricService.java
@@ -22,7 +22,6 @@ import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.fs.Path;
 import org.apache.hadoop.hive.common.AcidConstants;
 import org.apache.hadoop.hive.common.metrics.MetricsMBeanImpl;
-import org.apache.hadoop.hive.common.metrics.common.MetricsFactory;
 import org.apache.hadoop.hive.metastore.IMetaStoreClient;
 import org.apache.hadoop.hive.metastore.MetastoreTaskThread;
 import org.apache.hadoop.hive.metastore.api.CompactionMetricsDataRequest;
@@ -263,30 +262,27 @@ public class AcidMetricService implements MetastoreTaskThread {
   }
 
   private void updateDeltaMetrics() {
-    org.apache.hadoop.hive.common.metrics.common.Metrics metrics = MetricsFactory.getInstance();
-    if (metrics != null) {
-      try {
-        LOG.debug("Called reporting task.");
-        List<CompactionMetricsData> deltas = txnHandler.getTopCompactionMetricsDataPerType(maxCacheSize);
-        Map<String, Integer> deltasMap = deltas.stream().filter(d -> d.getMetricType() == NUM_DELTAS).collect(
-            Collectors.toMap(item -> getDeltaCountKey(item.getDbName(), item.getTblName(), item.getPartitionName()),
-                CompactionMetricsData::getMetricValue));
-        updateDeltaMBeanAndMetric(deltaObject, COMPACTION_NUM_DELTAS, deltasMap);
-
-        Map<String, Integer> smallDeltasMap = deltas.stream().filter(d -> d.getMetricType() == NUM_SMALL_DELTAS)
-            .collect(
-                Collectors.toMap(item -> getDeltaCountKey(item.getDbName(), item.getTblName(), item.getPartitionName()),
-                    CompactionMetricsData::getMetricValue));
-        updateDeltaMBeanAndMetric(smallDeltaObject, COMPACTION_NUM_SMALL_DELTAS, smallDeltasMap);
-
-        Map<String, Integer> obsoleteDeltasMap = deltas.stream().filter(d -> d.getMetricType() == NUM_OBSOLETE_DELTAS)
-            .collect(
-                Collectors.toMap(item -> getDeltaCountKey(item.getDbName(), item.getTblName(), item.getPartitionName()),
-                    CompactionMetricsData::getMetricValue));
-        updateDeltaMBeanAndMetric(obsoleteDeltaObject, COMPACTION_NUM_OBSOLETE_DELTAS, obsoleteDeltasMap);
-      } catch (Throwable e) {
-        LOG.warn("Caught exception while trying to fetch compaction metrics from metastore backend db.", e);
-      }
+    try {
+      LOG.debug("Called reporting task.");
+      List<CompactionMetricsData> deltas = txnHandler.getTopCompactionMetricsDataPerType(maxCacheSize);
+      Map<String, Integer> deltasMap = deltas.stream().filter(d -> d.getMetricType() == NUM_DELTAS).collect(
+          Collectors.toMap(item -> getDeltaCountKey(item.getDbName(), item.getTblName(), item.getPartitionName()),
+              CompactionMetricsData::getMetricValue));
+      updateDeltaMBeanAndMetric(deltaObject, COMPACTION_NUM_DELTAS, deltasMap);
+
+      Map<String, Integer> smallDeltasMap = deltas.stream().filter(d -> d.getMetricType() == NUM_SMALL_DELTAS)
+          .collect(
+              Collectors.toMap(item -> getDeltaCountKey(item.getDbName(), item.getTblName(), item.getPartitionName()),
+                  CompactionMetricsData::getMetricValue));
+      updateDeltaMBeanAndMetric(smallDeltaObject, COMPACTION_NUM_SMALL_DELTAS, smallDeltasMap);
+
+      Map<String, Integer> obsoleteDeltasMap = deltas.stream().filter(d -> d.getMetricType() == NUM_OBSOLETE_DELTAS)
+          .collect(
+              Collectors.toMap(item -> getDeltaCountKey(item.getDbName(), item.getTblName(), item.getPartitionName()),
+                  CompactionMetricsData::getMetricValue));
+      updateDeltaMBeanAndMetric(obsoleteDeltaObject, COMPACTION_NUM_OBSOLETE_DELTAS, obsoleteDeltasMap);
+    } catch (Throwable e) {
+      LOG.warn("Caught exception while trying to fetch compaction metrics from metastore backend db.", e);
     }
   }
 
@@ -298,7 +294,7 @@ public class AcidMetricService implements MetastoreTaskThread {
 
   private void updateMetrics() throws MetaException {
     ShowCompactResponse currentCompactions = txnHandler.showCompact(new ShowCompactRequest());
-    updateMetricsFromShowCompact(currentCompactions, conf);
+    updateMetricsFromShowCompact(currentCompactions);
     updateDBMetrics();
   }
 
@@ -322,7 +318,7 @@ public class AcidMetricService implements MetastoreTaskThread {
   }
 
   @VisibleForTesting
-  public static void updateMetricsFromShowCompact(ShowCompactResponse showCompactResponse, Configuration conf) {
+  public static void updateMetricsFromShowCompact(ShowCompactResponse showCompactResponse) {
     CompactionMetricData metricData = CompactionMetricData.of(showCompactResponse.getCompacts());
 
     // Get the current count for each state
@@ -341,9 +337,9 @@ public class AcidMetricService implements MetastoreTaskThread {
       }
     }
 
-    updateOldestCompactionMetric(COMPACTION_OLDEST_ENQUEUE_AGE, metricData.getOldestEnqueueTime(), conf);
-    updateOldestCompactionMetric(COMPACTION_OLDEST_WORKING_AGE, metricData.getOldestWorkingTime(), conf);
-    updateOldestCompactionMetric(COMPACTION_OLDEST_CLEANING_AGE, metricData.getOldestCleaningTime(), conf);
+    updateOldestCompactionMetric(COMPACTION_OLDEST_ENQUEUE_AGE, metricData.getOldestEnqueueTime());
+    updateOldestCompactionMetric(COMPACTION_OLDEST_WORKING_AGE, metricData.getOldestWorkingTime());
+    updateOldestCompactionMetric(COMPACTION_OLDEST_CLEANING_AGE, metricData.getOldestCleaningTime());
 
     Metrics.getOrCreateGauge(COMPACTION_NUM_INITIATORS)
         .set((int) metricData.getInitiatorsCount());
@@ -356,7 +352,7 @@ public class AcidMetricService implements MetastoreTaskThread {
         .set((int) metricData.getWorkerVersionsCount());
   }
 
-  private static void updateOldestCompactionMetric(String metricName, Long oldestTime, Configuration conf) {
+  private static void updateOldestCompactionMetric(String metricName, Long oldestTime) {
     if (oldestTime == null) {
       Metrics.getOrCreateGauge(metricName)
           .set(0);