-
Notifications
You must be signed in to change notification settings - Fork 24.9k
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
Fixed issues with loading #Build static initializer in OSGi container. #21955
Conversation
…ted runtime container
Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually? |
Thanks for the PR @trajar. I'm sorry though, but we do not support running in an OSGi container. This means that we do not test this use-case at all, which means that we can not provide any guarantees that this behavior works or continues to work in the future. We have a policy about accepting PRs for features/platforms/etc. that we do not support so I'm going to close this PR. |
For the record, I really do think the decision not to integrate this PR is shortsighted. As it stands, this class generates an uncatch-able runtime exception when the library is loaded within a container or protected class loader. It'd be one thing if the code executed during some factory or builder logic, which could be catch and recovered, but the exception is generated in the static initializer which is never good. While I understand need to get version information for ES during runtime, doing it in this manner (in static initialization and trying to read the jar itself during runtime) is not ideal. This PR is not an enhancement for OSGi container support, it's a fix to ensure a less catastrophic failure during static initialization. |
I think that's the point. Elasticsearch is not a library as Lucene is. It's a server as explained in: https://www.elastic.co/blog/elasticsearch-the-server |
@trajar Thank you for your thoughts here. Can you clarify one thing? You say:
Are you suggesting that this might be broken in something like JBoss or Wildfly? This would be disappointing to me and is probably worth fixing (relates #18999). Do you have a reproduction? To be clear, I'm referring to usage of the clients, not of embedding Elasticsearch which we do not support. |
To clarify, yes I'm simply trying to use the Java api to build a transport client and communicate with an ES instance. In an effort to determine if runtime exceptions are thrown in other non-OSGi containers (per your query), I took the 'helloworld' example over at https://github.com/wildfly/quickstart, added ES core and transport libraries, and inserted code to connect via PreBuiltTransportClient to ES instance on localhost. Good news is that the Wildfly container actually returns a non-null URL via #getElasticsearchCodebase logic, unlike the OSGi container which fails. Bad news is that it still throws a runtime exception in the static initialization, specifically a NPE on line Build:49. I presume it's from the chained accessor to #getMainAttributes when trying to pull the shortHash. Once that error hits, no client on subsequent servlet executions can connect because the Builder class is never actually loaded. I've attached the modified 'helloworld' maven project as well as the full server log. Defer to your team, but I'd welcome reconsideration of this PR or similar change that didn't generate a runtime exception in the static block. |
Thank you for doing this @trajar! I will take a look at this very soon and we will come up with something. |
@jasontedor no problem, happy to help! |
I'm sorry with my previous answer. I thought you were trying to embed an elasticsearch node. |
@trajar Can you please post the actual full stacktrace? In the original description it seems there are stack frames missing. I'm trying to figure out what is causing |
@rjernst Of course, can do. See below, which was generated today with recent official 5.3.0 release.
|
Thanks @rjernst, I really appreciate the team taking a look at this and working through a fix. |
This change makes the build info initialization only try to load a jar manifest if it is the elasticsearch jar. Anything else (eg a repackaged ES for use of transport client in an uber jar) will contain "Unknown" for the build info as it does for tests currently. fixes #21955
This change makes the build info initialization only try to load a jar manifest if it is the elasticsearch jar. Anything else (eg a repackaged ES for use of transport client in an uber jar) will contain "Unknown" for the build info as it does for tests currently. fixes #21955
This change makes the build info initialization only try to load a jar manifest if it is the elasticsearch jar. Anything else (eg a repackaged ES for use of transport client in an uber jar) will contain "Unknown" for the build info as it does for tests currently. fixes #21955
This change makes the build info initialization only try to load a jar manifest if it is the elasticsearch jar. Anything else (eg a repackaged ES for use of transport client in an uber jar) will contain "Unknown" for the build info as it does for tests currently. fixes #21955
We are adding tests for this in #24147. |
Fix static initialization runtime exception when running in OSGi or other protected runtime that intercepts calls to URL.openStream. The use case is embedding elasticsearch library in OSGi application for local client. An example of the stack trace:
[INFO] Caused by: java.lang.RuntimeException: java.io.IOException: FakeURLStreamHandler can not be used!
[INFO] at org.elasticsearch.Build.(Build.java:53) ~[?:?]
[INFO] ... 11 more
[INFO] Caused by: java.io.IOException: FakeURLStreamHandler can not be used!
[INFO] at org.apache.felix.framework.FakeURLStreamHandler.openConnection(FakeURLStreamHandler.java:39) ~[felix-4.2.1.jar!/:?]
[INFO] at java.net.URL.openConnection(URL.java:979) ~[?:1.8.0_112]
[INFO] at java.net.URL.openStream(URL.java:1045) ~[?:1.8.0_112]
[INFO] at org.elasticsearch.Build.(Build.java:47) ~[?:?]
[INFO] ... 11 more