You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2022/07/06 15:33:47 UTC
[GitHub] [pulsar] cbornet opened a new pull request, #16423: [fix][functions] Fix netty.DnsResolverUtil compat issue on JDK9+ for the function Runtimes
cbornet opened a new pull request, #16423:
URL: https://github.com/apache/pulsar/pull/16423
<!--
### Contribution Checklist
- PR title format should be *[type][component] summary*. For details, see *[Guideline - Pulsar PR Naming Convention](https://docs.google.com/document/d/1d8Pw6ZbWk-_pCKdOmdvx9rnhPiyuxwq60_TrD68d7BA/edit#heading=h.trs9rsex3xom)*.
- Fill out the template below to describe the changes contributed by the pull request. That will give reviewers the context they need to do the review.
- Each pull request should address only one issue, not mix up code from multiple issues.
- Each commit in the pull request has a meaningful commit message
- Once all items of the checklist are addressed, remove the above text and this checklist, leaving only the filled out template below.
**(The sections below can be removed for hotfixes of typos)**
-->
### Motivation
DnsResolverUtil fails to get existing DNS TTL settings. This happens in Java 17.
This has been fixed for the pulsar-client and pulsar launcher scripts in #15349 and #15540
### Modifications
Detect the java version of the function worker and add the JVM flag `--add-opens java.base/sun.net=ALL-UNNAMED` if the version is 9+
### Documentation
Check the box below or label this PR directly.
Need to update docs?
- [ ] `doc-required`
(Your PR needs to update docs and you will update later)
- [x] `doc-not-needed`
fix
- [ ] `doc`
(Your PR contains doc changes)
- [ ] `doc-complete`
(Docs have been already added)
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] cbornet commented on pull request #16423: [fix][functions] Fix netty.DnsResolverUtil compat issue on JDK9+ for the function Runtimes
Posted by GitBox <gi...@apache.org>.
cbornet commented on PR #16423:
URL: https://github.com/apache/pulsar/pull/16423#issuecomment-1187878918
/pulsarbot rerun-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] cbornet commented on a diff in pull request #16423: [fix][functions] Fix netty.DnsResolverUtil compat issue on JDK9+ for the function Runtimes
Posted by GitBox <gi...@apache.org>.
cbornet commented on code in PR #16423:
URL: https://github.com/apache/pulsar/pull/16423#discussion_r915897199
##########
pulsar-functions/runtime/src/main/java/org/apache/pulsar/functions/runtime/RuntimeUtils.java:
##########
@@ -255,6 +255,10 @@ public static List<String> getGoInstanceCmd(InstanceConfig instanceConfig,
return args;
}
+ public static boolean isJavaVersion9OrMore() {
+ return !System.getProperty("java.version").startsWith("1.");
Review Comment:
done
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] eolivelli commented on a diff in pull request #16423: [fix][functions] Fix netty.DnsResolverUtil compat issue on JDK9+ for the function Runtimes
Posted by GitBox <gi...@apache.org>.
eolivelli commented on code in PR #16423:
URL: https://github.com/apache/pulsar/pull/16423#discussion_r915623276
##########
pulsar-functions/runtime/src/main/java/org/apache/pulsar/functions/runtime/RuntimeUtils.java:
##########
@@ -320,6 +320,11 @@ public static List<String> getCmd(InstanceConfig instanceConfig,
args.add("-Dio.netty.tryReflectionSetAccessible=true");
+ // Needed for netty.DnsResolverUtil on JDK9+
+ if (!System.getProperty("java.version").startsWith("1.")) {
Review Comment:
we should have a consistent approach across the code base, do you see other cases in Java code where we detect the Java version ?
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] dlg99 commented on a diff in pull request #16423: [fix][functions] Fix netty.DnsResolverUtil compat issue on JDK9+ for the function Runtimes
Posted by GitBox <gi...@apache.org>.
dlg99 commented on code in PR #16423:
URL: https://github.com/apache/pulsar/pull/16423#discussion_r915028907
##########
pulsar-functions/runtime/src/main/java/org/apache/pulsar/functions/runtime/RuntimeUtils.java:
##########
@@ -320,6 +320,11 @@ public static List<String> getCmd(InstanceConfig instanceConfig,
args.add("-Dio.netty.tryReflectionSetAccessible=true");
+ // Needed for netty.DnsResolverUtil on JDK9+
+ if (!System.getProperty("java.version").startsWith("1.")) {
+ args.add("--add-opens java.base/sun.net=ALL-UNNAMED");
Review Comment:
Do you want to add other modules too? at least look at the changes that resulted in these and see if they are applicable to the functions
```
$ ggrep -rI "\-\-add-opens"
buildtools/pom.xml: --add-opens java.base/jdk.internal.loader=ALL-UNNAMED
buildtools/pom.xml: --add-opens java.base/java.lang=ALL-UNNAMED <!--Mockito-->
buildtools/dependency-reduced-pom.xml: <test.additional.args>--add-opens java.base/jdk.internal.loader=ALL-UNNAMED
buildtools/dependency-reduced-pom.xml: --add-opens java.base/java.lang=ALL-UNNAMED</test.additional.args>
bin/bookkeeper:# Start --add-opens options
bin/bookkeeper:# '--add-opens' option is not supported in jdk8
bin/bookkeeper: OPTS="$OPTS --add-opens java.base/java.io=ALL-UNNAMED --add-opens java.base/java.util.zip=ALL-UNNAMED"
bin/bookkeeper: OPTS="$OPTS --add-opens java.base/java.nio=ALL-UNNAMED --add-opens java.base/jdk.internal.misc=ALL-UNNAMED"
bin/function-localrunner:# Start --add-opens options
bin/function-localrunner:# '--add-opens' option is not supported in jdk8
bin/function-localrunner: OPTS="$OPTS --add-opens java.base/java.nio=ALL-UNNAMED --add-opens java.base/jdk.internal.misc=ALL-UNNAMED"
bin/pulsar:# Start --add-opens options
bin/pulsar:# '--add-opens' option is not supported in jdk8
bin/pulsar: OPTS="$OPTS --add-opens java.base/java.io=ALL-UNNAMED --add-opens java.base/java.util.zip=ALL-UNNAMED"
bin/pulsar: OPTS="$OPTS --add-opens java.base/java.nio=ALL-UNNAMED --add-opens java.base/jdk.internal.misc=ALL-UNNAMED"
bin/pulsar: OPTS="$OPTS --add-opens java.base/sun.net=ALL-UNNAMED --add-opens java.management/sun.management=ALL-UNNAMED"
bin/pulsar-client:# Start --add-opens options
bin/pulsar-client:# '--add-opens' option is not supported in jdk8
bin/pulsar-client: OPTS="$OPTS --add-opens java.base/sun.net=ALL-UNNAMED"
pom.xml: --add-opens java.base/jdk.internal.loader=ALL-UNNAMED
pom.xml: --add-opens java.base/java.lang=ALL-UNNAMED <!--Mockito-->
pom.xml: --add-opens java.base/java.io=ALL-UNNAMED <!--Bookkeeper NativeIO -->
pom.xml: --add-opens java.base/sun.net=ALL-UNNAMED <!--netty.DnsResolverUtil-->
pom.xml: --add-opens java.management/sun.management=ALL-UNNAMED <!--JvmDefaultGCMetricsLogger-->
```
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] cbornet commented on pull request #16423: [fix][functions] Fix netty.DnsResolverUtil compat issue on JDK9+ for the function Runtimes
Posted by GitBox <gi...@apache.org>.
cbornet commented on PR #16423:
URL: https://github.com/apache/pulsar/pull/16423#issuecomment-1178929781
/pulsarbot rerun-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] eolivelli commented on a diff in pull request #16423: [fix][functions] Fix netty.DnsResolverUtil compat issue on JDK9+ for the function Runtimes
Posted by GitBox <gi...@apache.org>.
eolivelli commented on code in PR #16423:
URL: https://github.com/apache/pulsar/pull/16423#discussion_r914987486
##########
pulsar-functions/runtime/src/main/java/org/apache/pulsar/functions/runtime/RuntimeUtils.java:
##########
@@ -320,6 +320,11 @@ public static List<String> getCmd(InstanceConfig instanceConfig,
args.add("-Dio.netty.tryReflectionSetAccessible=true");
+ // Needed for netty.DnsResolverUtil on JDK9+
+ if (!System.getProperty("java.version").startsWith("1.")) {
Review Comment:
and Java 100 ? :-)
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] cbornet commented on pull request #16423: [fix][functions] Fix netty.DnsResolverUtil compat issue on JDK9+ for the function Runtimes
Posted by GitBox <gi...@apache.org>.
cbornet commented on PR #16423:
URL: https://github.com/apache/pulsar/pull/16423#issuecomment-1186155848
/pulsarbot rerun-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] nicoloboschi commented on a diff in pull request #16423: [fix][functions] Fix netty.DnsResolverUtil compat issue on JDK9+ for the function Runtimes
Posted by GitBox <gi...@apache.org>.
nicoloboschi commented on code in PR #16423:
URL: https://github.com/apache/pulsar/pull/16423#discussion_r915553488
##########
pulsar-functions/runtime/src/main/java/org/apache/pulsar/functions/runtime/RuntimeUtils.java:
##########
@@ -320,6 +320,11 @@ public static List<String> getCmd(InstanceConfig instanceConfig,
args.add("-Dio.netty.tryReflectionSetAccessible=true");
+ // Needed for netty.DnsResolverUtil on JDK9+
+ if (!System.getProperty("java.version").startsWith("1.")) {
Review Comment:
what about using commons-lang3 utility
`SystemUtils.isJavaVersionAtLeast(JavaVersion.JAVA_9)` ?
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] nicoloboschi commented on a diff in pull request #16423: [fix][functions] Fix netty.DnsResolverUtil compat issue on JDK9+ for the function Runtimes
Posted by GitBox <gi...@apache.org>.
nicoloboschi commented on code in PR #16423:
URL: https://github.com/apache/pulsar/pull/16423#discussion_r915840413
##########
pulsar-functions/runtime/src/main/java/org/apache/pulsar/functions/runtime/RuntimeUtils.java:
##########
@@ -255,6 +255,10 @@ public static List<String> getGoInstanceCmd(InstanceConfig instanceConfig,
return args;
}
+ public static boolean isJavaVersion9OrMore() {
+ return !System.getProperty("java.version").startsWith("1.");
Review Comment:
I suggest to use commons-lang3 to detect the version
https://github.com/apache/pulsar/pull/16423#discussion_r915553488
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] cbornet commented on a diff in pull request #16423: [fix][functions] Fix netty.DnsResolverUtil compat issue on JDK9+ for the function Runtimes
Posted by GitBox <gi...@apache.org>.
cbornet commented on code in PR #16423:
URL: https://github.com/apache/pulsar/pull/16423#discussion_r915874241
##########
pulsar-functions/runtime/src/main/java/org/apache/pulsar/functions/runtime/RuntimeUtils.java:
##########
@@ -255,6 +255,10 @@ public static List<String> getGoInstanceCmd(InstanceConfig instanceConfig,
return args;
}
+ public static boolean isJavaVersion9OrMore() {
+ return !System.getProperty("java.version").startsWith("1.");
Review Comment:
👍
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] cbornet commented on pull request #16423: [fix][functions] Fix netty.DnsResolverUtil compat issue on JDK9+ for the function Runtimes
Posted by GitBox <gi...@apache.org>.
cbornet commented on PR #16423:
URL: https://github.com/apache/pulsar/pull/16423#issuecomment-1177822297
/pulsarbot rerun-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] cbornet commented on pull request #16423: [fix][functions] Fix netty.DnsResolverUtil compat issue on JDK9+ for the function Runtimes
Posted by GitBox <gi...@apache.org>.
cbornet commented on PR #16423:
URL: https://github.com/apache/pulsar/pull/16423#issuecomment-1178720257
/pulsarbot rerun-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] cbornet commented on a diff in pull request #16423: [fix][functions] Fix netty.DnsResolverUtil compat issue on JDK9+ for the function Runtimes
Posted by GitBox <gi...@apache.org>.
cbornet commented on code in PR #16423:
URL: https://github.com/apache/pulsar/pull/16423#discussion_r914998760
##########
pulsar-functions/runtime/src/main/java/org/apache/pulsar/functions/runtime/RuntimeUtils.java:
##########
@@ -320,6 +320,11 @@ public static List<String> getCmd(InstanceConfig instanceConfig,
args.add("-Dio.netty.tryReflectionSetAccessible=true");
+ // Needed for netty.DnsResolverUtil on JDK9+
+ if (!System.getProperty("java.version").startsWith("1.")) {
Review Comment:
It will not start with `1.` I think.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] nicoloboschi commented on a diff in pull request #16423: [fix][functions] Fix netty.DnsResolverUtil compat issue on JDK9+ for the function Runtimes
Posted by GitBox <gi...@apache.org>.
nicoloboschi commented on code in PR #16423:
URL: https://github.com/apache/pulsar/pull/16423#discussion_r915551317
##########
pulsar-functions/runtime/src/main/java/org/apache/pulsar/functions/runtime/RuntimeUtils.java:
##########
@@ -320,6 +320,11 @@ public static List<String> getCmd(InstanceConfig instanceConfig,
args.add("-Dio.netty.tryReflectionSetAccessible=true");
+ // Needed for netty.DnsResolverUtil on JDK9+
+ if (!System.getProperty("java.version").startsWith("1.")) {
+ args.add("--add-opens java.base/sun.net=ALL-UNNAMED");
Review Comment:
`java.base/sun.net`should be enough for Pulsar client since it's the same configuration used in pulsar-client CLI tool.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] cbornet commented on a diff in pull request #16423: [fix][functions] Fix netty.DnsResolverUtil compat issue on JDK9+ for the function Runtimes
Posted by GitBox <gi...@apache.org>.
cbornet commented on code in PR #16423:
URL: https://github.com/apache/pulsar/pull/16423#discussion_r915771978
##########
pulsar-functions/runtime/src/main/java/org/apache/pulsar/functions/runtime/RuntimeUtils.java:
##########
@@ -320,6 +320,11 @@ public static List<String> getCmd(InstanceConfig instanceConfig,
args.add("-Dio.netty.tryReflectionSetAccessible=true");
+ // Needed for netty.DnsResolverUtil on JDK9+
+ if (!System.getProperty("java.version").startsWith("1.")) {
Review Comment:
No. That's the only place I found
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] eolivelli merged pull request #16423: [fix][functions] Fix netty.DnsResolverUtil compat issue on JDK9+ for the function Runtimes
Posted by GitBox <gi...@apache.org>.
eolivelli merged PR #16423:
URL: https://github.com/apache/pulsar/pull/16423
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org