Clarify usage of jvm.options in its comments
#61456
Conversation
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.
DaveCTurner
added
>enhancement
:Delivery/Packaging
|
Pinging @elastic/es-core-infra (:Core/Infra/Packaging) |
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.
distribution/src/config/jvm.options
Outdated
| ## All settings below here are considered expert settings. Do | ||
| ## not tamper with them unless you understand what you are | ||
| ## doing. If you want to adjust any of these settings then do | ||
| ## not do so by editing this file; instead, create a new file in |
There was a problem hiding this comment.
The wording here is a bit confusing (when I first read it, I think it is do so, my mind accidentally skipped over the first "do not"). Could we simplify the first clause to simply If you want to adjust any of these settings then do not edit this file;?
There was a problem hiding this comment.
Good point, the line break ended up in a bad place there. I tried a few ideas and ended up copying the wording from the earlier section, see cd1b171.
| ## should create one or more files in the jvm.options.d | ||
| ## directory containing your adjustments. | ||
| ## | ||
| ## See https://www.elastic.co/guide/en/elasticsearch/reference/current/jvm-options.html |
There was a problem hiding this comment.
I think we want to avoid a link here, since it could easily break in the future. It also will link users to the wrong version for their version. For example, linking to current would redirect a user to 8.0.0 when that's released, but maybe they're on 7.9 and we've made breaking changes.
There was a problem hiding this comment.
I'm not too worried about breaking this link, the docs team put quite some effort into avoiding broken links to current docs in future versions since it's so harmful to the searchability of our docs. Note that there's prior art: we already include a link to the current heap size docs further down this file.
I couldn't see a simple way to link to the correct version of these docs today, although this would be a nice feature to have.
There was a problem hiding this comment.
In the spirit of progress over perfection I merged this PR and opened #65808 regarding this comment.
(edit: fixed link)
| # Xms represents the initial size of total heap space | ||
| # Xmx represents the maximum size of total heap space | ||
| # Xms represents the initial size of the JVM heap | ||
| # Xmx represents the maximum size of the JVM heap | ||
|
|
||
| -Xms${heap.min} | ||
| -Xmx${heap.max} |
There was a problem hiding this comment.
I wonder if we should take this further and remove this from this file. The way I envision this working is that in JvmOptionsParser, we'd emit a -Xms1g and/or -Xmx1g if Xms (MinHeapSize) or Xmx (MaxHeapSize) are not set respectively.
There was a problem hiding this comment.
Yes; does the same reasoning apply to all the other settings in this file too?
However I don't have the capacity to take this work on myself at the moment.
There was a problem hiding this comment.
I can pick this up - I'll raise a new issue to move the default heap settings.
mark-vieira
added
Team:Delivery
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.
pugnascotia
changed the title
jvm.options in its comments
Since #51882 we recommend not editing the
jvm.optionsfile, preferringinstead to override its contents with additional files in
jvm.options.d. However the inline comments in this file do not pointusers in that direction. This commit adjusts these inline comments.