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

Reduce memory allocation when diff'ing large manifests #832

Closed
sjentzsch opened this issue Oct 16, 2023 · 1 comment
Closed

Reduce memory allocation when diff'ing large manifests #832

sjentzsch opened this issue Oct 16, 2023 · 1 comment
Assignees
Labels
bug This issue describes a defect or unexpected behavior carvel accepted This issue should be considered for future work and that the triage process has been completed

Comments

@sjentzsch
Copy link

sjentzsch commented Oct 16, 2023

With latest kapp-controller v0.48.1 (and related kapp version) we face the following issue:

We have an app with large manifests (based on https://github.com/prometheus-community/helm-charts/tree/main/charts/kube-prometheus-stack), namely about 4.4MB of manifest files. When we release a new version of this app, and say all labels do change, so the diff is large, kapp seems to allocate so much memory, that it crashed on our system with 4GB of memory allocated for the container (Reconcile failed: Deploying: signal: killed). After providing about 7GB of memory (limits), it finally could reconcile successfully.

image
image

This issue is about tracking this issue, and improving upon the memory allocation required for large diffs.

As @praveenrewar wrote:

The total memory allocation is almost 3 times when there is diff vs when there is no diff. I will try to look into it, meanwhile I think you should be able to reduce the memory limits for the container, unless you are planning to bump the helm chart version again. Also, feel free to create an issue on GitHub for tracking the progress.

Related Slack channel: https://kubernetes.slack.com/archives/CH8KCCKA5/p1686575635485759

(maybe also relates to #599 ?)

@sjentzsch sjentzsch added the carvel triage This issue has not yet been reviewed for validity label Oct 16, 2023
@praveenrewar praveenrewar added bug This issue describes a defect or unexpected behavior carvel accepted This issue should be considered for future work and that the triage process has been completed and removed carvel triage This issue has not yet been reviewed for validity labels Oct 16, 2023
@praveenrewar praveenrewar self-assigned this Oct 16, 2023
@praveenrewar
Copy link
Member

We did some memory optimization as part of #863. Most of it is enabled by default. One optimization is feature gated. We have introduced a new flags --diff-anchored, this uses a different algorithm to compute diff between existing and new resources which consumes less memory, however the diff produced is also different from before, hence we haven't enabled it by default.
This flag has been enabled in kapp-controller starting kapp-controller v0.51.0, and it can be enabled via the kapp raw opts in kapp-controller configuration or in the package/app.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue describes a defect or unexpected behavior carvel accepted This issue should be considered for future work and that the triage process has been completed
Projects
Archived in project
Development

No branches or pull requests

2 participants