From 60ad75a6d225126c9eec38c76c44cc809ccaed5f Mon Sep 17 00:00:00 2001 From: tison Date: Sun, 28 May 2023 08:46:24 +0800 Subject: [PATCH 1/4] chore: refactor code a bit Signed-off-by: tison --- .../common/util/netty/DnsResolverUtil.java | 6 ++---- .../common/util/netty/DnsResolverTest.java | 21 ++++++++++++------- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/pulsar-common/src/main/java/org/apache/pulsar/common/util/netty/DnsResolverUtil.java b/pulsar-common/src/main/java/org/apache/pulsar/common/util/netty/DnsResolverUtil.java index f49a6453c72b3..bc460b8adb027 100644 --- a/pulsar-common/src/main/java/org/apache/pulsar/common/util/netty/DnsResolverUtil.java +++ b/pulsar-common/src/main/java/org/apache/pulsar/common/util/netty/DnsResolverUtil.java @@ -21,9 +21,11 @@ import io.netty.resolver.dns.DnsNameResolverBuilder; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; +import lombok.experimental.UtilityClass; import lombok.extern.slf4j.Slf4j; @Slf4j +@UtilityClass public class DnsResolverUtil { private static final int MIN_TTL = 0; private static final int TTL; @@ -54,10 +56,6 @@ public class DnsResolverUtil { NEGATIVE_TTL = negativeTtl < 0 ? DEFAULT_NEGATIVE_TTL : negativeTtl; } - private DnsResolverUtil() { - // utility class with static methods, prevent instantiation - } - /** * Configure Netty's {@link DnsNameResolverBuilder}'s ttl and negativeTtl to match the JDK's DNS caching settings. * If the JDK setting for TTL is forever (-1), the TTL will be set to 60 seconds. diff --git a/pulsar-common/src/test/java/org/apache/pulsar/common/util/netty/DnsResolverTest.java b/pulsar-common/src/test/java/org/apache/pulsar/common/util/netty/DnsResolverTest.java index 0ccb960e79887..1dc60f918e2c1 100644 --- a/pulsar-common/src/test/java/org/apache/pulsar/common/util/netty/DnsResolverTest.java +++ b/pulsar-common/src/test/java/org/apache/pulsar/common/util/netty/DnsResolverTest.java @@ -18,24 +18,29 @@ */ package org.apache.pulsar.common.util.netty; +import static org.assertj.core.api.Assertions.assertThat; import io.netty.channel.EventLoop; +import io.netty.resolver.dns.DnsNameResolver; import io.netty.resolver.dns.DnsNameResolverBuilder; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import org.mockito.MockedConstruction; import org.mockito.Mockito; -import org.testng.Assert; import org.testng.annotations.Test; public class DnsResolverTest { @Test - public void testMaxTtl() { - EventLoop eventLoop = Mockito.mock(EventLoop.class); - DnsNameResolverBuilder dnsNameResolverBuilder = new DnsNameResolverBuilder(eventLoop); + public void testMaxTtl() throws Exception { + DnsNameResolverBuilder dnsNameResolverBuilder = new DnsNameResolverBuilder(Mockito.mock(EventLoop.class)); + assertThat(dnsNameResolverBuilder).isNotNull(); DnsResolverUtil.applyJdkDnsCacheSettings(dnsNameResolverBuilder); - // If the maxTtl is <=0, it will throw IllegalArgumentException. - try { + + CountDownLatch latch = new CountDownLatch(1); + try (MockedConstruction ignore = Mockito.mockConstruction( + DnsNameResolver.class, (a, b) -> latch.countDown())) { dnsNameResolverBuilder.build(); - } catch (Exception ex) { - Assert.assertFalse(ex instanceof IllegalArgumentException); + assertThat(latch.await(5, TimeUnit.SECONDS)).isTrue(); } } } From 1d8c33e847ded6c7c681136596dc9453c723435d Mon Sep 17 00:00:00 2001 From: tison Date: Sun, 28 May 2023 09:08:00 +0800 Subject: [PATCH 2/4] chore: consistently IS_JAVA_8 condition Signed-off-by: tison --- bin/bookkeeper | 2 +- bin/function-localrunner | 12 ++++++------ bin/pulsar | 2 +- bin/pulsar-admin-common.sh | 2 +- bin/pulsar-perf | 2 +- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/bin/bookkeeper b/bin/bookkeeper index fb516a98acdc2..0cc07dd49aba5 100755 --- a/bin/bookkeeper +++ b/bin/bookkeeper @@ -168,7 +168,7 @@ OPTS="$OPTS -Dlog4j.configurationFile=`basename $BOOKIE_LOG_CONF`" # Allow Netty to use reflection access OPTS="$OPTS -Dio.netty.tryReflectionSetAccessible=true" -IS_JAVA_8=`$JAVA -version 2>&1 |grep version|grep '"1\.8'` +IS_JAVA_8=$( $JAVA -version 2>&1 | grep version | grep '"1\.8' ) # Start --add-opens options # '--add-opens' option is not supported in jdk8 if [[ -z "$IS_JAVA_8" ]]; then diff --git a/bin/function-localrunner b/bin/function-localrunner index 45a37cb306794..86ed6cf6eab07 100755 --- a/bin/function-localrunner +++ b/bin/function-localrunner @@ -40,13 +40,13 @@ PULSAR_MEM=${PULSAR_MEM:-"-Xmx128m -XX:MaxDirectMemorySize=128m"} PULSAR_GC=${PULSAR_GC:-"-XX:+UseZGC -XX:+PerfDisableSharedMem -XX:+AlwaysPreTouch"} # Garbage collection log. -IS_JAVA_8=`$JAVA -version 2>&1 |grep version|grep '"1\.8'` -# java version has space, use [[ -n $PARAM ]] to judge if variable exists -if [[ -n "$IS_JAVA_8" ]]; then - PULSAR_GC_LOG=${PULSAR_GC_LOG:-"-Xloggc:logs/pulsar_gc_%p.log -XX:+PrintGCDetails -XX:+PrintGCDateStamps -XX:+UseGCLogFileRotation -XX:NumberOfGCLogFiles=10 -XX:GCLogFileSize=20M"} -else -# After jdk 9, gc log param should config like this. Ignoring version less than jdk 8 +IS_JAVA_8=$( $JAVA -version 2>&1 | grep version | grep '"1\.8' ) +if [[ -z "$IS_JAVA_8" ]]; then + # >= JDK 9 PULSAR_GC_LOG=${PULSAR_GC_LOG:-"-Xlog:gc:logs/pulsar_gc_%p.log:time,uptime:filecount=10,filesize=20M"} +else + # == JDK 1.8 + PULSAR_GC_LOG=${PULSAR_GC_LOG:-"-Xloggc:logs/pulsar_gc_%p.log -XX:+PrintGCDetails -XX:+PrintGCDateStamps -XX:+UseGCLogFileRotation -XX:NumberOfGCLogFiles=10 -XX:GCLogFileSize=20M"} fi # Extra options to be passed to the jvm diff --git a/bin/pulsar b/bin/pulsar index e3b22caced52e..20ed1f7f22b0f 100755 --- a/bin/pulsar +++ b/bin/pulsar @@ -291,7 +291,7 @@ OPTS="$OPTS -Dzookeeper.clientTcpKeepAlive=true" # Allow Netty to use reflection access OPTS="$OPTS -Dio.netty.tryReflectionSetAccessible=true" -IS_JAVA_8=`$JAVA -version 2>&1 |grep version|grep '"1\.8'` +IS_JAVA_8=$( $JAVA -version 2>&1 | grep version | grep '"1\.8' ) # Start --add-opens options # '--add-opens' option is not supported in jdk8 if [[ -z "$IS_JAVA_8" ]]; then diff --git a/bin/pulsar-admin-common.sh b/bin/pulsar-admin-common.sh index 8223ac5b3bf24..8aa21c00f634d 100755 --- a/bin/pulsar-admin-common.sh +++ b/bin/pulsar-admin-common.sh @@ -91,7 +91,7 @@ PULSAR_CLASSPATH="`dirname $PULSAR_LOG_CONF`:$PULSAR_CLASSPATH" OPTS="$OPTS -Dlog4j.configurationFile=`basename $PULSAR_LOG_CONF`" OPTS="$OPTS -Djava.net.preferIPv4Stack=true" -IS_JAVA_8=`$JAVA -version 2>&1 |grep version|grep '"1\.8'` +IS_JAVA_8=$( $JAVA -version 2>&1 | grep version | grep '"1\.8' ) # Start --add-opens options # '--add-opens' option is not supported in jdk8 if [[ -z "$IS_JAVA_8" ]]; then diff --git a/bin/pulsar-perf b/bin/pulsar-perf index 47c02bc3d67d5..bdc1dc1ed8b8c 100755 --- a/bin/pulsar-perf +++ b/bin/pulsar-perf @@ -134,7 +134,7 @@ PULSAR_CLASSPATH="$PULSAR_JAR:$PULSAR_CLASSPATH:$PULSAR_EXTRA_CLASSPATH" PULSAR_CLASSPATH="`dirname $PULSAR_LOG_CONF`:$PULSAR_CLASSPATH" OPTS="$OPTS -Dlog4j.configurationFile=`basename $PULSAR_LOG_CONF` -Djava.net.preferIPv4Stack=true" -IS_JAVA_8=`$JAVA -version 2>&1 |grep version|grep '"1\.8'` +IS_JAVA_8=$( $JAVA -version 2>&1 | grep version | grep '"1\.8' ) # Start --add-opens options # '--add-opens' option is not supported in jdk8 if [[ -z "$IS_JAVA_8" ]]; then From 6c4f2d8bd1a76c1412e1727eacfc435c543dcee8 Mon Sep 17 00:00:00 2001 From: tison Date: Sun, 28 May 2023 09:40:02 +0800 Subject: [PATCH 3/4] fix: fulfill add-opens to function-localrunner also Signed-off-by: tison --- bin/function-localrunner | 2 ++ 1 file changed, 2 insertions(+) diff --git a/bin/function-localrunner b/bin/function-localrunner index 86ed6cf6eab07..2e0aa0f6dffe2 100755 --- a/bin/function-localrunner +++ b/bin/function-localrunner @@ -44,6 +44,8 @@ IS_JAVA_8=$( $JAVA -version 2>&1 | grep version | grep '"1\.8' ) if [[ -z "$IS_JAVA_8" ]]; then # >= JDK 9 PULSAR_GC_LOG=${PULSAR_GC_LOG:-"-Xlog:gc:logs/pulsar_gc_%p.log:time,uptime:filecount=10,filesize=20M"} + # '--add-opens' option is not supported in JDK 1.8 + OPTS="$OPTS --add-opens java.base/sun.net=ALL-UNNAMED --add-opens java.base/java.lang=ALL-UNNAMED" else # == JDK 1.8 PULSAR_GC_LOG=${PULSAR_GC_LOG:-"-Xloggc:logs/pulsar_gc_%p.log -XX:+PrintGCDetails -XX:+PrintGCDateStamps -XX:+UseGCLogFileRotation -XX:NumberOfGCLogFiles=10 -XX:GCLogFileSize=20M"} From 34077e6601244e5c2ad4f9173ba2fe8b0f730049 Mon Sep 17 00:00:00 2001 From: tison Date: Mon, 29 May 2023 09:11:06 +0800 Subject: [PATCH 4/4] Revert "chore: refactor code a bit" This reverts commit 60ad75a6d225126c9eec38c76c44cc809ccaed5f. --- .../common/util/netty/DnsResolverUtil.java | 6 ++++-- .../common/util/netty/DnsResolverTest.java | 21 +++++++------------ 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/pulsar-common/src/main/java/org/apache/pulsar/common/util/netty/DnsResolverUtil.java b/pulsar-common/src/main/java/org/apache/pulsar/common/util/netty/DnsResolverUtil.java index bc460b8adb027..f49a6453c72b3 100644 --- a/pulsar-common/src/main/java/org/apache/pulsar/common/util/netty/DnsResolverUtil.java +++ b/pulsar-common/src/main/java/org/apache/pulsar/common/util/netty/DnsResolverUtil.java @@ -21,11 +21,9 @@ import io.netty.resolver.dns.DnsNameResolverBuilder; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; -import lombok.experimental.UtilityClass; import lombok.extern.slf4j.Slf4j; @Slf4j -@UtilityClass public class DnsResolverUtil { private static final int MIN_TTL = 0; private static final int TTL; @@ -56,6 +54,10 @@ public class DnsResolverUtil { NEGATIVE_TTL = negativeTtl < 0 ? DEFAULT_NEGATIVE_TTL : negativeTtl; } + private DnsResolverUtil() { + // utility class with static methods, prevent instantiation + } + /** * Configure Netty's {@link DnsNameResolverBuilder}'s ttl and negativeTtl to match the JDK's DNS caching settings. * If the JDK setting for TTL is forever (-1), the TTL will be set to 60 seconds. diff --git a/pulsar-common/src/test/java/org/apache/pulsar/common/util/netty/DnsResolverTest.java b/pulsar-common/src/test/java/org/apache/pulsar/common/util/netty/DnsResolverTest.java index 1dc60f918e2c1..0ccb960e79887 100644 --- a/pulsar-common/src/test/java/org/apache/pulsar/common/util/netty/DnsResolverTest.java +++ b/pulsar-common/src/test/java/org/apache/pulsar/common/util/netty/DnsResolverTest.java @@ -18,29 +18,24 @@ */ package org.apache.pulsar.common.util.netty; -import static org.assertj.core.api.Assertions.assertThat; import io.netty.channel.EventLoop; -import io.netty.resolver.dns.DnsNameResolver; import io.netty.resolver.dns.DnsNameResolverBuilder; -import java.util.concurrent.CountDownLatch; -import java.util.concurrent.TimeUnit; -import org.mockito.MockedConstruction; import org.mockito.Mockito; +import org.testng.Assert; import org.testng.annotations.Test; public class DnsResolverTest { @Test - public void testMaxTtl() throws Exception { - DnsNameResolverBuilder dnsNameResolverBuilder = new DnsNameResolverBuilder(Mockito.mock(EventLoop.class)); - assertThat(dnsNameResolverBuilder).isNotNull(); + public void testMaxTtl() { + EventLoop eventLoop = Mockito.mock(EventLoop.class); + DnsNameResolverBuilder dnsNameResolverBuilder = new DnsNameResolverBuilder(eventLoop); DnsResolverUtil.applyJdkDnsCacheSettings(dnsNameResolverBuilder); - - CountDownLatch latch = new CountDownLatch(1); - try (MockedConstruction ignore = Mockito.mockConstruction( - DnsNameResolver.class, (a, b) -> latch.countDown())) { + // If the maxTtl is <=0, it will throw IllegalArgumentException. + try { dnsNameResolverBuilder.build(); - assertThat(latch.await(5, TimeUnit.SECONDS)).isTrue(); + } catch (Exception ex) { + Assert.assertFalse(ex instanceof IllegalArgumentException); } } }