Skip to content

Commit 386dad7

Browse files
balaji-gbalajinvda
andcommitted
fix(container-cache): size nginx workers from the CPU limit, not the host
worker_processes was `auto`, which makes nginx call sysconf(_SC_NPROCESSORS_ONLN). That reports every core on the host and is blind to the cgroup quota, so a pod with a CPU limit well below the node's core count starts several times more workers than it can ever run. Observed in production: about 128 worker processes against a 32 core limit on a 128 vCPU node. It does not usually show up as sustained throttling, and did not here: the measured throttled-period ratio was under 0.004 percent. The costs are elsewhere. - Memory. Each worker preallocates its own worker_connections structures, so that footprint is multiplied by the host core count rather than by the quota. - Upstream keepalive. Pools are per worker process, so more workers than the pod can schedule fragments every pool and defeats connection reuse, including the peer-hop reuse added alongside this. - Burst latency. Many runnable workers drain the quota well before the 100ms CFS period ends, so a burst becomes a multi-second stall rather than a small slowdown. One pod showed 431 seconds of throttled time across only 47 throttled periods, which is about 9 seconds per event. Derive it from resources.limits.cpu, with cache.workerProcesses as an override. Both plain-integer and milliCPU limits are handled, sub-core limits floor to one worker rather than zero, and no limit at all still falls back to auto, which is the one case where the host count is the correct answer. worker_connections is unchanged. It is a per-worker figure and its current value was raised deliberately. Tests: adds tests/chart-render/verify-worker-processes.sh covering the default derivation, an explicit override, milliCPU conversion, the sub-core floor, integer-typed limits, the no-limit fallback, and that worker_connections is left alone. Verified the suite fails when the derivation is reverted to auto. Relates to #1037 Co-Authored-By: Balaji Ganesan <bganesan@nvidia.com>
1 parent 3a84e0f commit 386dad7

3 files changed

Lines changed: 87 additions & 1 deletion

File tree

deploy/helm/container-cache/deploy/files/nginx.conf

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,33 @@
1414
# limitations under the License.
1515
user nvs;
1616
env NODE_IP;
17-
worker_processes auto;
17+
{{- /*
18+
worker_processes must track the container's CPU limit, not the node.
19+
`auto` makes nginx call sysconf(_SC_NPROCESSORS_ONLN), which reports every
20+
core on the host and is blind to the cgroup quota. On a large node with a
21+
much smaller CPU limit that spawns several times more workers than the pod
22+
can ever run, which costs memory (each worker preallocates its own
23+
worker_connections structures), fragments the per-worker upstream keepalive
24+
pools, and turns a burst into a multi-second CFS stall because many runnable
25+
workers drain the quota well before the 100ms period ends.
26+
27+
Derived from resources.limits.cpu unless cache.workerProcesses overrides it.
28+
Falls back to auto only when no limit is set, which is the one case where the
29+
node count is the right answer.
30+
*/}}
31+
{{- $wp := ((($.Values.cache).workerProcesses) | default "" | toString) -}}
32+
{{- if eq $wp "" -}}
33+
{{- $lim := (((($.Values.resources).limits).cpu) | default "" | toString) -}}
34+
{{- if hasSuffix "m" $lim -}}
35+
{{- $wp = (max 1 (div (atoi (trimSuffix "m" $lim)) 1000)) | toString -}}
36+
{{- else if $lim -}}
37+
{{- $n := atoi $lim -}}
38+
{{- $wp = ternary "auto" ($n | toString) (eq $n 0) -}}
39+
{{- else -}}
40+
{{- $wp = "auto" -}}
41+
{{- end -}}
42+
{{- end }}
43+
worker_processes {{ $wp }};
1844
worker_rlimit_nofile {{ $.Values.cache.workerRlimitNofile | default 65536 }};
1945
error_log /var/log/nginx/error.log warn;
2046

deploy/helm/container-cache/deploy/values.yaml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,14 @@ cache:
8181
# under heavy concurrent large transfers, which starved even /healthz and
8282
# tripped the liveness probe.
8383
workerConnection: 16384
84+
# Number of nginx worker processes. Leave empty to derive it from
85+
# resources.limits.cpu, which is almost always what you want: nginx's own
86+
# `auto` reads the host's core count and cannot see the cgroup quota, so on a
87+
# large node it starts several times more workers than the pod can run.
88+
#
89+
# This is per process, and worker_connections above is per worker, so total
90+
# connection capacity is the product of the two.
91+
workerProcesses: ""
8492

8593
# Liveness/readiness probe tuning for the nginx-proxy container. /healthz is
8694
# served on a dedicated port but by the same worker processes, so under heavy
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
#!/usr/bin/env bash
2+
# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
3+
# SPDX-License-Identifier: Apache-2.0
4+
#
5+
# worker_processes must track the container CPU limit, never the host core
6+
# count. nginx's own `auto` calls sysconf(_SC_NPROCESSORS_ONLN) and cannot see
7+
# the cgroup quota, so on a large node it starts several times more workers than
8+
# the pod can run. Run from the chart subtree:
9+
# bash tests/chart-render/verify-worker-processes.sh
10+
set -euo pipefail
11+
CHART_DIR="$(cd "$(dirname "$0")/../.." && pwd)/deploy"
12+
fail() { echo "FAIL: $*" >&2; exit 1; }
13+
14+
# Emits the rendered worker_processes value for the given --set arguments.
15+
wp() {
16+
helm template t "$CHART_DIR" "$@" 2>/dev/null \
17+
| grep -m1 'worker_processes' \
18+
| sed -E 's/.*worker_processes +([^;]+);.*/\1/'
19+
}
20+
21+
echo "1. default: derived from resources.limits.cpu, not from the host"
22+
got="$(wp)"
23+
[ "$got" = "32" ] || fail "expected 32 from the default cpu limit, got '$got'"
24+
25+
echo "2. explicit cache.workerProcesses wins"
26+
got="$(wp --set cache.workerProcesses=8)"
27+
[ "$got" = "8" ] || fail "explicit override ignored, got '$got'"
28+
29+
echo "3. milliCPU limits are converted, not parsed as a bare integer"
30+
got="$(wp --set resources.limits.cpu=16500m)"
31+
[ "$got" = "16" ] || fail "expected 16 from 16500m, got '$got'"
32+
33+
echo "4. sub-core limits still yield at least one worker"
34+
# A naive integer division here yields 0, and `worker_processes 0` is a config
35+
# error that stops nginx from serving at all.
36+
got="$(wp --set resources.limits.cpu=500m)"
37+
[ "$got" = "1" ] || fail "expected 1 worker for a 500m limit, got '$got'"
38+
39+
echo "5. integer-typed limits parse the same as string-typed"
40+
got="$(wp --set-json resources.limits.cpu=8)"
41+
[ "$got" = "8" ] || fail "expected 8 from an integer-typed limit, got '$got'"
42+
43+
echo "6. no cpu limit falls back to auto"
44+
# With no quota the host core count is the correct answer, so auto is right.
45+
got="$(wp --set resources.limits.cpu=null)"
46+
[ "$got" = "auto" ] || fail "expected auto when no cpu limit is set, got '$got'"
47+
48+
echo "7. worker_connections is per worker and is left alone"
49+
helm template t "$CHART_DIR" 2>/dev/null | grep -q 'worker_connections 16384' \
50+
|| fail "worker_connections must remain at its configured value"
51+
52+
echo "PASS: worker_processes tracks the CPU limit in every value shape"

0 commit comments

Comments
 (0)