Avoid early initializing Netty #19819
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Today when we load the Netty plugins, we indirectly cause several Netty classes to initialize. This is because we attempt to load some classes by name, and loading these classes is done in a way that triggers a long chain of class initializers within Netty. We should not do this, this can lead to log messages before the logger is loader, and it leads to initialization in cases when the classes would never be needed (for example, Netty 3 class initialization is never needed if Netty 4 is used, and vice versa). This commit avoids this early initialization of these classes by explicitly telling the class loader to not initialize.
jasontedor
added
>enhancement
review
:Distributed Coordination/Network
Member
Author
|
Before this change: $ ~/elasticsearch/elasticsearch-5.0.0-alpha5-SNAPSHOT/bin/elasticsearch
[2016-08-05 00:47:23,792][INFO ][node ] [] initializing ...
[2016-08-05 00:47:23,849][INFO ][env ] [ga-klWp] using [1] data paths, mounts [[/ (/dev/disk1)]], net usable_space [334.6gb], net total_space [464.7gb], spins? [unknown], types [hfs]
[2016-08-05 00:47:23,849][INFO ][env ] [ga-klWp] heap size [1.9gb], compressed ordinary object pointers [true]
[2016-08-05 00:47:23,850][INFO ][node ] [ga-klWp] node name [ga-klWp] derived from node ID; set [node.name] to override
[2016-08-05 00:47:23,851][INFO ][node ] [ga-klWp] version[5.0.0-alpha5-SNAPSHOT], pid[5231], build[3f6a3c0/2016-08-05T02:34:42.860Z], OS[Mac OS X/10.11.6/x86_64], JVM[Oracle Corporation/Java HotSpot(TM) 64-Bit Server VM/1.8.0_92/25.92-b14]
[2016-08-05 00:47:24,536][INFO ][io.netty.util.internal.PlatformDependent] Your platform does not provide complete low-level API for accessing direct buffers reliably. Unless explicitly requested, heap buffer will always be preferred to avoid potential system instability.After this change: $ ± ~/elasticsearch/elasticsearch-5.0.0-alpha5-SNAPSHOT/bin/elasticsearch
[2016-08-05 00:46:15,181][INFO ][node ] [] initializing ...
[2016-08-05 00:46:15,240][INFO ][env ] [ga-klWp] using [1] data paths, mounts [[/ (/dev/disk1)]], net usable_space [334.6gb], net total_space [464.7gb], spins? [unknown], types [hfs]
[2016-08-05 00:46:15,241][INFO ][env ] [ga-klWp] heap size [1.9gb], compressed ordinary object pointers [true]
[2016-08-05 00:46:15,242][INFO ][node ] [ga-klWp] node name [ga-klWp] derived from node ID; set [node.name] to override
[2016-08-05 00:46:15,243][INFO ][node ] [ga-klWp] version[5.0.0-alpha5-SNAPSHOT], pid[5058], build[3f6a3c0/2016-08-05T02:34:42.860Z], OS[Mac OS X/10.11.6/x86_64], JVM[Oracle Corporation/Java HotSpot(TM) 64-Bit Server VM/1.8.0_92/25.92-b14]
[2016-08-05 00:46:15,873][INFO ][plugins ] [ga-klWp] loaded module [aggs-matrix-stats]
[2016-08-05 00:46:15,873][INFO ][plugins ] [ga-klWp] loaded module [ingest-common]
[2016-08-05 00:46:15,873][INFO ][plugins ] [ga-klWp] loaded module [lang-expression]
[2016-08-05 00:46:15,874][INFO ][plugins ] [ga-klWp] loaded module [lang-groovy]
[2016-08-05 00:46:15,874][INFO ][plugins ] [ga-klWp] loaded module [lang-mustache]
[2016-08-05 00:46:15,874][INFO ][plugins ] [ga-klWp] loaded module [lang-painless]
[2016-08-05 00:46:15,874][INFO ][plugins ] [ga-klWp] loaded module [percolator]
[2016-08-05 00:46:15,874][INFO ][plugins ] [ga-klWp] loaded module [reindex]
[2016-08-05 00:46:15,874][INFO ][plugins ] [ga-klWp] loaded module [transport-netty3]
[2016-08-05 00:46:15,874][INFO ][plugins ] [ga-klWp] loaded module [transport-netty4]
[2016-08-05 00:46:15,874][INFO ][plugins ] [ga-klWp] no plugins loaded
[2016-08-05 00:46:17,486][INFO ][node ] [ga-klWp] initialized
[2016-08-05 00:46:17,487][INFO ][node ] [ga-klWp] starting ...
[2016-08-05 00:46:17,521][INFO ][netty.util.internal.PlatformDependent] Your platform does not provide complete low-level API for accessing direct buffers reliably. Unless explicitly requested, heap buffer will always be preferred to avoid potential system instability.Note the placement and the package name on the scary logging statement (will be removed after #19786 and netty/netty#5624). |
Today we early initialize Netty components in a privileged block. This is because these components set a system property that avoids a rare race condition deep inside the JVM. However, this bug has been fixed since JDK 7. Since we depend on JDK 8, we no longer need to rely on this property being set. Netty will still try to set this system property, but will fail because of a missing privileged block on the property write. This is okay since the bug is fixed in the versions of the JDK that we depend on.
This commit fixes the compilation of ReindexFromRemoteWithAuthTests.java which resulted from removing a class yet not updating this source file after doing so.
* master: Update to Jackson 2.8.1 Upon being elected as master, prefer joins' node info to existing cluster state (#19743) Remove offset rounding Remove TimeZoneRounding abstraction Remove unused rounding code
Contributor
|
LGTM |
This commit adds a missing suppress forbidden annotation in Netty3Utils as otherwise setting sun.nio.ch.bugLevel there will lead to a build-time failure.
This commit fixes a typo in a comment in Netty3Utils; the comment was missing a key negation.
This commit moves a suppress forbidden annotation to the method that needs the suppression. The issue is that the violating method is defined in an anonymous class and the annotation was applied to the method containing the anonymous class definition but not the method that is actually violating the forbidden APIs.
This commit removes a test that is setting netty.assert.buglevel but that setting has been removed.
Member
Author
|
Thanks for reviewing @s1monw. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Today when we load the Netty plugins, we indirectly cause several Netty
classes to initialize. This is because we attempt to load some classes
by name, and loading these classes is done in a way that triggers a long
chain of class initializers within Netty. We should not do this, this
can lead to log messages before the logger is loader, and it leads to
initialization in cases when the classes would never be needed (for
example, Netty 3 class initialization is never needed if Netty 4 is
used, and vice versa). This commit avoids this early initialization of
these classes by removing the need for the early loading.
Relates netty/netty#5644