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

Remove setVmMaxMapCount from Elasticsearch CRD #1839

Merged
merged 7 commits into from
Oct 3, 2019

Conversation

anyasabo
Copy link
Contributor

@anyasabo anyasabo commented Oct 1, 2019

Resolves #1712

  • Removes init container option in spec
  • Updates quickstart to disable mmap
  • Adds docs on how to add one manually

The doc build is failing locally and I'm not sure what I'm missing, will need to troubleshoot in the future

Copy link
Contributor

@sebgl sebgl left a comment

Choose a reason for hiding this comment

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

We can also remove pkg/controller/elasticsearch/initcontainer/os_settings.go.
There's also a reference in docs/openshift.asciidoc.
Maybe we could include the init container bit in one of our samples? (the Elasticsearch one?)

@@ -171,16 +171,37 @@ spec:
=== Virtual memory

By default, Elasticsearch uses memory mapping (`mmap`) to efficiently access indices.
Usually, default values for virtual address space on Linux distributions are too low for Elasticsearch to work properly, which may result in out-of-memory exceptions.
To increase virtual memory, ECK sets the recommended value by default.
Usually, default values for virtual address space on Linux distributions are too low for Elasticsearch to work properly, which may result in out-of-memory exceptions. This is why the quickstart example disables `mmap` via the `node.store.allow_mmap: false` setting. For production workloads, it is strongly recommended to increase the kernel setting `vm.max_map_count` to `2621441` and leave `node.store.allow_mmap` unset.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you want to link back to the 'quickstart' here, while it is abundantly clear for people working on ECK what is meant, it might not be clear for other readers.

@anyasabo
Copy link
Contributor Author

anyasabo commented Oct 2, 2019

Maybe we could include the init container bit in one of our samples? (the Elasticsearch one?)

Do you mean this part: https://github.com/elastic/cloud-on-k8s/pull/1839/files#diff-a131bd60e1a908bde08e6409a55c9415R195

or something else?

@sebgl
Copy link
Contributor

sebgl commented Oct 2, 2019

@anyasabo anyasabo marked this pull request as ready for review October 2, 2019 15:03
EOF
----

The operator automatically manages Pods and resources corresponding to the desired cluster. It may take up to a few minutes until the cluster is ready.

Note that `node.store.allow_mmap: false` has performance implications and should be tuned for production workloads as described link:k8s-elasticsearch-spec.html#virtual-memory[here].
Copy link
Collaborator

Choose a reason for hiding this comment

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

Link is broken. I believe this has to be k8s-virtual-memory.html

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That part I wasn't sure about, so was trying to get a doc build locally and failing. Will add some notes in a readme about how to build docs locally once I get it figured out

@pebrc pebrc merged commit bb501e9 into elastic:master Oct 3, 2019
@pebrc pebrc changed the title Remove vmmaxmapcount Remove setVmMaxMapCount from Elasticsearch CRD Oct 9, 2019
@thbkrkr thbkrkr mentioned this pull request Oct 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move setVMMaxMapCount from top-level in spec
3 participants