Introduce jvm.options.d for customizing JVM options #51882
Conversation
This commit introduces the ability to override JVM options by adding custom JVM options files to a jvm.options.d directory. This simplifies administration of Elasticsearch by not requiring administrators to keep the root jvm.options file in sync with changes that we make to the root jvm.options file. Instead, they are not expected to modify this file but instead supply their own in jvm.options.d. In Docker installations, this means they can bind mount this directory in. In future versions of Elasticsearch, we can consider removing the root jvm.options file (instead, providing all options there as system JVM options).
jasontedor
added
>enhancement
:Delivery/Packaging
|
Pinging @elastic/es-core-infra (:Core/Infra/Packaging) |
| <<docker, Docker distribution of {es}>> you can bind mount these files into | ||
| `/usr/share/elasticsearch/config/jvm.options.d/`. You should never need to | ||
| modify the root `jvm.options` file instead preferring to use custom JVM options | ||
| files. Custom JVM options files are processed in lexicographic order (although |
There was a problem hiding this comment.
This last sentence seems to be contradicting itself. Do we use lexicographic order, or is this non-deterministic? Lexicographic order is pretty strictly defined. If we are just iterating over a directory list (which it seems we are) then we shouldn't imply any kind of ordering guarantees. FWIW, it would be trivial for us to actually perform a sort here, which would allow folks to do stuff like 10_options and 00_options for layering overrides in a reliable way.
There was a problem hiding this comment.
We do sort them, see https://github.com/elastic/elasticsearch/pull/51882/files#diff-38516abb6f177c355f29699955e7e60eR68 in the implementation. However, this relies on the Path#compareTo implementation which explicitly says:
* Compares two abstract paths lexicographically. The ordering defined by
* this method is provider specific, and in the case of the default
* provider, platform specific. This method does not access the file system
* and neither file is required to exist.
"Provider" here refers to the filesystem provider. I am assuming (but did not investigate) this enables to account for odd sorting behavior such as on Windows via StrCmpLogicalW which sorts filenames by like f-1, f-2, f-10 in the order f-1, f-2, f-10 instead of f-1, f-10, f-2.
There was a problem hiding this comment.
I'm inclined to say that if this differs across providers we should even mention that it's lexicographic or not, or even perform the sort for that matter. If we do want to order, we should use a JDK implementation that isn't filesystem dependent, such as simply doing a String ordering using the filename. Which I think is effectively going to be what you described in your example.
Co-Authored-By: Mark Vieira <portugee@gmail.com>
|
@elasticmachine update branch |
|
@elasticmachine update branch |
...bution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/JvmOptionsParser.java
Outdated
Show resolved
Hide resolved
...bution/tools/launchers/src/main/java/org/elasticsearch/tools/launchers/JvmOptionsParser.java
Outdated
Show resolved
Hide resolved
qa/os/src/test/java/org/elasticsearch/packaging/test/PackageTests.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
I'm in two minds about whether we should include a Docker packaging test here. On the one hand, the new tests you've already written should cover it - on the other hand, this issue partly arose from a Docker user. If you felt that adding a Docker test was prudent, you could look at test070BindMountCustomPathConfAndJvmOptions as a reference.
|
I've merged #51867 by the way, which tweaked a few Docker-related docs. |
|
@elasticmachine run elasticsearch-ci/1 |
This commit introduces the ability to override JVM options by adding custom JVM options files to a jvm.options.d directory. This simplifies administration of Elasticsearch by not requiring administrators to keep the root jvm.options file in sync with changes that we make to the root jvm.options file. Instead, they are not expected to modify this file but instead supply their own in jvm.options.d. In Docker installations, this means they can bind mount this directory in. In future versions of Elasticsearch, we can consider removing the root jvm.options file (instead, providing all options there as system JVM options).
Relates: elastic/elasticsearch#51882 With the introduction of jvm.options.d directory for customizing jvm.options, the config directory path is now passed to jvm.options parser tool instead of the jvm.options path. See the elasticsearch.bat change for comparison: https://github.com/elastic/elasticsearch/pull/51882/files#diff-6c06e8a0d0742dc053b8afcbfb2b4201R76 Fixes #363
Relates: elastic/elasticsearch#51882 With the introduction of jvm.options.d directory for customizing jvm.options, the config directory path is now passed to jvm.options parser tool instead of the jvm.options path. See the elasticsearch.bat change for comparison: https://github.com/elastic/elasticsearch/pull/51882/files#diff-6c06e8a0d0742dc053b8afcbfb2b4201R76 Fixes #363
Relates: elastic/elasticsearch#51882 With the introduction of jvm.options.d directory for customizing jvm.options, the config directory path is now passed to jvm.options parser tool instead of the jvm.options path. See the elasticsearch.bat change for comparison: https://github.com/elastic/elasticsearch/pull/51882/files#diff-6c06e8a0d0742dc053b8afcbfb2b4201R76 Fixes #363 (cherry picked from commit 31f27ce)
Relates: elastic/elasticsearch#51882 With the introduction of jvm.options.d directory for customizing jvm.options, the config directory path is now passed to jvm.options parser tool instead of the jvm.options path. See the elasticsearch.bat change for comparison: https://github.com/elastic/elasticsearch/pull/51882/files#diff-6c06e8a0d0742dc053b8afcbfb2b4201R76 Fixes #363 (cherry picked from commit 31f27ce)
Relates: elastic/elasticsearch#51882 With the introduction of jvm.options.d directory for customizing jvm.options, the config directory path is now passed to jvm.options parser tool instead of the jvm.options path. See the elasticsearch.bat change for comparison: https://github.com/elastic/elasticsearch/pull/51882/files#diff-6c06e8a0d0742dc053b8afcbfb2b4201R76 Fixes #363 (cherry picked from commit 31f27ce)
Since elastic#51882 we recommend not editing the `jvm.options` file, preferring instead to override its contents with additional files in `jvm.options.d`. However the inline comments in this file do not point users in that direction. This commit adjusts these inline comments.
This commit moves JVM options that we are setting on behalf of the user that we do not expect them to fiddle with out of the jvm.options configuration file and into the JVM options parser. In this way, we discourage fiddling with these settings, but more importantly, we ensure that as we evolve or add to these settings that a user would pick these pick instead of being left behind if they have a modified jvm.options file and do not pick any new that come with the distribution.
Since #51882 we recommend not editing the `jvm.options` file, preferring instead to override its contents with additional files in `jvm.options.d`. However the inline comments in this file do not point users in that direction. This commit adjusts these inline comments.
Since #51882 we recommend not editing the `jvm.options` file, preferring instead to override its contents with additional files in `jvm.options.d`. However the inline comments in this file do not point users in that direction. This commit adjusts these inline comments.
This commit introduces the ability to override JVM options by adding custom JVM options files to a jvm.options.d directory. This simplifies administration of Elasticsearch by not requiring administrators to keep the root jvm.options file in sync with changes that we make to the root jvm.options file. Instead, they are not expected to modify this file but instead supply their own in jvm.options.d. In Docker installations, this means they can bind mount this directory in. In future versions of Elasticsearch, we can consider removing the root jvm.options file (instead, providing all options there as system JVM options).
Relates #51626