Skip to content

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

Merged
eolivelli merged 4 commits into
apache:masterfrom
nicoloboschi:pulsar-bk-native-warning
Apr 2, 2022
Merged

[fix][jdk17] remove illegal access warning and enable reads optimization on jdk17#14999
eolivelli merged 4 commits into
apache:masterfrom
nicoloboschi:pulsar-bk-native-warning

Conversation

@nicoloboschi

@nicoloboschi nicoloboschi commented Apr 1, 2022

Copy link
Copy Markdown
Contributor

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
Comment thread bin/bookkeeper
# 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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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