Skip to content

Commit 0619c79

Browse files
nadaverellclaude
andauthored
Size the Go GC to the container memory limit (#1805)
## Why bd1ec82 raised the chart's memory limit to 1Gi, but the Go runtime never reads the cgroup on its own — without `GOMEMLIMIT` the GC runs at GOGC=100 with no idea a wall exists, so on a large cluster Radar can be OOMKilled while the collector still has plenty of garbage to reclaim. The bigger limit only helps if the GC knows about it. ## What Deliberately minimal: one read at startup, no background state. - New `internal/memlimit`: at startup, set `GOMEMLIMIT` to **85%** of the cgroup memory limit. Set once — a limit changed in place after startup is not followed (a resources change rolls new pods anyway, and a refresh loop would have to decide whether a failed read means "limit removed"). - **Resolves the process's own cgroup** from `/proc/self/cgroup` and takes the tightest finite limit walking up to the mount root — v2 (`memory.max`) and v1 (`memory.limit_in_bytes`). Reading only the mount root works under a private cgroup namespace (the k8s default on v2) but silently finds nothing under a host namespace or a systemd scope. Fails closed: a malformed path reads nothing. - An explicit `GOMEMLIMIT` env var wins, **including `GOMEMLIMIT=off`** (which the runtime reports the same as unset, so the env var is checked directly). - **No fallback default** (unlike skyhook-connector's 768MB): when no per-container limit is readable — laptops, pods with no limit — the runtime is left alone. There's no value that's right for both a 16Gi workstation and a 256Mi sidecar. - A cgroup value above the host's `MemTotal` is treated as the host limit leaking through, not a cap. - Non-Linux builds compile to a no-op; the resolver itself is platform-neutral and unit-tested with fixture trees (v2 private ns, v2 host ns nested path, tighter ancestor, v1 sentinel, comma-joined controllers, host leak, MemTotal-equal, malformed input). - Chart README resources rows updated to the new defaults (they still said `512Mi`/`128Mi`) and document the GOMEMLIMIT behaviour + override. Done in the binary rather than as a chart `resourceFieldRef` env so every deploy path benefits (Helm, raw manifests, Docker, Radar Hub images). **Why 85%:** GOMEMLIMIT covers Go-managed memory only; what also counts against the cgroup is the binary's mapped pages, modernc SQLite's own allocator and page cache. Radar is `CGO_ENABLED=0` and the kernel reclaims file-backed pages before OOM-killing, so ~150MiB at 1Gi is ample. `automemlimit` defaults to 90%; the connector uses 75%. ## Verified Linux binary in Docker, checking the startup log: | run | result | |---|---| | `--memory=1g` | `GOMEMLIMIT set to 870MiB from cgroup memory limit` | | `--memory=512m` | `GOMEMLIMIT set to 435MiB …` | | `--memory=1g --cgroupns=host` (root has no `memory.max`, limit lives on `/docker/<id>`) | `GOMEMLIMIT set to 870MiB …` | | no limit | no memlimit line, runtime untouched | | `--memory=1g -e GOMEMLIMIT=off` | no memlimit line, env respected | `go vet` for darwin + linux, `go build` for darwin/linux/windows, `go test ./internal/memlimit ./cmd/explorer`, `helm lint`. Cross-reviewed with Codex (adversarial) and Qodo; the nested-cgroup-path and `GOMEMLIMIT=off` findings are fixed here, the refresh-loop findings are moot since there is no loop. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Startup-only runtime tuning with env override and no-op when no limit is detected; cgroup reads are read-only and fail closed. > > **Overview** > Radar now sizes the Go GC to the container memory cap so large clusters are less likely to be **OOMKilled** while the runtime still has reclaimable heap. > > A new **`internal/memlimit`** package runs once at explorer startup (`memlimit.Apply()` in `cmd/explorer/main.go`): it resolves the process cgroup (v1/v2, private vs host namespace), takes the tightest finite limit, and sets **`GOMEMLIMIT` to 85%** via `debug.SetMemoryLimit`. Explicit **`GOMEMLIMIT`** (including `off`) wins; no readable cgroup limit leaves the runtime unchanged. Non-Linux builds no-op; cgroup parsing is covered by fixture tests. > > The Helm chart **README** updates documented default **CPU/memory requests and limits** (e.g. **1Gi** memory limit) and notes the automatic **GOMEMLIMIT** behavior and **`env` override**. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 0f04707. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
1 parent e0f6d8d commit 0619c79

7 files changed

Lines changed: 298 additions & 2 deletions

File tree

‎cmd/explorer/main.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
"github.com/skyhook-io/radar/internal/diagnosecli"
2525
"github.com/skyhook-io/radar/internal/k8s"
2626
mcppkg "github.com/skyhook-io/radar/internal/mcp"
27+
"github.com/skyhook-io/radar/internal/memlimit"
2728
"github.com/skyhook-io/radar/internal/reachability"
2829
"github.com/skyhook-io/radar/internal/server"
2930
versionpkg "github.com/skyhook-io/radar/internal/version"
@@ -257,6 +258,7 @@ func main() {
257258
startupMode = "Radar Cloud"
258259
}
259260
log.Printf("Radar %s starting (mode=%s, auth=%s)...", version, startupMode, *authMode)
261+
memlimit.Apply()
260262

261263
// Validate flags
262264
switch *authMode {

‎deploy/helm/radar/README.md‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -217,8 +217,10 @@ trend charts remain unavailable for Kubecost.
217217
| `argocd.token` | Inline Argo CD API token (dev only — lands in the release state) | `""` |
218218
| `argocd.url` | Explicit `argocd-server` URL; blank auto-discovers in-cluster | `""` |
219219
| `argocd.insecureTls` | Skip TLS verification for a self-signed `argocd-server` | `false` |
220-
| `resources.limits.memory` | Memory limit | `512Mi` |
221-
| `resources.requests.memory` | Memory request | `128Mi` |
220+
| `resources.requests.cpu` | CPU request | `200m` |
221+
| `resources.requests.memory` | Memory request | `256Mi` |
222+
| `resources.limits.cpu` | CPU limit | `2` |
223+
| `resources.limits.memory` | Memory limit. Radar sets `GOMEMLIMIT` to 85% of the container's cgroup limit at startup, so the GC collects harder as it approaches the limit; set `GOMEMLIMIT` under `env` to override | `1Gi` |
222224

223225
See `values.yaml` for all configuration options.
224226

‎internal/memlimit/cgroup.go‎

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
package memlimit
2+
3+
import (
4+
"math"
5+
"os"
6+
"path/filepath"
7+
"strconv"
8+
"strings"
9+
)
10+
11+
// limitFromFS returns the tightest memory limit enforced on this process, or 0
12+
// when no per-container cap is readable.
13+
//
14+
// Under a private cgroup namespace (the Kubernetes default on cgroup v2) the
15+
// process sits at "/" and the mount root carries its limit. Under a host
16+
// namespace or a systemd scope it sits in a nested path whose root has no
17+
// limit at all, so the walk starts at the process's own cgroup and takes the
18+
// minimum of every finite limit up to the root — an ancestor's cap is enforced
19+
// just the same. A value above the machine's MemTotal is the host limit
20+
// leaking through, not a cap; equal to MemTotal is a pod sized to the whole
21+
// node and is kept.
22+
func limitFromFS(mount, selfCgroup string, memTotal int64) int64 {
23+
v2Path, v1Path := parseSelfCgroup(selfCgroup)
24+
25+
limit := int64(math.MaxInt64)
26+
if v2Path != "" {
27+
limit = minAlongPath(filepath.Join(mount, v2Path), mount, "memory.max", parseV2Limit, limit)
28+
}
29+
if v1Path != "" {
30+
limit = minAlongPath(filepath.Join(mount, "memory", v1Path), filepath.Join(mount, "memory"), "memory.limit_in_bytes", parseV1Limit, limit)
31+
}
32+
if limit == math.MaxInt64 {
33+
return 0
34+
}
35+
if memTotal > 0 && limit > memTotal {
36+
return 0
37+
}
38+
return limit
39+
}
40+
41+
// parseSelfCgroup returns the process's cgroup v2 path (the "0::" entry) and its
42+
// v1 memory-controller path, either of which may be empty.
43+
func parseSelfCgroup(content string) (v2, v1 string) {
44+
for _, line := range strings.Split(content, "\n") {
45+
parts := strings.SplitN(line, ":", 3)
46+
if len(parts) != 3 {
47+
continue
48+
}
49+
switch {
50+
case parts[0] == "0" && parts[1] == "":
51+
v2 = parts[2]
52+
case containsController(parts[1], "memory"):
53+
v1 = parts[2]
54+
}
55+
}
56+
return v2, v1
57+
}
58+
59+
func containsController(list, name string) bool {
60+
for _, c := range strings.Split(list, ",") {
61+
if c == name {
62+
return true
63+
}
64+
}
65+
return false
66+
}
67+
68+
// minAlongPath reads file at every directory from start up to and including
69+
// stop, folding each finite value into limit.
70+
func minAlongPath(start, stop, file string, parse func(string) (int64, bool), limit int64) int64 {
71+
start, stop = filepath.Clean(start), filepath.Clean(stop)
72+
if !strings.HasPrefix(start, stop) {
73+
return limit
74+
}
75+
for dir := start; ; dir = filepath.Dir(dir) {
76+
if b, err := os.ReadFile(filepath.Join(dir, file)); err == nil {
77+
if v, ok := parse(strings.TrimSpace(string(b))); ok && v < limit {
78+
limit = v
79+
}
80+
}
81+
if dir == stop || dir == filepath.Dir(dir) {
82+
return limit
83+
}
84+
}
85+
}
86+
87+
func parseV2Limit(s string) (int64, bool) {
88+
if s == "max" {
89+
return 0, false
90+
}
91+
v, err := strconv.ParseInt(s, 10, 64)
92+
return v, err == nil && v > 0
93+
}
94+
95+
// cgroup v1 reports "unlimited" as a near-MaxInt64 page-rounded sentinel.
96+
func parseV1Limit(s string) (int64, bool) {
97+
v, err := strconv.ParseInt(s, 10, 64)
98+
return v, err == nil && v > 0 && v < int64(1)<<62
99+
}
100+
101+
func memTotalBytes() int64 {
102+
b, err := os.ReadFile("/proc/meminfo")
103+
if err != nil {
104+
return 0
105+
}
106+
for _, line := range strings.Split(string(b), "\n") {
107+
if !strings.HasPrefix(line, "MemTotal:") {
108+
continue
109+
}
110+
fields := strings.Fields(line)
111+
if len(fields) >= 2 {
112+
if kb, err := strconv.ParseInt(fields[1], 10, 64); err == nil {
113+
return kb * 1024
114+
}
115+
}
116+
}
117+
return 0
118+
}

‎internal/memlimit/cgroup_linux.go‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
package memlimit
2+
3+
import "os"
4+
5+
const cgroupMount = "/sys/fs/cgroup"
6+
7+
func cgroupLimitBytes() int64 {
8+
self, err := os.ReadFile("/proc/self/cgroup")
9+
if err != nil {
10+
return 0
11+
}
12+
return limitFromFS(cgroupMount, string(self), memTotalBytes())
13+
}

‎internal/memlimit/cgroup_other.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
//go:build !linux
2+
3+
package memlimit
4+
5+
func cgroupLimitBytes() int64 { return 0 }

‎internal/memlimit/cgroup_test.go‎

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
package memlimit
2+
3+
import (
4+
"os"
5+
"path/filepath"
6+
"testing"
7+
)
8+
9+
const gib = int64(1) << 30
10+
11+
func writeTree(t *testing.T, root string, files map[string]string) {
12+
t.Helper()
13+
for rel, content := range files {
14+
p := filepath.Join(root, rel)
15+
if err := os.MkdirAll(filepath.Dir(p), 0o755); err != nil {
16+
t.Fatal(err)
17+
}
18+
if err := os.WriteFile(p, []byte(content+"\n"), 0o644); err != nil {
19+
t.Fatal(err)
20+
}
21+
}
22+
}
23+
24+
func TestLimitFromFS(t *testing.T) {
25+
tests := []struct {
26+
name string
27+
self string
28+
files map[string]string
29+
memTotal int64
30+
want int64
31+
}{
32+
{
33+
name: "v2 private namespace: limit sits at the mount root",
34+
self: "0::/",
35+
files: map[string]string{"memory.max": "1073741824"},
36+
want: gib,
37+
},
38+
{
39+
name: "v2 host namespace: limit sits on the nested cgroup, root is max",
40+
self: "0::/kubepods.slice/kubepods-burstable.slice/cri-containerd-abc.scope",
41+
files: map[string]string{
42+
"memory.max": "max",
43+
"kubepods.slice/memory.max": "max",
44+
"kubepods.slice/kubepods-burstable.slice/memory.max": "max",
45+
"kubepods.slice/kubepods-burstable.slice/cri-containerd-abc.scope/memory.max": "536870912",
46+
},
47+
want: 512 << 20,
48+
},
49+
{
50+
name: "v2 nested: a tighter ancestor cap wins",
51+
self: "0::/parent/child",
52+
files: map[string]string{
53+
"parent/memory.max": "268435456",
54+
"parent/child/memory.max": "1073741824",
55+
},
56+
want: 256 << 20,
57+
},
58+
{
59+
name: "v2 unlimited everywhere",
60+
self: "0::/",
61+
files: map[string]string{"memory.max": "max"},
62+
want: 0,
63+
},
64+
{
65+
name: "v1: controller path, unlimited sentinel on the root",
66+
self: "12:memory:/docker/abc\n0::/",
67+
files: map[string]string{
68+
"memory/memory.limit_in_bytes": "9223372036854771712",
69+
"memory/docker/abc/memory.limit_in_bytes": "1073741824",
70+
},
71+
want: gib,
72+
},
73+
{
74+
name: "v1 in a comma-joined controller list",
75+
self: "3:cpu,memory,pids:/",
76+
files: map[string]string{"memory/memory.limit_in_bytes": "536870912"},
77+
want: 512 << 20,
78+
},
79+
{
80+
name: "host limit leaking through is not a cap",
81+
self: "0::/",
82+
files: map[string]string{"memory.max": "68719476736"},
83+
memTotal: 16 * gib,
84+
want: 0,
85+
},
86+
{
87+
name: "a limit equal to MemTotal is a pod sized to the node",
88+
self: "0::/",
89+
files: map[string]string{"memory.max": "17179869184"},
90+
memTotal: 16 * gib,
91+
want: 16 * gib,
92+
},
93+
{
94+
name: "unparseable /proc/self/cgroup",
95+
self: "garbage",
96+
files: map[string]string{"memory.max": "1073741824"},
97+
want: 0,
98+
},
99+
{
100+
name: "cgroup path escaping the mount is ignored",
101+
self: "0::/../../etc",
102+
files: map[string]string{"memory.max": "1073741824"},
103+
want: 0,
104+
},
105+
}
106+
for _, tt := range tests {
107+
t.Run(tt.name, func(t *testing.T) {
108+
root := t.TempDir()
109+
writeTree(t, root, tt.files)
110+
if got := limitFromFS(root, tt.self, tt.memTotal); got != tt.want {
111+
t.Fatalf("limitFromFS() = %d, want %d", got, tt.want)
112+
}
113+
})
114+
}
115+
}

‎internal/memlimit/memlimit.go‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
// Package memlimit sizes the Go GC to the container's memory limit.
2+
//
3+
// The Go runtime honors GOMEMLIMIT only when told about it; it never reads the
4+
// cgroup on its own, so a process under a Kubernetes memory limit otherwise
5+
// runs at GOGC=100 with no idea a wall exists and can be OOMKilled while the
6+
// collector still has plenty of garbage to reclaim.
7+
package memlimit
8+
9+
import (
10+
"log"
11+
"math"
12+
"os"
13+
"runtime/debug"
14+
)
15+
16+
// headroom is the fraction of the cgroup limit handed to GOMEMLIMIT. The limit
17+
// governs Go-managed memory only (heap, stacks, runtime structures); the
18+
// binary's mapped pages, SQLite's own allocator and page cache also count
19+
// against the cgroup, and the GC death-spirals when pinned right at the OOM
20+
// boundary.
21+
const headroom = 0.85
22+
23+
// Apply sets GOMEMLIMIT once, from the cgroup memory limit in force at startup.
24+
// An explicit GOMEMLIMIT env var wins — including GOMEMLIMIT=off, which the
25+
// runtime reports the same as unset — and a process outside a cgroup limit (a
26+
// laptop, a pod with no limit set) is left alone: there is no sensible default
27+
// that is right for both a 16Gi workstation and a 256Mi sidecar. A limit
28+
// changed in place after startup is not followed; a resources change rolls new
29+
// pods anyway.
30+
func Apply() {
31+
if os.Getenv("GOMEMLIMIT") != "" || debug.SetMemoryLimit(-1) != math.MaxInt64 {
32+
return
33+
}
34+
limit := cgroupLimitBytes()
35+
if limit <= 0 {
36+
return
37+
}
38+
target := int64(float64(limit) * headroom)
39+
debug.SetMemoryLimit(target)
40+
log.Printf("[memlimit] GOMEMLIMIT set to %dMiB from cgroup memory limit", target>>20)
41+
}

0 commit comments

Comments
 (0)