feat(operator): derive GOMEMLIMIT and expose pprof on loopback - #643
Open
graveland wants to merge 1 commit into
Open
feat(operator): derive GOMEMLIMIT and expose pprof on loopback#643graveland wants to merge 1 commit into
graveland wants to merge 1 commit into
Conversation
This comment has been minimized.
This comment has been minimized.
graveland
force-pushed
the
chore/operator-memory-instrumentation
branch
2 times, most recently
from
September 11, 2026 02:38
d6e0bad to
0a4c6a0
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
graveland
marked this pull request as ready for review
September 11, 2026 14:33
graveland
force-pushed
the
chore/operator-memory-instrumentation
branch
from
September 11, 2026 14:35
0a4c6a0 to
a915b11
Compare
This comment has been minimized.
This comment has been minimized.
Verolop
reviewed
Sep 11, 2026
Verolop
reviewed
Sep 11, 2026
Verolop
left a comment
Verolop
left a comment
Contributor
There was a problem hiding this comment.
nice! I just have a small ux request.
Go derives GOMAXPROCS from the cgroup CPU quota on its own, but has no equivalent for memory: golang/go#75164 is still an open proposal, so GOMEMLIMIT defaults to math.MaxInt64 and a reconcile burst OOMKills instead of making the collector work harder. Derive it from the limit the downward API projects, using resourceFieldRef so it resolves after any kustomize patch and cannot drift from the limit it is derived from. Defer to an explicit GOMEMLIMIT when one is set. The manifest supplies MEMORY_LIMIT_BYTES unconditionally, so an overlay that lowers GOMEMLIMIT to leave more headroom would otherwise be overwritten with no signal beyond the "resolved runtime limits" line. Add --pprof-bind-address, enabled by default, set to 127.0.0.1:6060 in the shipped manifest. If it's not enabled by default, then it wouldn't be available when we most need it. Binding to localhost means we have to port-forward to access it, so k8s authentication is the guard. The resource values replace untouched kubebuilder scaffold defaults (128Mi / 500m), not measured ones, and stay provisional until those profiles exist. The floor on the memory limit is arithmetic rather than empirical: GOMEMLIMIT excludes the binary's own mapping, ~58MiB stripped, so at 128Mi the 90% handed to the runtime could not fit alongside it. No CPU limit on purpose, since a limit also sets GOMAXPROCS (500m pinned it to 2) and throttling the collector is the wrong way to fail when GOMEMLIMIT is what keeps the process inside its memory limit. Anchor the multigres-operator ignore rule while here: unanchored it matched any path component, which also hid cmd/multigres-operator/. Signed-off-by: Brent Graveland <graveland@supabase.io>
graveland
force-pushed
the
chore/operator-memory-instrumentation
branch
from
September 11, 2026 16:17
a915b11 to
eb43528
Compare
🔬 Go Test Coverage ReportSummary
Status✅ PASS DetailShow New Coverage |
Verolop
approved these changes
Sep 11, 2026
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
2 participants
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 changed
config/manager/manager.yamlpasses--pprof-bind-address=127.0.0.1:6060. The flag itself defaults to empty, soa bare
./managerstays off and does not claim the port during local runs.GOMEMLIMITderived at startup fromMEMORY_LIMIT_BYTES, projected fromlimits.memoryvia the downward API, at 90% of the limit.512Mi, requests100m/256Mi, no CPU limit. These replaceuntouched kubebuilder scaffold values (
128Mi/500m), not measured ones.Why
Go derives
GOMAXPROCSfrom the cgroup CPU quota but has no equivalent formemory (golang/go#75164), so without
GOMEMLIMITthe collector has no reasonto run before the cgroup OOM-kills the process.
pprof is on by default because it cannot usefully be enabled on demand:
changing args rolls the Deployment, so the process holding the heap under
investigation is replaced by a fresh one. A slow leak takes days to reappear,
and after an OOMKill the evidence is already gone.
Notable details / risks
Access control is delegated to Kubernetes on purpose. Binding to loopback
means the only route in is the API server's
pods/portforwardsubresource, soreaching the endpoint requires a principal that authenticates to the cluster
and that RBAC authorizes for portforward in this namespace, and the attempt is
recorded in the API server audit log. The endpoint stays unauthenticated
because adding a check there would duplicate one that already happened, while
introducing a second credential to issue, rotate and leak. Revocation stays
where cluster access is already managed.
What is actually exposed, to be precise about it: profiles are sampled stack
traces and byte counts, so they reveal code paths and allocation sizes, not
the contents of memory. The endpoint does serve this process's argv via
/debug/pprof/cmdline, and/debug/pprof/profileand/tracelet any callerthat reaches the port consume CPU on demand, which matters for a
leader-elected singleton.
No CPU limit is deliberate: five controllers run at
MaxConcurrentReconciles20, and throttling the collector is the wrong way to fail when
GOMEMLIMITiswhat keeps the process inside its memory limit. A CPU limit also sets
GOMAXPROCS(500m pinned it to 2).The
512Mifloor is arithmetic, not empirical:GOMEMLIMITexcludes thebinary's own mapping (~58MiB stripped), so at
128Mithe 90% handed to theruntime could not fit alongside it. Resource values are provisional pending
the profiles this PR makes possible.
.gitignore: the baremultigres-operatorentry matched any path componentand was hiding
cmd/multigres-operator/, which blockedmain_test.go.Anchored to
/multigres-operatorrather than deleted, since it still guardsagainst an ad-hoc
go build ./cmd/multigres-operatordropping a binary atthe repo root.