Proposal: enhanced memory limit settings

[clockworksoul]

Currently memory allocation is defined exclusively using the ES_JAVA_OPTS env property, which on memory constrained systems can lead to crash looping as pods are scheduled onto nodes with less available memory than specified in the JVM options, causing a swift and sometimes vague crash due to insufficient memory.

I propose a single value, set individually for each of the els node types, that sets both the ES_JAVA_OPTS property and a resource request value:

env:
- name: ES_JAVA_OPTS
  value: "-Xms{{ .Values.data.requests.memory }}m -Xmx{{ .Values.data.requests.memory }}m"

and

resources:
    requests:
        memory: {{ .Values.data.requests.memory }}Mi

Thoughts?

[mikn]

I would propose keeping it as two different ones, if only because they have different size-types. I would personally just put them together in the values.yaml and comment on that requests.memory should be the same as heapSize.

[clockworksoul]

they have different size-types.

@mikn Fair point about decimal vs binary. I'll whip this up with two distinct values, including commentary about decimal vs. binary memory notation and that the values should still generally be the same.

How do you feel about something like this for a values.yaml?

data:
  requests:
    heapMemory: 256m
    containerMemory: 256Mi

This feels a little awkward to me though, since heap memory and container memory requests a very different things but this arrangement implies they're similar. Thoughts?

[clockworksoul]

Just found this in https://kubernetes.io/docs/concepts/configuration/manage-compute-resources-container/#meaning-of-memory

Limits and requests for memory are measured in bytes. You can express memory as a plain integer or as a fixed-point integer using one of these suffixes: E, P, T, G, M, k. You can also use the power-of-two equivalents: Ei, Pi, Ti, Gi, Mi, Ki. For example, the following represent roughly the same value:

So we can use consistent memory types, as long as they're both base 10. I'll use this mode.

[mikn]

Oh, so, what I meant was that java heap size m == Mi for resources in kubernetes. I always dislike omitting one/two characters for such a thing, it means that you cannot change the notation from MiB and you cannot set the requests higher than the heap size either.

Note that the JVM uses more memory than just the heap. For example Java methods, thread stacks and native handles are allocated in memory separate from the heap, as well as JVM internal data structures.

This excerpt is taken straight from the java documentation, and I can imagine scenarios where you might want the requests to err on the side of caution. Making someone copy the helm chart and change it just for a slight bit of added convenience, I think is unnecessary. :)

[clockworksoul]

Oh, so, what I meant was that java heap size m == Mi for resources in kubernetes.

@mikn I interpreted that as you intended. :) Shall we move this over to the PR https://github.com/clockworksoul/helm-elasticsearch/pull/29?