From 71e14cf866d2ddeee7238a3f15b59912c2537943 Mon Sep 17 00:00:00 2001 From: Xuedong Luan Date: Fri, 23 Jul 2021 19:57:35 +0800 Subject: [PATCH] [HUDI-2213] Remove unnecessary parameter for HoodieMetrics constructor and fix NPE in UT (#3333) --- .../hudi/client/AbstractHoodieWriteClient.java | 2 +- .../org/apache/hudi/metrics/HoodieMetrics.java | 4 ++-- .../main/java/org/apache/hudi/metrics/Metrics.java | 4 +--- .../hudi/metrics/MetricsReporterFactory.java | 14 +++++++------- .../hudi/metrics/TestHoodieConsoleMetrics.java | 2 +- .../apache/hudi/metrics/TestHoodieJmxMetrics.java | 4 ++-- .../org/apache/hudi/metrics/TestHoodieMetrics.java | 3 ++- .../metrics/prometheus/TestPrometheusReporter.java | 3 ++- .../prometheus/TestPushGateWayReporter.java | 3 ++- 9 files changed, 20 insertions(+), 19 deletions(-) diff --git a/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/AbstractHoodieWriteClient.java b/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/AbstractHoodieWriteClient.java index f0a45c5bc..6470cfa93 100644 --- a/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/AbstractHoodieWriteClient.java +++ b/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/AbstractHoodieWriteClient.java @@ -130,7 +130,7 @@ public abstract class AbstractHoodieWriteClient timelineService) { super(context, writeConfig, timelineService); - this.metrics = new HoodieMetrics(config, config.getTableName()); + this.metrics = new HoodieMetrics(config); this.index = createIndex(writeConfig); this.txnManager = new TransactionManager(config, fs); } diff --git a/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/HoodieMetrics.java b/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/HoodieMetrics.java index 5db28e241..67befea42 100644 --- a/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/HoodieMetrics.java +++ b/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/HoodieMetrics.java @@ -54,9 +54,9 @@ public class HoodieMetrics { private Timer clusteringTimer = null; private Timer indexTimer = null; - public HoodieMetrics(HoodieWriteConfig config, String tableName) { + public HoodieMetrics(HoodieWriteConfig config) { this.config = config; - this.tableName = tableName; + this.tableName = config.getTableName(); if (config.isMetricsOn()) { Metrics.init(config); this.rollbackTimerName = getMetricsName("timer", HoodieTimeline.ROLLBACK_ACTION); diff --git a/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/Metrics.java b/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/Metrics.java index 5667a66a5..1e4d83ab5 100644 --- a/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/Metrics.java +++ b/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/Metrics.java @@ -53,9 +53,7 @@ public class Metrics { } reporter.start(); - Runtime.getRuntime().addShutdownHook(new Thread(() -> { - reportAndCloseReporter(); - })); + Runtime.getRuntime().addShutdownHook(new Thread(this::reportAndCloseReporter)); } private void reportAndCloseReporter() { diff --git a/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/MetricsReporterFactory.java b/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/MetricsReporterFactory.java index 66cdeebe9..820c1a3e8 100644 --- a/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/MetricsReporterFactory.java +++ b/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/MetricsReporterFactory.java @@ -41,20 +41,20 @@ public class MetricsReporterFactory { private static final Logger LOG = LogManager.getLogger(MetricsReporterFactory.class); public static MetricsReporter createReporter(HoodieWriteConfig config, MetricRegistry registry) { - MetricsReporterType type = config.getMetricsReporterType(); - MetricsReporter reporter = null; + String reporterClassName = config.getMetricReporterClassName(); - if (!StringUtils.isNullOrEmpty(config.getMetricReporterClassName())) { - Object instance = ReflectionUtils - .loadClass(config.getMetricReporterClassName(), - new Class[] {Properties.class, MetricRegistry.class}, config.getProps(), registry); + if (!StringUtils.isNullOrEmpty(reporterClassName)) { + Object instance = ReflectionUtils.loadClass( + reporterClassName, new Class[] {Properties.class, MetricRegistry.class}, config.getProps(), registry); if (!(instance instanceof AbstractUserDefinedMetricsReporter)) { throw new HoodieException(config.getMetricReporterClassName() - + " is not a subclass of AbstractUserDefinedMetricsReporter"); + + " is not a subclass of AbstractUserDefinedMetricsReporter"); } return (MetricsReporter) instance; } + MetricsReporterType type = config.getMetricsReporterType(); + MetricsReporter reporter = null; switch (type) { case GRAPHITE: reporter = new MetricsGraphiteReporter(config, registry); diff --git a/hudi-client/hudi-client-common/src/test/java/org/apache/hudi/metrics/TestHoodieConsoleMetrics.java b/hudi-client/hudi-client-common/src/test/java/org/apache/hudi/metrics/TestHoodieConsoleMetrics.java index 5db2ef079..467a9f792 100644 --- a/hudi-client/hudi-client-common/src/test/java/org/apache/hudi/metrics/TestHoodieConsoleMetrics.java +++ b/hudi-client/hudi-client-common/src/test/java/org/apache/hudi/metrics/TestHoodieConsoleMetrics.java @@ -41,7 +41,7 @@ public class TestHoodieConsoleMetrics { when(config.getTableName()).thenReturn("console_metrics_test"); when(config.isMetricsOn()).thenReturn(true); when(config.getMetricsReporterType()).thenReturn(MetricsReporterType.CONSOLE); - new HoodieMetrics(config, "raw_table"); + new HoodieMetrics(config); } @AfterEach diff --git a/hudi-client/hudi-client-common/src/test/java/org/apache/hudi/metrics/TestHoodieJmxMetrics.java b/hudi-client/hudi-client-common/src/test/java/org/apache/hudi/metrics/TestHoodieJmxMetrics.java index 8254bf6b1..a752aa36e 100644 --- a/hudi-client/hudi-client-common/src/test/java/org/apache/hudi/metrics/TestHoodieJmxMetrics.java +++ b/hudi-client/hudi-client-common/src/test/java/org/apache/hudi/metrics/TestHoodieJmxMetrics.java @@ -52,7 +52,7 @@ public class TestHoodieJmxMetrics { when(config.getMetricsReporterType()).thenReturn(MetricsReporterType.JMX); when(config.getJmxHost()).thenReturn("localhost"); when(config.getJmxPort()).thenReturn(String.valueOf(NetworkTestUtils.nextFreePort())); - new HoodieMetrics(config, "raw_table"); + new HoodieMetrics(config); registerGauge("jmx_metric1", 123L); assertEquals("123", Metrics.getInstance().getRegistry().getGauges() .get("jmx_metric1").getValue().toString()); @@ -65,7 +65,7 @@ public class TestHoodieJmxMetrics { when(config.getMetricsReporterType()).thenReturn(MetricsReporterType.JMX); when(config.getJmxHost()).thenReturn("localhost"); when(config.getJmxPort()).thenReturn(String.valueOf(NetworkTestUtils.nextFreePort())); - new HoodieMetrics(config, "raw_table"); + new HoodieMetrics(config); registerGauge("jmx_metric2", 123L); assertEquals("123", Metrics.getInstance().getRegistry().getGauges() .get("jmx_metric2").getValue().toString()); diff --git a/hudi-client/hudi-client-common/src/test/java/org/apache/hudi/metrics/TestHoodieMetrics.java b/hudi-client/hudi-client-common/src/test/java/org/apache/hudi/metrics/TestHoodieMetrics.java index a734b8c98..a5ea531c9 100755 --- a/hudi-client/hudi-client-common/src/test/java/org/apache/hudi/metrics/TestHoodieMetrics.java +++ b/hudi-client/hudi-client-common/src/test/java/org/apache/hudi/metrics/TestHoodieMetrics.java @@ -50,8 +50,9 @@ public class TestHoodieMetrics { @BeforeEach void setUp() { when(config.isMetricsOn()).thenReturn(true); + when(config.getTableName()).thenReturn("raw_table"); when(config.getMetricsReporterType()).thenReturn(MetricsReporterType.INMEMORY); - metrics = new HoodieMetrics(config, "raw_table"); + metrics = new HoodieMetrics(config); } @AfterEach diff --git a/hudi-client/hudi-client-common/src/test/java/org/apache/hudi/metrics/prometheus/TestPrometheusReporter.java b/hudi-client/hudi-client-common/src/test/java/org/apache/hudi/metrics/prometheus/TestPrometheusReporter.java index 9010fe56e..79b127165 100644 --- a/hudi-client/hudi-client-common/src/test/java/org/apache/hudi/metrics/prometheus/TestPrometheusReporter.java +++ b/hudi-client/hudi-client-common/src/test/java/org/apache/hudi/metrics/prometheus/TestPrometheusReporter.java @@ -46,10 +46,11 @@ public class TestPrometheusReporter { @Test public void testRegisterGauge() { when(config.isMetricsOn()).thenReturn(true); + when(config.getTableName()).thenReturn("foo"); when(config.getMetricsReporterType()).thenReturn(MetricsReporterType.PROMETHEUS); when(config.getPrometheusPort()).thenReturn(9090); assertDoesNotThrow(() -> { - new HoodieMetrics(config, "raw_table"); + new HoodieMetrics(config); }); } } diff --git a/hudi-client/hudi-client-common/src/test/java/org/apache/hudi/metrics/prometheus/TestPushGateWayReporter.java b/hudi-client/hudi-client-common/src/test/java/org/apache/hudi/metrics/prometheus/TestPushGateWayReporter.java index eb4d09c2b..dcbf72c39 100644 --- a/hudi-client/hudi-client-common/src/test/java/org/apache/hudi/metrics/prometheus/TestPushGateWayReporter.java +++ b/hudi-client/hudi-client-common/src/test/java/org/apache/hudi/metrics/prometheus/TestPushGateWayReporter.java @@ -48,6 +48,7 @@ public class TestPushGateWayReporter { @Test public void testRegisterGauge() { when(config.isMetricsOn()).thenReturn(true); + when(config.getTableName()).thenReturn("foo"); when(config.getMetricsReporterType()).thenReturn(MetricsReporterType.PROMETHEUS_PUSHGATEWAY); when(config.getPushGatewayHost()).thenReturn("localhost"); when(config.getPushGatewayPort()).thenReturn(9091); @@ -57,7 +58,7 @@ public class TestPushGateWayReporter { when(config.getPushGatewayRandomJobNameSuffix()).thenReturn(false); assertDoesNotThrow(() -> { - new HoodieMetrics(config, "raw_table"); + new HoodieMetrics(config); }); registerGauge("pushGateWayReporter_metric", 123L);