Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[fix][jdk17] remove illegal access warning and enable reads optimization on jdk17 #14999

Merged
merged 4 commits into from
Apr 2, 2022

Conversation

nicoloboschi
Copy link
Contributor

@nicoloboschi nicoloboschi commented Apr 1, 2022

Motivation

In BookKeeper there's an optimization in order to improve file reads on Linux using posix_fadvise native instructions.
In order to do that it needs to access java.io.FileDescriptor using the reflection.

For Netty there is the java.nio.DirectBuffer usage with it's currently used in Pulsar and it should improve the direct memory footprint.

At the moment:

  • if you run the Bookie with jdk11, you see spammy warnings like: Illegal reflective access by io.netty.util.internal.ReflectionUtil (file:/Users/nicolo.boschi/.m2/repository/io/netty/netty-common/4.1.74.Final/netty-common-4.1.74.Final.jar) to constructor java.nio.DirectByteBuffer(long,int)
  • if you run the Bookie with jdk11, the optimization is disabled

This pull is also intended to keep the same behavior across JDK11 and JDK17.

Modifications

  • Add --add-opens java.base/java.io=ALL-UNNAMED --add-opens java.base/sun.nio=ALL-UNNAMED to the command line inside ´bin´ directory and allow reflections on java.io package. This option is available only starting from Java 9.

note: this is not documented on BookKeeper project, I will follow-up there.

  • no-need-doc

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Apr 1, 2022
@nicoloboschi nicoloboschi changed the title [fix][bookie] Bookie: remove illegal access warning and enable reads optimization on jdk17 [fix][jdk17] Bookie: remove illegal access warning and enable reads optimization on jdk17 Apr 1, 2022
@nicoloboschi nicoloboschi changed the title [fix][jdk17] Bookie: remove illegal access warning and enable reads optimization on jdk17 [fix][jdk17] remove illegal access warning and enable reads optimization on jdk17 Apr 1, 2022
@@ -166,6 +166,17 @@ 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'`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about jdk 11.0.1.8?
Will this check fail?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In BK we have another way to detect modern jdks with modules

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand better now, you added a double quote in the beginning so it should match only "1.8

@eolivelli eolivelli merged commit 94cc46a into apache:master Apr 2, 2022
Nicklee007 pushed a commit to Nicklee007/pulsar that referenced this pull request Apr 20, 2022
…ion on jdk17 (apache#14999)

* [fix][bookie] Bookie: remove illegal access warning and enable reads optimization on jdk17
nicoloboschi added a commit to datastax/pulsar that referenced this pull request Jan 18, 2023
…ion on jdk17 (apache#14999)

* [fix][bookie] Bookie: remove illegal access warning and enable reads optimization on jdk17

(cherry picked from commit 94cc46a)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc-not-needed Your PR changes do not impact docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants