ilkerandClaude Opus 5
e8d90cf375
CI / test (push) Successful in 52s
CI went red on Linux with two failures that never appear on macOS. Both were worth the trip. The first is a real bug. processAlive used kill(pid, 0), which succeeds on a zombie — a process that has exited and is only waiting to be reaped. That happens for real whenever an engine's parent dies before it does and nothing reaps the orphan, which in a container is the normal case: PID 1 is usually the workload rather than an init. The supervisor would then report a dead adopted engine as running forever, and hold its GPUs against every future placement. Zombies are now detected through procfs on Linux, where the answer exists, and the check degrades to its old behaviour elsewhere rather than to something worse. The second was a bad test. TestTenantFairnessWithinBand asserted on the order goroutines sent to a channel after being admitted, which is the order the Go scheduler woke them, not the order the dispatcher chose. Under load it measured the runtime. The rotation itself lives in bandQueue.pop, which is pure, so it is tested there — including what happens when a tenant runs out mid-rotation and when a cancelled waiter is removed. The controller-level test now asserts the thing that is actually stable: nobody is starved. Verified with three consecutive `go test -race ./...` runs in a Linux container. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
66 lines
2.0 KiB
Go
66 lines
2.0 KiB
Go
package supervisor
|
|
|
|
import (
|
|
"bytes"
|
|
"os"
|
|
"strconv"
|
|
"syscall"
|
|
)
|
|
|
|
// killGroup signals a whole process group.
|
|
//
|
|
// Engines fork worker processes that hold GPU memory. Signalling only the
|
|
// leader leaves those workers alive, and the next launch then fails to
|
|
// allocate — with an error that points at the new process rather than the old.
|
|
func killGroup(pid int, sig syscall.Signal) error {
|
|
// The negative pid addresses the group. Setpgid at launch made the leader's
|
|
// pid the group id.
|
|
if err := syscall.Kill(-pid, sig); err != nil {
|
|
if err == syscall.ESRCH {
|
|
return os.ErrProcessDone
|
|
}
|
|
return err
|
|
}
|
|
return nil
|
|
}
|
|
|
|
// processAlive reports whether a pid belongs to a process that is still doing
|
|
// something.
|
|
//
|
|
// A zombie is not. It has exited and is only waiting to be reaped, but
|
|
// kill(pid, 0) still succeeds on it — so a naive check reports a dead engine as
|
|
// running forever. That happens for real whenever an engine's parent dies
|
|
// before it does and nothing reaps the orphan: in a container, PID 1 is usually
|
|
// the workload rather than an init, and orphans simply accumulate.
|
|
func processAlive(pid int) bool {
|
|
if pid <= 0 {
|
|
return false
|
|
}
|
|
// Signal 0 performs the permission and existence checks without delivering
|
|
// anything.
|
|
if err := syscall.Kill(pid, 0); err != nil {
|
|
// EPERM means it exists but belongs to someone else, which for our
|
|
// purposes is still "there".
|
|
return err == syscall.EPERM
|
|
}
|
|
return !isZombie(pid)
|
|
}
|
|
|
|
// isZombie reports whether a pid has exited but not been reaped.
|
|
//
|
|
// Only Linux exposes this; elsewhere there is no procfs and the answer is
|
|
// false, which degrades to the old behaviour rather than to something worse.
|
|
func isZombie(pid int) bool {
|
|
b, err := os.ReadFile("/proc/" + strconv.Itoa(pid) + "/stat")
|
|
if err != nil {
|
|
return false
|
|
}
|
|
// The comm field is parenthesised and may itself contain spaces or
|
|
// parentheses, so the state character is found from the last ')'.
|
|
i := bytes.LastIndexByte(b, ')')
|
|
if i < 0 || i+2 >= len(b) {
|
|
return false
|
|
}
|
|
return b[i+2] == 'Z'
|
|
}
|