Modularize Elasticsearch#81066
Merged
ChrisHegarty merged 374 commits intoelastic:masterfrom May 20, 2022
Merged
Conversation
93 tasks
465f53b to
fac1aee
Compare
b4efc31 to
31a3cf9
Compare
f343277 to
0b81308
Compare
110772d to
4d1127d
Compare
Contributor
Author
|
@elasticmachine retest this please |
4d1127d to
22b3977
Compare
4dca7c9 to
558753b
Compare
558753b to
e24572a
Compare
rjernst
approved these changes
May 19, 2022
Member
rjernst
left a comment
There was a problem hiding this comment.
This looks good.
A couple high level thoughts that can be followups:
- It's difficult to picture the hierarchy of loaders (it was already the case, but I think more so now). It might be useful to have some high level docs on the PluginService describing all the layers and loaders that we create.
- We have the plugin loader creation already separated out to a separate jar (and separate permission), but this change now adds those permissions (get/create) also to server. Long term I'm hoping we can move plugin loading out of server into it's own isolated module (so that we can add eg asm to do some fancier dynamic wrapping of server's interaction with each plugin). Since the PluginService is now directly manipulating the Module of server, I'm not sure that is possible. Not a huge deal, just something to think about as we move forward, how we might achieve separation of the plugin service from server.
...-internal/src/main/java/org/elasticsearch/gradle/internal/ElasticsearchJavaModulePlugin.java
Outdated
Show resolved
Hide resolved
...-internal/src/main/java/org/elasticsearch/gradle/internal/ElasticsearchJavaModulePlugin.java
Outdated
Show resolved
Hide resolved
...main/java/org/elasticsearch/gradle/internal/InternalDistributionModuleCheckTaskProvider.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/plugins/PluginsService.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/plugins/PluginsService.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/plugins/PluginsService.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/plugins/PluginsService.java
Outdated
Show resolved
Hide resolved
Contributor
Author
Excellent points. I will address them in a follow up PR. |
elasticsearchmachine
pushed a commit
that referenced
this pull request
May 23, 2022
This is a follow up to #81066 which ensures that we track the module path as a input on Java compilation tasks. Since we truncate the compile classpath and pass the module path as separate CLI arguments we need to track those separately. We do that by simply annotating a getter on the `CommandLineArgumentProvider`.
ywangd
added a commit
to ywangd/elasticsearch
that referenced
this pull request
May 24, 2022
org.apache.logging.log4j.core is referenced by the LoggingAuditTrail class. This PR adds it to the requires list of the security module. Relates: elastic#81066
elasticsearchmachine
pushed a commit
that referenced
this pull request
May 24, 2022
org.apache.logging.log4j.core is referenced in the LoggingAuditTrail class. This PR adds it to the requires list of the security module. Relates: #81066
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR represents the initial phase of Modularizing Elasticsearch (with
Java Modules).
This initial phase modularizes the core of the Elasticsearch server
with Java Modules, which is then used to load and configure extension
components atop the server. Only a subset of extension components are
modularized at this stage (other components come in a later phase).
Components are loaded dynamically at runtime with custom class loaders
(same as is currently done). Components with a module-info.class are
defined to a module layer.
This architecture is somewhat akin to the Modular JDK, where
applications run on the classpath. In the analogy, the Elasticsearch
server modules are the platform (thus are always resolved and present),
while components without a module-info.class are non-modular code
running atop the Elasticsearch server modules. The extension components
cannot access types from non-exported packages of the server modules, in
the same way that classpath applications cannot access types from
non-exported packages of modules from the JDK. Broadly, the core
Elasticseach java modules simply "wrap" the existing packages and export
them. There are opportunites to export less, which is best done in more
narrowly focused follow-up PRs.
The Elasticsearch distribution startup scripts are updated to put jars
on the module path (the class path is empty), so the distribution will
run the core of the server as java modules. A number of key components
have been retrofitted with module-info.java's too, and the remaining
components can follow later. Unit and functional tests run as
non-modular (since they commonly require package-private access), while
higher-level integration tests, that run the distribution, run as
modular.
relates #78744