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

helm: set GOMEMLIMIT via k8s downward API #27958

Merged
merged 1 commit into from
Sep 13, 2023

Conversation

bimmlerd
Copy link
Member

@bimmlerd bimmlerd commented Sep 6, 2023

The rationale for setting GOMEMLIMIT is to give the Go Garbage Collector (GC) more information about the potentially constrained environment it is running in. Without setting GOMEMLIMIT, scenarios can occur where the GC may consider it beneficial to not run, when in fact the pod is about to be OOM-killed. This occurs since the GC doesn't understand container resource limits, and hence may let garbage balloon over the limit, since it continually must make a trade-off between the CPU overhead and the memory usage.

GOMEMLIMIT is a soft limit for the Go GC. It instructs the GC that it should try to keep the total memory usage of the Go runtime below said limit. You can learn more about it at https://pkg.go.dev/runtime, or specifically at https://pkg.go.dev/runtime/debug#SetMemoryLimit.

The K8s downward API allows setting environment variables based on values of the container specification, such as the memory limit in this case. If the limit isn't set, it will fall back to a sensible value, see https://kubernetes.io/docs/concepts/workloads/pods/downward-api/#fallback-information-for-resource-limits.

The cilium-agent now sets GOMEMLIMIT to the container's memory resource limit, which helps the Go GC to avoid unnecessary OOMs.

The rationale for setting GOMEMLIMIT is to give the Go Garbage
Collector (GC) more information about the potentially constrained
environment it is running in. Without setting GOMEMLIMIT, scenarios can
occur where the GC may consider it beneficial to _not_ run, when in fact
the pod is about to be OOM-killed. This occurs since the GC doesn't
understand container resource limits, and hence may let garbage balloon
over the limit, since it continually must make a trade-off between the
CPU overhead and the memory usage.

GOMEMLIMIT is a soft limit for the Go GC. It instructs the GC that it
should try to keep the total memory usage of the Go runtime below said
limit. You can learn more about it at https://pkg.go.dev/runtime, or
specifically at https://pkg.go.dev/runtime/debug#SetMemoryLimit.

The K8s downward API allows setting environment variables based on
values of the container specification, such as the memory limit in this
case. If the limit isn't set, it will fall back to a sensible value, see
https://kubernetes.io/docs/concepts/workloads/pods/downward-api/#fallback-information-for-resource-limits.

Signed-off-by: David Bimmler <david.bimmler@isovalent.com>
@bimmlerd bimmlerd added kind/enhancement This would improve or streamline existing functionality. sig/k8s Impacts the kubernetes API, or kubernetes -> cilium internals translation layers. release-note/minor This PR changes functionality that users may find relevant to operating Cilium. area/helm Impacts helm charts and user deployment experience sig/agent Cilium agent related. labels Sep 6, 2023
@bimmlerd bimmlerd marked this pull request as ready for review September 6, 2023 09:35
@bimmlerd bimmlerd requested review from a team as code owners September 6, 2023 09:35
@bimmlerd
Copy link
Member Author

bimmlerd commented Sep 6, 2023

/test

CI triage:

Copy link
Member

@gandro gandro left a comment

Choose a reason for hiding this comment

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

Looks straight forward enough!

@bimmlerd
Copy link
Member Author

bimmlerd commented Sep 7, 2023

/ci-e2e

@joamaki

This comment was marked as resolved.

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Sep 13, 2023
@nathanjsweet nathanjsweet merged commit 7694bba into cilium:main Sep 13, 2023
61 of 62 checks passed
@bimmlerd bimmlerd deleted the pr/bimmlerd/set-go-memlimit branch September 14, 2023 07:47
@christarazi christarazi added the kind/performance There is a performance impact of this. label Sep 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/helm Impacts helm charts and user deployment experience kind/enhancement This would improve or streamline existing functionality. kind/performance There is a performance impact of this. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/minor This PR changes functionality that users may find relevant to operating Cilium. sig/agent Cilium agent related. sig/k8s Impacts the kubernetes API, or kubernetes -> cilium internals translation layers.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants