Update heap size and compute resources documentation #1785
Conversation
docs/elasticsearch-spec.asciidoc
Outdated
| @@ -84,7 +84,51 @@ spec: | |||
| memory: 4Gi | |||
| ---- | |||
|
|
|||
| For more information, see the Elasticsearch documentation on link:https://www.elastic.co/guide/en/elasticsearch/reference/current/heap-size.html[Setting the heap size]. | |||
|
|
|||
| If no resource constraints are defined in the object specification, the operator applies a default memory `request` of 2Gi based on the assumption that Elasticsearch heap size is set to the default of 1Gi. If the Kubernetes cluster is configured with https://kubernetes.io/docs/tasks/administer-cluster/manage-resources/memory-default-namespace/[LimitRanges] that enforce a minimum memory constraint, they could interfere with the operator default and cause object creation to fail. | |||
There was a problem hiding this comment.
It feels a bit weird to me that we explain our default 2GB request in the "JVM heap size" section. I would move that to "Set custom resources", and here only mention we apply the default 1Gi heap size (with maybe a link to the "Set custom resources" section).
There was a problem hiding this comment.
The custom resources page mentions the default and directs to here:
For Elasticsearch, if no memory request is set in the
podTemplatespec, the operator applies a default memory request as described in the <<{p}-jvm-heap-size>> section.
In fact, whenever Elasticsearch is mentioned, the custom resources page always instructs the users to read the heap size section. I think that's a good way to get people to consider heap sizing and also ensure that we don't repeat things in multiple places. WDYT?
There was a problem hiding this comment.
ensure that we don't repeat things in multiple places
Agreed.
However, if I want to learn about how ECK deals with resources limits and requests, and how does that relate to LimitRanges, I think I would go look at "Managing compute resources". I'd probably be fine since that links to the JVM heap size section as you mentioned. But I think I would prefer things being done the other way around: the explanation about LimitRanges, default requests and limits should appear in the "Managing compute resources" section IMHO. And have the JVM heap size section link to that.
Would it simplify things to have all memory/heap size related stuff on the same single page?
(That's not a strong opinion, just a small preference, still approving the PR as is :))
docs/elasticsearch-spec.asciidoc
Outdated
| @@ -84,7 +84,51 @@ spec: | |||
| memory: 4Gi | |||
| ---- | |||
|
|
|||
| For more information, see the Elasticsearch documentation on link:https://www.elastic.co/guide/en/elasticsearch/reference/current/heap-size.html[Setting the heap size]. | |||
|
|
|||
| If no resource constraints are defined in the object specification, the operator applies a default memory `request` of 2Gi based on the assumption that Elasticsearch heap size is set to the default of 1Gi. If the Kubernetes cluster is configured with https://kubernetes.io/docs/tasks/administer-cluster/manage-resources/memory-default-namespace/[LimitRanges] that enforce a minimum memory constraint, they could interfere with the operator default and cause object creation to fail. | |||
There was a problem hiding this comment.
ensure that we don't repeat things in multiple places
Agreed.
However, if I want to learn about how ECK deals with resources limits and requests, and how does that relate to LimitRanges, I think I would go look at "Managing compute resources". I'd probably be fine since that links to the JVM heap size section as you mentioned. But I think I would prefer things being done the other way around: the explanation about LimitRanges, default requests and limits should appear in the "Managing compute resources" section IMHO. And have the JVM heap size section link to that.
Would it simplify things to have all memory/heap size related stuff on the same single page?
(That's not a strong opinion, just a small preference, still approving the PR as is :))
|
@sebgl Please have another look |
Removes information duplicated between the two sections and emphasises the need to set resource constraints.
Implementation of #1454 will necessitate a few more minor updates to these sections but this change covers the important points.
Part of #1734