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

Support enabling mimalloc and turn it on by default #54

Closed
wants to merge 3 commits into from

Conversation

sslavic
Copy link

@sslavic sslavic commented Apr 12, 2020

This was supported in https://github.com/kubernetes/ingress-nginx/releases/tag/nginx-0.26.0 and recently in upstream chart too. We should turn it on to reduce memory utilization - expected at least 10% reduction. Memory utilization increased with recent tuning of max-worker-connections #50 optimization for high performance scenarios.

Even without max-worker-connections switched to 0, some tenant clusters where HPA trials were run had large amount of Ingress resource definitions, resulting in high memory utilization especially during nginx configuration reloads. To better support these use cases out of the box, together with enabling HPA default memory requests for most non-trivial clusters were increased to 2.5GB. Hopefully with applying mimalloc enabled we can lower the utilization. Smaller nginx memory footprint is good for both cost efficiency, faster scheduling and better elasticity - in production use cases like flash sale spikes nginx can scale out / add same capacity faster while creating less pressure on worker nodes scale; with larger nginx it is more likely that additional worker nodes have to be added to place new nginx replicas, and it takes time for cluster-autoscaler to kick in and nodes to be provisioned.

In addition to making mimalloc optional and enabled by default, this PR adds support for configuring additional environment variables for the nginx controller container, like in upstream chart. This among other things enables configuring additional mimalloc options if we find needed.

@sslavic sslavic self-assigned this Apr 12, 2020
@sslavic
Copy link
Author

sslavic commented Apr 13, 2020

This depends on kubernetes/ingress-nginx#5357 being merged and released.

@sslavic
Copy link
Author

sslavic commented Oct 5, 2020

mimalloc got enabled by default upstream. We've synced defaults with upstream via upgrade to ingress-nginx v0.40.1 PR #122. Closing this PR in favor of the upgrade one.

@sslavic sslavic closed this Oct 5, 2020
@sslavic sslavic deleted the mimalloc branch October 5, 2020 08:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant