-
Notifications
You must be signed in to change notification settings - Fork 25k
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
Update heap setting documentation in light of machine dependent heap #66567
Update heap setting documentation in light of machine dependent heap #66567
Conversation
Pinging @elastic/es-docs (Team:Docs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me overall. I left a few comments and suggestions but nothing blocking.
I think creating the Advanced configuration settings
page is a good move. However, we probably want to tell users that they shouldn't mess with these unless they know what they're doing. We get a lot of search traffic, and I could see someone landing here without other context.
It may also be worth auditing some of our xrefs to the current heap size settings. I found one during the review, but there may be others. I can do that as a separate effort if wanted though.
Thanks @mark-vieira!
@@ -0,0 +1,89 @@ | |||
[[advanced-configuration]] | |||
=== Advanced configuration settings | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You may want to add a couple brief sentences before the heap size heading. I'd just note:
- We recommend using default settings in most production use cases
- These settings are for experts only.
If someone lands on this page from Google, they should see "There be dragons here."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added some language here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New copy looks great to me! Thanks!
Co-authored-by: James Rodewig <40268737+jrodewig@users.noreply.github.com>
Co-authored-by: James Rodewig <40268737+jrodewig@users.noreply.github.com>
Co-authored-by: James Rodewig <40268737+jrodewig@users.noreply.github.com>
Co-authored-by: James Rodewig <40268737+jrodewig@users.noreply.github.com>
Co-authored-by: James Rodewig <40268737+jrodewig@users.noreply.github.com>
Co-authored-by: James Rodewig <40268737+jrodewig@users.noreply.github.com>
Co-authored-by: James Rodewig <40268737+jrodewig@users.noreply.github.com>
Co-authored-by: James Rodewig <40268737+jrodewig@users.noreply.github.com>
Co-authored-by: James Rodewig <40268737+jrodewig@users.noreply.github.com>
Thanks for the review @jrodewig! I've applied your suggested and made a couple other minor tweaks. |
Documentation updates to our heap size recommendations as a follow up to #65905.