From 76e758753be4c817618c5e371e99cbd52fb09a46 Mon Sep 17 00:00:00 2001 From: Jihwan Lee Date: Wed, 15 May 2024 23:10:37 +0900 Subject: [PATCH 1/3] [HUDI-7763] Fix that multiple jmx reporter can exist if metadata enables --- .../hudi/metrics/TestHoodieJmxMetrics.java | 54 +++++++++++++++++++ .../hudi/metrics/JmxMetricsReporter.java | 31 +++++++---- 2 files changed, 76 insertions(+), 9 deletions(-) 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 9daebd086619..931690f8bb01 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 @@ -22,6 +22,7 @@ import org.apache.hudi.common.testutils.NetworkTestUtils; import org.apache.hudi.config.HoodieWriteConfig; import org.apache.hudi.config.metrics.HoodieMetricsConfig; +import org.apache.hudi.exception.HoodieException; import org.apache.hudi.storage.StorageConfiguration; import org.junit.jupiter.api.AfterEach; @@ -34,6 +35,8 @@ import java.util.UUID; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.Mockito.clearInvocations; import static org.mockito.Mockito.when; /** @@ -80,4 +83,55 @@ public void testRegisterGaugeByRangerPort() { assertEquals("123", metrics.getRegistry().getGauges() .get("jmx_metric2").getValue().toString()); } + + @Test + public void testMultipleJmxReporterServer() { + String ports = "9889-9890"; + clearInvocations(metricsConfig); + when(metricsConfig.getMetricsReporterType()).thenReturn(MetricsReporterType.JMX); + when(metricsConfig.getJmxHost()).thenReturn("localhost"); + when(metricsConfig.getJmxPort()).thenReturn(ports); + when(metricsConfig.getBasePath()).thenReturn("s3://test" + UUID.randomUUID()); + hoodieMetrics = new HoodieMetrics(writeConfig, storageConf); + metrics = hoodieMetrics.getMetrics(); + + clearInvocations(metricsConfig); + when(metricsConfig.getMetricsReporterType()).thenReturn(MetricsReporterType.JMX); + when(metricsConfig.getJmxHost()).thenReturn("localhost"); + when(metricsConfig.getJmxPort()).thenReturn(ports); + when(metricsConfig.getBasePath()).thenReturn("s3://test2" + UUID.randomUUID()); + + hoodieMetrics = new HoodieMetrics(writeConfig, storageConf); + Metrics metrics2 = hoodieMetrics.getMetrics(); + + metrics.registerGauge("jmx_metric3", 123L); + assertEquals("123", metrics.getRegistry().getGauges() + .get("jmx_metric3").getValue().toString()); + + metrics2.registerGauge("jmx_metric4", 123L); + assertEquals("123", metrics2.getRegistry().getGauges() + .get("jmx_metric4").getValue().toString()); + } + + @Test + public void testMultipleJmxReporterServerFailedForOnePort() { + String ports = "9891"; + clearInvocations(metricsConfig); + when(metricsConfig.getMetricsReporterType()).thenReturn(MetricsReporterType.JMX); + when(metricsConfig.getJmxHost()).thenReturn("localhost"); + when(metricsConfig.getJmxPort()).thenReturn(ports); + when(metricsConfig.getBasePath()).thenReturn("s3://test" + UUID.randomUUID()); + hoodieMetrics = new HoodieMetrics(writeConfig, storageConf); + metrics = hoodieMetrics.getMetrics(); + + clearInvocations(metricsConfig); + when(metricsConfig.getMetricsReporterType()).thenReturn(MetricsReporterType.JMX); + when(metricsConfig.getJmxHost()).thenReturn("localhost"); + when(metricsConfig.getJmxPort()).thenReturn(ports); + when(metricsConfig.getBasePath()).thenReturn("s3://test2" + UUID.randomUUID()); + + assertThrows(HoodieException.class, () -> { + hoodieMetrics = new HoodieMetrics(writeConfig, storageConf); + }); + } } diff --git a/hudi-common/src/main/java/org/apache/hudi/metrics/JmxMetricsReporter.java b/hudi-common/src/main/java/org/apache/hudi/metrics/JmxMetricsReporter.java index b341fc356f1d..1b2e92d97a4b 100644 --- a/hudi-common/src/main/java/org/apache/hudi/metrics/JmxMetricsReporter.java +++ b/hudi-common/src/main/java/org/apache/hudi/metrics/JmxMetricsReporter.java @@ -53,16 +53,13 @@ public JmxMetricsReporter(HoodieMetricsConfig config, MetricRegistry registry) { host, portsConfig)); } int[] ports = getPortRangeFromString(portsConfig); - boolean successfullyStartedServer = false; - for (int port : ports) { - jmxReporterServer = createJmxReport(host, port); - LOG.info("Started JMX server on port " + port + "."); - successfullyStartedServer = true; - break; - } + initializeJmxReporterServer(host, ports); + + boolean successfullyStartedServer = isServerCreated(); if (!successfullyStartedServer) { throw new HoodieException( - "Could not start JMX server on any configured port. Ports: " + portsConfig); + "Could not start JMX server on any configured port. Ports: " + portsConfig + + ". Maybe require port range for multiple hoodie tables"); } LOG.info("Configured JMXReporter with {port:" + portsConfig + "}"); } catch (Exception e) { @@ -72,9 +69,25 @@ public JmxMetricsReporter(HoodieMetricsConfig config, MetricRegistry registry) { } } + private boolean isServerCreated() { + return jmxReporterServer != null; + } + + private void initializeJmxReporterServer(String host, int[] ports) { + for (int port : ports) { + try { + jmxReporterServer = createJmxReport(host, port); + LOG.info("Started JMX server on port " + port + "."); + break; + } catch (Exception e) { + LOG.info("Skip for initializing jmx port " + port + " because of already in use"); + } + } + } + @Override public void start() { - if (jmxReporterServer != null) { + if (isServerCreated()) { jmxReporterServer.start(); } else { LOG.error("Cannot start as the jmxReporter is null."); From c7b402870d78e079662a5d810f7484e39dc20f83 Mon Sep 17 00:00:00 2001 From: Jihwan Lee Date: Thu, 16 May 2024 11:29:43 +0900 Subject: [PATCH 2/3] [HUDI-7763] apply comment - catch exception that caused by already in use port --- .../java/org/apache/hudi/metrics/JmxMetricsReporter.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/hudi-common/src/main/java/org/apache/hudi/metrics/JmxMetricsReporter.java b/hudi-common/src/main/java/org/apache/hudi/metrics/JmxMetricsReporter.java index 1b2e92d97a4b..b56a41888ecb 100644 --- a/hudi-common/src/main/java/org/apache/hudi/metrics/JmxMetricsReporter.java +++ b/hudi-common/src/main/java/org/apache/hudi/metrics/JmxMetricsReporter.java @@ -28,6 +28,7 @@ import javax.management.MBeanServer; import java.lang.management.ManagementFactory; +import java.rmi.server.ExportException; import java.util.Objects; import java.util.stream.IntStream; @@ -80,7 +81,11 @@ private void initializeJmxReporterServer(String host, int[] ports) { LOG.info("Started JMX server on port " + port + "."); break; } catch (Exception e) { - LOG.info("Skip for initializing jmx port " + port + " because of already in use"); + if (e.getCause() instanceof ExportException) { + LOG.info("Skip for initializing jmx port " + port + " because of already in use"); + } else { + LOG.info("Failed to initialize jmx port " + port + ". " + e.getMessage()); + } } } } From ea65bd2789ac8019af2783c4f80f19846352f978 Mon Sep 17 00:00:00 2001 From: Jihwan Lee Date: Wed, 26 Jun 2024 15:44:36 +0900 Subject: [PATCH 3/3] [HUDI-7763] Fix test --- .../org/apache/hudi/metrics/TestHoodieJmxMetrics.java | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) 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 e2fc87a3d2c0..80f42e934219 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 @@ -23,7 +23,6 @@ import org.apache.hudi.config.HoodieWriteConfig; import org.apache.hudi.config.metrics.HoodieMetricsConfig; import org.apache.hudi.exception.HoodieException; -import org.apache.hudi.storage.StorageConfiguration; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; @@ -91,7 +90,7 @@ public void testMultipleJmxReporterServer() { when(metricsConfig.getJmxHost()).thenReturn("localhost"); when(metricsConfig.getJmxPort()).thenReturn(ports); when(metricsConfig.getBasePath()).thenReturn("s3://test" + UUID.randomUUID()); - hoodieMetrics = new HoodieMetrics(writeConfig, storageConf); + hoodieMetrics = new HoodieMetrics(writeConfig, HoodieTestUtils.getDefaultStorage()); metrics = hoodieMetrics.getMetrics(); clearInvocations(metricsConfig); @@ -100,7 +99,7 @@ public void testMultipleJmxReporterServer() { when(metricsConfig.getJmxPort()).thenReturn(ports); when(metricsConfig.getBasePath()).thenReturn("s3://test2" + UUID.randomUUID()); - hoodieMetrics = new HoodieMetrics(writeConfig, storageConf); + hoodieMetrics = new HoodieMetrics(writeConfig, HoodieTestUtils.getDefaultStorage()); Metrics metrics2 = hoodieMetrics.getMetrics(); metrics.registerGauge("jmx_metric3", 123L); @@ -120,7 +119,7 @@ public void testMultipleJmxReporterServerFailedForOnePort() { when(metricsConfig.getJmxHost()).thenReturn("localhost"); when(metricsConfig.getJmxPort()).thenReturn(ports); when(metricsConfig.getBasePath()).thenReturn("s3://test" + UUID.randomUUID()); - hoodieMetrics = new HoodieMetrics(writeConfig, storageConf); + hoodieMetrics = new HoodieMetrics(writeConfig, HoodieTestUtils.getDefaultStorage()); metrics = hoodieMetrics.getMetrics(); clearInvocations(metricsConfig); @@ -130,7 +129,7 @@ public void testMultipleJmxReporterServerFailedForOnePort() { when(metricsConfig.getBasePath()).thenReturn("s3://test2" + UUID.randomUUID()); assertThrows(HoodieException.class, () -> { - hoodieMetrics = new HoodieMetrics(writeConfig, storageConf); + hoodieMetrics = new HoodieMetrics(writeConfig, HoodieTestUtils.getDefaultStorage()); }); } }