carlos / platform Public

instancectl/cgroup: use cgroup.HighOf for MemoryHigh, poll cgroup wait at 50ms (plan 2, task 5 fix round 1)

Export cgroup.highOf as HighOf so instancectl's set-property MemoryHigh
matches exactly what the cgroup manager writes for a unit tier and an
exec-child tier (spec §4.3), rather than the brief's own test-derived
constants. setTier now calls cgroup.HighOf(tier.MemoryMax); the two
TestStart* expectations move to 483183819 and 966367638, HighOf's actual
output for the two platform tiers.

Both WaitCgroupEmpty call sites (handleSeed, handleSweep) now poll every
50ms as the controller ruling specified, correcting the 200ms literal in
the task-5 brief's own code block. TestSeedFailsWhenTheCgroupStaysPopulated
still returns 502 deterministically under its 200ms budget (verified over
five repeated runs).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YZ1ZvWj7naf73GjTX8MBv5

Paul Campbell pushed by paul@keymail.dev 1 month ago 7a1e717c61235e8dfe8a730e11940a0901cca2c5 parent 82caa69

4 files changed, +12 −14

internal/cgroup/cgroup.go +6 −5
internal/cgroup/strict_linux.go +1 −1
internal/instancectl/server.go +3 −6
internal/instancectl/server_test.go +2 −2

diff --git a/internal/cgroup/cgroup.go b/internal/cgroup/cgroup.go

index 94ccaff70..912208afc 100644

--- a/internal/cgroup/cgroup.go

+++ b/internal/cgroup/cgroup.go

@@ -61,9 +61,10 @@ var (

TierChildren = Limit{MemoryMax: 1 << 30}

)

-// highOf is memory.high for a given memory.max: 90%, so reclaim and

-// throttling start well before the kill.

-func highOf(max int64) int64 { return max / 10 * 9 }

+// HighOf is the memory.high for a memory.max: 90 percent, computed as the

+// cgroup manager always has, so a unit tier and an exec-child tier are the

+// same number.

+func HighOf(max int64) int64 { return max / 10 * 9 }

// ExecOOMScoreAdj is written to every exec child's /proc/<pid>/oom_score_adj

// right after it starts. The kernel's OOM killer — the memcg one that fires

@@ -307,7 +308,7 @@ func (m *Manager) applyLimit(host, dir string, limit Limit) {

m.logf("cgroup: %s: memory.max: %v — child runs uncapped", host, err)

return

}

- if err := os.WriteFile(filepath.Join(dir, "memory.high"), []byte(strconv.FormatInt(highOf(limit.MemoryMax), 10)), 0o644); err != nil {

+ if err := os.WriteFile(filepath.Join(dir, "memory.high"), []byte(strconv.FormatInt(HighOf(limit.MemoryMax), 10)), 0o644); err != nil {

m.logf("cgroup: %s: memory.high: %v — child is capped but not throttled first", host, err)

}

// Swap is bounded too, or the cap isn't one: with zram on the box a

@@ -322,7 +323,7 @@ func (m *Manager) applyLimit(host, dir string, limit Limit) {

if err := os.WriteFile(filepath.Join(dir, "memory.oom.group"), []byte("1"), 0o644); err != nil {

m.logf("cgroup: %s: memory.oom.group: %v — an OOM kill here takes one process, not the family", host, err)

}

- m.logf("cgroup: %s: memory.max=%dM memory.high=%dM", host, limit.MemoryMax>>20, highOf(limit.MemoryMax)>>20)

+ m.logf("cgroup: %s: memory.max=%dM memory.high=%dM", host, limit.MemoryMax>>20, HighOf(limit.MemoryMax)>>20)

}

// startAndMove is the fallback spawn: plain start, then write the child's

diff --git a/internal/cgroup/strict_linux.go b/internal/cgroup/strict_linux.go

index 7d365038e..8c96b3d20 100644

--- a/internal/cgroup/strict_linux.go

+++ b/internal/cgroup/strict_linux.go

@@ -206,7 +206,7 @@ func (m *Manager) PrepareStrict(host string, limit Limit) (out *Prepared, err er

if populated, e := strictPopulation(p.events); e != nil || populated {

return nil, ErrStrictRefused

}

- high := highOf(limit.MemoryMax) / int64(os.Getpagesize()) * int64(os.Getpagesize())

+ high := HighOf(limit.MemoryMax) / int64(os.Getpagesize()) * int64(os.Getpagesize())

p.expected = map[string]string{"memory.max": strconv.FormatInt(limit.MemoryMax, 10), "memory.high": strconv.FormatInt(high, 10), "memory.swap.max": strconv.FormatInt(limit.MemoryMax, 10), "memory.oom.group": "1"}

for name := range p.expected {

p.limits[name], e = strictOpen(p.dir, name, unix.O_RDWR)

diff --git a/internal/instancectl/server.go b/internal/instancectl/server.go

index 5995fc057..18425ba9a 100644

--- a/internal/instancectl/server.go

+++ b/internal/instancectl/server.go

@@ -207,10 +207,7 @@ func parseRequest(r *http.Request, roles []Role, wantTier bool) (request, error)

func (s *Server) setTier(ctx context.Context, host string, tier cgroup.Limit) error {

max := strconv.FormatInt(tier.MemoryMax, 10)

- // Multiply before dividing: MemoryMax/10*9 truncates a step early and

- // mis-rounds by up to a few bytes against the tier constants' expected

- // MemoryHigh (536870912 -> 483183820, not 483183819).

- high := strconv.FormatInt(tier.MemoryMax*9/10, 10)

+ high := strconv.FormatInt(cgroup.HighOf(tier.MemoryMax), 10)

_, err := s.Systemctl(ctx, "set-property", "--runtime", SliceName(host),

"MemoryMax="+max, "MemoryHigh="+high, "MemorySwapMax="+max)

return err

@@ -282,7 +279,7 @@ func (s *Server) handleSeed(w http.ResponseWriter, r *http.Request) {

m := parseShow(out)

wctx, cancel := context.WithTimeout(ctx, s.waitBudget())

defer cancel()

- if err := WaitCgroupEmpty(wctx, s.CgroupRoot, SliceName(req.host), unit, 200*time.Millisecond); err != nil {

+ if err := WaitCgroupEmpty(wctx, s.CgroupRoot, SliceName(req.host), unit, 50*time.Millisecond); err != nil {

http.Error(w, "seed: "+err.Error(), http.StatusBadGateway)

return

}

@@ -368,7 +365,7 @@ func (s *Server) handleSweep(w http.ResponseWriter, r *http.Request) {

}

_, esc, _ := ParseUnitName(unit)

wctx, cancel := context.WithTimeout(ctx, s.waitBudget())

- err := WaitCgroupEmpty(wctx, s.CgroupRoot, "carlos-instances-"+esc+".slice", unit, 200*time.Millisecond)

+ err := WaitCgroupEmpty(wctx, s.CgroupRoot, "carlos-instances-"+esc+".slice", unit, 50*time.Millisecond)

cancel()

if err != nil {

http.Error(w, "sweep: "+unit+": "+err.Error(), http.StatusBadGateway)

diff --git a/internal/instancectl/server_test.go b/internal/instancectl/server_test.go

index d686645ff..309c0d18b 100644

--- a/internal/instancectl/server_test.go

+++ b/internal/instancectl/server_test.go

@@ -98,7 +98,7 @@ func TestStartSetsTheTierThenStartsThenReadsTheInvocation(t *testing.T) {

if err := json.NewDecoder(resp.Body).Decode(&out); err != nil || out.InvocationID != "abc123" {

t.Errorf("response = %+v, %v", out, err)

}

- want := "set-property --runtime carlos-instances-bill\\x2d2.example.com.slice MemoryMax=536870912 MemoryHigh=483183820 MemorySwapMax=536870912\n" +

+ want := "set-property --runtime carlos-instances-bill\\x2d2.example.com.slice MemoryMax=536870912 MemoryHigh=483183819 MemorySwapMax=536870912\n" +

"start carlos-instance@bill\\x2d2.example.com.service\n" +

"show -p InvocationID --value carlos-instance@bill\\x2d2.example.com.service"

if got := ctl.joined(); got != want {

@@ -133,7 +133,7 @@ func TestStartReportsSystemctlFailureAs502(t *testing.T) {

if resp.StatusCode != 502 {

t.Errorf("status %d, want 502", resp.StatusCode)

}

- if !strings.Contains(ctl.joined(), "MemoryMax=1073741824 MemoryHigh=966367641 MemorySwapMax=1073741824") {

+ if !strings.Contains(ctl.joined(), "MemoryMax=1073741824 MemoryHigh=966367638 MemorySwapMax=1073741824") {

t.Errorf("children tier not applied: %s", ctl.joined())

}