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

Use our provided JNA library, versus one installed on the system #11163

Merged
merged 1 commit into from
May 14, 2015

Conversation

rmuir
Copy link
Contributor

@rmuir rmuir commented May 14, 2015

The JNA provided by the system might be older/incompatible and not work.

It happened easily with my system openjdk because some package installed an older JNA .so (from version 3). This means things like mlockall don't work with current master and that JDK because of "dll hell".

By specifying -Djna.nosys=true, we always use the bundled native library over anything else that happens to be on the system. In most cases this is just the same logic already happening, unless you have DLL hell.

See docs in the source code: https://github.com/twall/jna/blob/master/src/com/sun/jna/Native.java#L64-L77

Only potential downside is as described in those docs: users with special security configurations might have to configure this differently. But they have to do special configuration for JNA to work anyway! On the other hand, it will work out of box better in the general case, where there is a conflict.

Tested on linux (including my JDK with conflicting JNA library), mac, and windows.

@kimchy
Copy link
Member

kimchy commented May 14, 2015

looks great!, I can't imagine how long it too to find it...

rmuir added a commit that referenced this pull request May 14, 2015
Use our provided JNA library, versus one installed on the system
@rmuir rmuir merged commit 1d3a8ad into elastic:master May 14, 2015
@rmuir rmuir mentioned this pull request May 15, 2015
@jimmyjones2
Copy link
Contributor

Won't this break systems with noexec /tmp? These already don't work out of the box, but at least the library can be extracted and moved into a library path, whereas this will require another undocumented step. Or even better, could the ES package already have the library extracted out under the ES directory hierarchy?

While noexec /tmp isn't the default in any mainstream distro, it's security 101 to not allow code to be executed from /tmp so is reasonably common.

@rmuir
Copy link
Contributor Author

rmuir commented May 19, 2015

Right, as I described in my comment that is the tradeoff here. I think its far more common that someone would have a conflict with a system JNA. Someone with a special security config can just change the sysprop to do what they like.

@rmuir
Copy link
Contributor Author

rmuir commented May 19, 2015

Or even better, could the ES package already have the library extracted out under the ES directory hierarchy?

Just set JAVA_OPTS="-Djava.io.tmpdir=/personal/preference" for this. Or set it via the other mechanisms in your OS: TMPDIR, TEMP, TMP, whatever it respects depending. We don't hijack this in any way and just respect what you configure in the JVM/OS environment.

@rmuir
Copy link
Contributor Author

rmuir commented May 19, 2015

By the way, there is also a separate system property jna.tmpdir just for unpacking to a temporary place. So you can set JAVA_OPTS="-Djna.tmpdir=/personal/preference" to somewhere, if you want fine-grained control just over that.

Again, this was all done before, so its totally unrelated to this PR. Unpacking could always potentially happen, the problem was conflicts from so-called system JNAs which we now ignore by default.

A pull request that gave jna.tmpdir a different default would be reasonable IMO, if we think it improves things out of box. It seems a lot less extreme than hijacking all temporary file configuration from what the user expects!

@clintongormley clintongormley added >enhancement :Delivery/Packaging RPM and deb packaging, tar and zip archives, shell and batch scripts labels Jun 8, 2015
@jlecour
Copy link
Contributor

jlecour commented Feb 17, 2016

Hi,

I hope I'm not off-topic, but i've found this issue when looking for a solution to an impossible startup of Elasticsearch on a Debian server with a /tmp mounted with noexec and a /usr/share/elasticsearch mounted with ro.

Your comments helped me. I've added this line in /etc/default/elasticsearch :

ES_JAVA_OPTS="-Djava.io.tmpdir=/personal/preference"

I was thinking that ES might be able to detect that TMPDIR is ro or noexec and use a directory inside it's DATADIR.

In the meantime, I've changed my java options like this :

ES_JAVA_OPTS="-Djava.io.tmpdir=${DATA_DIR}/tmp"

and it works great.

Also @rmuir suggested to simply change the TMPDIR environment variable, but I didn't find how to do this in the /etc/default/elasticsearch or /etc/init.d/elasqticsearch` files. Maybe some guidance could be added there.

Thanks anyway for the great work on ES. I love using it.

@nik9000
Copy link
Member

nik9000 commented Feb 17, 2016

Also @rmuir suggested to simply change the TMPDIR environment variable

Lots of us are used to running Elasticsearch from the tar distribution so such things are simpler. I suspect you'd want to change the systemd service definition for elasticsearch to set that environment variable if you are using the package. Or the init script.

I was thinking that ES might be able to detect that TMPDIR is ro or noexec and use a directory inside it's DATADIR.

We probably could catch the write failures but DATA_DIR/tmp feels a bit hacky. If this is a common problem I think we should do something, but if its super uncommon just having this github issue come up in google searches might be enough.

@rmuir
Copy link
Contributor Author

rmuir commented Feb 17, 2016

We probably could catch the write failures but DATA_DIR/tmp feels a bit hacky.

This is not possible, because elasticsearch is overconfigurable. java.io.tmpdir must be supplied as a -D to the jvm initially. It cannot be set with System.setProperty after the fact.

hence this path cannot be configurable in es.yml. that is why i suspect it will not get fixed.

@rmuir
Copy link
Contributor Author

rmuir commented Feb 17, 2016

#14372

@mark-vieira mark-vieira added the Team:Delivery Meta label for Delivery team label Nov 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Delivery/Packaging RPM and deb packaging, tar and zip archives, shell and batch scripts >enhancement Team:Delivery Meta label for Delivery team v2.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants