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

Recognise direct buffers in heap size docs #42070

Merged
merged 5 commits into from
May 10, 2019

Conversation

DaveCTurner
Copy link
Contributor

This commit slightly reworks the recommendations in the docs about setting the
heap size:

  • the "rules of thumb" are actually instructions that should be followed

  • the reason for setting Xmx to 50% of the heap size is more subtle than just
    leaving space for the filesystem cache

  • it is normal to see Elasticsearch using more memory than Xmx

  • replace cutoff and limit with threshold since all three terms are used
    interchangeably

  • since we recommend setting Xmx equal to Xms, avoid talking about setting
    Xmx in isolation

Relates #41954

This commit slightly reworks the recommendations in the docs about setting the
heap size:

* the "rules of thumb" are actually instructions that should be followed

* the reason for setting `Xmx` to 50% of the heap size is more subtle than just
  leaving space for the filesystem cache

* it is normal to see Elasticsearch using more memory than `Xmx`

* replace `cutoff` and `limit` with `threshold` since all three terms are used
  interchangeably

* since we recommend setting `Xmx` equal to `Xms`, avoid talking about setting
  `Xmx` in isolation

Relates elastic#41954
@DaveCTurner DaveCTurner added >docs General docs changes :Core/Infra/Resiliency Keep running when everything is ok. Die quickly if things go horribly wrong. v8.0.0 v7.2.0 labels May 10, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

Copy link
Member

@jasontedor jasontedor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left some comments.

@DaveCTurner
Copy link
Contributor Author

Build failed for mysterious possibly-infrastructural reasons. @elasticmachine please run elasticsearch-ci/docbldesx.

Copy link
Member

@jasontedor jasontedor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left one more comment at your discretion. Otherwise LGTM.


heap address: 0x0000000118400000, size: 28672 MB, Compressed Oops with base: 0x00000001183ff000
--

The more heap available to {es}, the more memory it can use for its internal
caches, but the less memory it leaves available for the operating system to use
for the filesystem cache. Also, larger heaps can sometimes cause longer garbage
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should leave "sometimes" out here (the sentence is already indefinite by saying "can" and "sometimes" makes the sentence too weak IMO).

@jasontedor
Copy link
Member

@elasticmachine run elasticsearch-ci/docbldesx

@DaveCTurner DaveCTurner merged commit e56d557 into elastic:master May 10, 2019
@DaveCTurner DaveCTurner deleted the 2019-05-10-heap-size-docs branch May 10, 2019 12:55
DaveCTurner added a commit that referenced this pull request May 10, 2019
This commit slightly reworks the recommendations in the docs about setting the
heap size:

* the "rules of thumb" are actually instructions that should be followed

* the reason for setting `Xmx` to 50% of the heap size is more subtle than just
  leaving space for the filesystem cache

* it is normal to see Elasticsearch using more memory than `Xmx`

* replace `cutoff` and `limit` with `threshold` since all three terms are used
  interchangeably

* since we recommend setting `Xmx` equal to `Xms`, avoid talking about setting
  `Xmx` in isolation

Relates #41954
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request May 10, 2019
* elastic/master: (84 commits)
  [ML] adds geo_centroid aggregation support to data frames (elastic#42088)
  Add documentation for calendar/fixed intervals (elastic#41919)
  Remove global checkpoint assertion in peer recovery (elastic#41987)
  Don't create tempdir for cli scripts (elastic#41913)
  Fix debian-8 update (elastic#42056)
  Cleanup plugin bin directories (elastic#41907)
  Prevent order being lost for _nodes API filters (elastic#42045)
  Change IndexAnalyzers default analyzer access (elastic#42011)
  Remove reference to fs.data.spins in docs
  Mute failing AsyncTwoPhaseIndexerTests
  Remove close method in PageCacheRecycler/Recycler (elastic#41917)
  [ML] adding pivot.max_search_page_size option for setting paging size (elastic#41920)
  Docs: Tweak list formatting
  Simplify handling of keyword field normalizers (elastic#42002)
  [ML] properly nesting objects in document source (elastic#41901)
  Remove extra `ms` from log message (elastic#42068)
  Increase the sample space for random inner hits name generator (elastic#42057)
  Recognise direct buffers in heap size docs (elastic#42070)
  shouldRollGeneration should execute under read lock (elastic#41696)
  Wait for active shard after close in mixed cluster (elastic#42029)
  ...
gurkankaymak pushed a commit to gurkankaymak/elasticsearch that referenced this pull request May 27, 2019
This commit slightly reworks the recommendations in the docs about setting the
heap size:

* the "rules of thumb" are actually instructions that should be followed

* the reason for setting `Xmx` to 50% of the heap size is more subtle than just
  leaving space for the filesystem cache

* it is normal to see Elasticsearch using more memory than `Xmx`

* replace `cutoff` and `limit` with `threshold` since all three terms are used
  interchangeably

* since we recommend setting `Xmx` equal to `Xms`, avoid talking about setting
  `Xmx` in isolation

Relates elastic#41954
@cjcjameson
Copy link
Contributor

@DaveCTurner @jasontedor thank you for this!

When I read the diff of the docs, it made me feel like there was a difference in guidance about setting Xms and Xmx to the same value. However, reading the commit message, I see that it was just text-reorganization.

Can you remind me the reasons and severity of "should" in You should set these two settings to be equal to each other ...? We are running Elasticsearch on shared hardware with other data analytics software. Our other java processes let the JVM vertically scale based on memory pressure; my colleagues say that Elasticsearch should be flexible in the same way.

Is it "strongly should" in the typical deployment of Elasticsearch, where it's the only / the dominant application on a particular machine; but more flexible in more complex shared-usage situations? Or is the intensity of "should" the same for multi-tenant and dedicated deployments both?

@DaveCTurner
Copy link
Contributor Author

@cjcjameson would you ask this question over on the discussion forum? It's a good question, but this isn't the right place to answer it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Core/Infra/Resiliency Keep running when everything is ok. Die quickly if things go horribly wrong. >docs General docs changes v7.2.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants