Repository navigation
[Backport release-1.6] fix(tcp-balancer): cap HAProxy fds and threads by the pod's own limits - #5006
Open
europrinter (yankawai) wants to merge 1 commit into
Conversation
Without a global section HAProxy 3.4 sizes itself from the node. maxconn comes from the nofile limit, cut to the built-in fd-hard-limit of 1048576, so about 524k, and the thread count is the number of CPUs the process may run on, whatever the CPU limit says. With one thread that alone idles at about 85 MiB, and every thread adds about 0.6 MiB, so a t1.nano replica with its 128Mi limit runs on a small node and is OOMKilled at startup on a node with many CPUs. fd-hard-limit is now 32 per MiB of the memory limit, from which HAProxy derives about 16 connections per MiB, 2035 for the t1.nano defaults. That leaves 64 KiB per connection against the roughly 35 KiB HAProxy budgets per connection itself when it is given a memory limit with -m, and the rest covers the process and the kernel socket buffers charged to the same cgroup. It caps the derived value instead of setting maxconn, because an explicit maxconn above half the hard nofile limit stops HAProxy at startup (strict-limits is on by default), while fd-hard-limit still follows a lower system limit. thread-hard-limit is the CPU limit rounded up, at least 1. It only caps the detected thread count, so a node with fewer CPUs is not forced to run more threads than it has. Assisted-by: LLM Signed-off-by: Yan Bondarenko <[email protected]> (cherry picked from commit 20545d0)
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
size/M
1 of 12 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
Backport of #5005 to
release-1.6. The commit is cherry-picked from 20545d0 without conflicts and keeps its cherry-pick reference. The tcp-balancer chart on this branch differs from main only in its README, so the HAProxy config template is the same as on main before the fix.HAProxy now takes its fd and thread caps from the replica's own memory and CPU limits instead of the node. With the chart defaults on v1.6.5 (
t1.nano, 128Mi,haproxy:3.4.4) HAProxy picked maxconn 524126 and both pods were OOMKilled in a crash loop. With this changet1.nanorendersfd-hard-limit 4096andthread-hard-limit 1, HAProxy derives maxconn 2035 and idles at about 19 MiB. The measurements are in #5005.On this branch
helm unittestpasses 13 of 13, against 9 of 9 onrelease-1.6, and against therelease-1.6template the four new cases fail and the nine existing ones pass.helm templaterenders the same manifests as #5005 on main for the default values, the whitelist,kubernetesAndTalos,t1.mediumand explicitresources. I have not run the change on a live cluster.Screenshots
Not a UI change.
Downstream repositories
One chart template and its unittest suite in
packages/apps/tcp-balancer. No values key, schema, default, README or package change, so no trigger map entry applies.Release note