fix(runtime): use automaxprocs for cgroup-aware GOMAXPROCS#162
Conversation
There was a problem hiding this comment.
Code Review
This pull request replaces the manual GOMAXPROCS configuration with the go.uber.org/automaxprocs library to automatically adjust CPU quotas in containerized environments. While this improves container compatibility, the reviewer noted that the previous implementation specifically reserved two CPU cores to prevent contention with the host process (ArmA 3). Since automaxprocs defaults to the full core count on bare metal, this change may lead to performance issues if the original 2-core buffer was intentional for stability.
Merging this branch will not change overall coverage
Coverage by fileChanged files (no unit tests)
Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code. |
Summary
go.uber.org/automaxprocsand callmaxprocs.Setininit()instead of the oldGOMAXPROCS(numCPUs - 2)math.runtime.NumCPU()returns the host count, not the cgroup quota — on a 24-core host with a 200% Docker CPU limit the old code had Go scheduling 22 OS threads against a 2-core budget, producing exactly the "0 fps for 5-10s" stalls that motivate this PR.Why
A user running Pelican (Docker) reported their ArmA server hanging during
:MISSION:SAVE:. Raising the container CPU limit from 200% to 400% made a 30-minute test pass where 200% had reliably stalled — a textbook CFS-quota oversubscription symptom. The extension's log showednumCPUs=24even inside the container, confirming thatruntime.NumCPU()was returning the host count and that GOMAXPROCS was set to 22.automaxprocsreads/sys/fs/cgroup/cpu.max(cgroup v2) or the v1 equivalents and setsGOMAXPROCSto the actual CPU allowance.On bare metal / macOS / Windows where there's no cgroup quota,
maxprocs.Setfalls through toruntime.NumCPU(), so nothing changes.Test plan
go build ./cmd/ocap_recordergo test ./...