lhotari commented on code in PR #22329:
URL: https://github.com/apache/pulsar/pull/22329#discussion_r1535644635
##########
pulsar-common/src/test/java/org/apache/pulsar/common/util/netty/DnsResolverTest.java:
##########
@@ -18,13 +18,42 @@
*/
package org.apache.pulsar.common.util.netty;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.testng.Assert.assertEquals;
import io.netty.channel.EventLoop;
import io.netty.resolver.dns.DnsNameResolverBuilder;
+import java.security.Security;
+import org.mockito.ArgumentCaptor;
import org.mockito.Mockito;
import org.testng.Assert;
+import org.testng.annotations.BeforeClass;
import org.testng.annotations.Test;
public class DnsResolverTest {
+ private static final int MIN_TTL = 0;
+ private static final int TTL = 101;
+ private static final int NEGATIVE_TTL = 121;
+
+ @BeforeClass
+ public void beforeClass() {
+ Security.setProperty("networkaddress.cache.ttl",Integer.toString(TTL));
+
Security.setProperty("networkaddress.cache.negative.ttl",Integer.toString(NEGATIVE_TTL));
Review Comment:
Since the test changes these settings, it should recover the previous
settings after the test
##########
pulsar-common/src/main/java/org/apache/pulsar/common/util/netty/DnsResolverUtil.java:
##########
@@ -19,12 +19,19 @@
package org.apache.pulsar.common.util.netty;
import io.netty.resolver.dns.DnsNameResolverBuilder;
-import java.lang.reflect.InvocationTargetException;
-import java.lang.reflect.Method;
+import java.security.Security;
+import java.util.Optional;
import lombok.extern.slf4j.Slf4j;
@Slf4j
public class DnsResolverUtil {
+
+ private static final String CACHE_POLICY_PROP = "networkaddress.cache.ttl";
+ private static final String CACHE_POLICY_PROP_FALLBACK =
"sun.net.inetaddr.ttl";
+ private static final String NEGATIVE_CACHE_POLICY_PROP =
"networkaddress.cache.negative.ttl";
+ private static final String NEGATIVE_CACHE_POLICY_PROP_FALLBACK =
"sun.net.inetaddr.negative.ttl";
+ /* default value for positive lookups */
+ private static final int JDK_DEFAULT_TTL = 30;
Review Comment:
I'm in the understanding that the JDK default is infinite for positive
lookups. Do you have a reference to share that this is 30 seconds?
##########
pulsar-common/src/test/java/org/apache/pulsar/common/util/netty/DnsResolverTest.java:
##########
@@ -18,13 +18,42 @@
*/
package org.apache.pulsar.common.util.netty;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.testng.Assert.assertEquals;
import io.netty.channel.EventLoop;
import io.netty.resolver.dns.DnsNameResolverBuilder;
+import java.security.Security;
+import org.mockito.ArgumentCaptor;
import org.mockito.Mockito;
import org.testng.Assert;
+import org.testng.annotations.BeforeClass;
import org.testng.annotations.Test;
public class DnsResolverTest {
+ private static final int MIN_TTL = 0;
+ private static final int TTL = 101;
+ private static final int NEGATIVE_TTL = 121;
+
+ @BeforeClass
Review Comment:
```suggestion
@BeforeClass(alwaysRun = true)
```
TestNG has some surprising behaviors unless `alwaysRun=true` is used. It
happens when using test groups. Because of that, it's better to always add
`alwaysRun = true to BeforeClass/AfterClass.
##########
pulsar-common/src/main/java/org/apache/pulsar/common/util/netty/DnsResolverUtil.java:
##########
@@ -39,19 +46,35 @@ public class DnsResolverUtil {
int ttl = DEFAULT_TTL;
int negativeTtl = DEFAULT_NEGATIVE_TTL;
try {
- // use reflection to call sun.net.InetAddressCachePolicy's get and
getNegative methods for getting
- // effective JDK settings for DNS caching
- Class<?> inetAddressCachePolicyClass =
Class.forName("sun.net.InetAddressCachePolicy");
- Method getTTLMethod = inetAddressCachePolicyClass.getMethod("get");
- ttl = (Integer) getTTLMethod.invoke(null);
- Method getNegativeTTLMethod =
inetAddressCachePolicyClass.getMethod("getNegative");
- negativeTtl = (Integer) getNegativeTTLMethod.invoke(null);
- } catch (NoSuchMethodException | ClassNotFoundException |
InvocationTargetException
- | IllegalAccessException e) {
+ String ttlStr = Security.getProperty(CACHE_POLICY_PROP);
+ if (ttlStr == null) {
+ // Compatible with sun.net.inetaddr.ttl settings
+ ttlStr = System.getProperty(CACHE_POLICY_PROP_FALLBACK);
+ }
+ String negativeTtlStr =
Security.getProperty(NEGATIVE_CACHE_POLICY_PROP);
+ if (negativeTtlStr == null) {
+ // Compatible with sun.net.inetaddr.negative.ttl settings
+ negativeTtlStr =
System.getProperty(NEGATIVE_CACHE_POLICY_PROP_FALLBACK);
+ }
+ ttl = Optional.ofNullable(ttlStr)
+ .map(Integer::decode)
+ .filter(i -> i > 0)
+ .orElseGet(() -> {
+ if (System.getSecurityManager() == null) {
+ return JDK_DEFAULT_TTL;
+ }
Review Comment:
Is this necessary? Why?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
