From 04902ff24a0ef60dc291d0875c39377db9c94d81 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 26 Oct 2026 13:37:00 +0100 Subject: [PATCH 1/6] Build seats yield: idle priority and a per-node processor cap on their runtimes, threads per step within it (issue 1806) The operator's desktop slows while it builds: the Go build runtime took 1650% of shanks' 24 processors with nothing holding it back. A build seat's holder now runs its runtime at idle priority under its node's cpu-cap share (declaration Yields, composed from Seat.Yields and the module's cpu-cap setting), and buildcpu/buildstep give each step its part of that cap in threads. --- go/buildcpu/buildcpu.go | 258 +++++++++++++++++++++++++++++++++ go/buildcpu/buildcpu_test.go | 92 ++++++++++++ go/buildstep/capacity_test.go | 16 ++ go/buildstep/cpu.go | 40 +++++ go/catalogue/runtime.go | 8 + go/catalogue/runtime_test.go | 28 ++++ go/catalogue/seats.go | 11 +- go/catalogue/yields.go | 41 ++++++ go/declaration/declaration.go | 43 ++++++ go/declaration/process_test.go | 19 +++ 10 files changed, 553 insertions(+), 3 deletions(-) create mode 100644 go/buildcpu/buildcpu.go create mode 100644 go/buildcpu/buildcpu_test.go create mode 100644 go/buildstep/cpu.go create mode 100644 go/catalogue/yields.go diff --git a/go/buildcpu/buildcpu.go b/go/buildcpu/buildcpu.go new file mode 100644 index 0000000..4df094e --- /dev/null +++ b/go/buildcpu/buildcpu.go @@ -0,0 +1,258 @@ +// Package buildcpu is how much of a build node's processors its builds may take, and how they yield to everything else +// there (novox/hq issue 1806; the operator, 2026-10-26: "limit CPU usage on our build nodes so the total cpu usage never +// impacts the normal workings of our nodes"). +// +// Three parts, each with one owner: +// +// - **Priority.** A build seat's holder (catalogue.Seat.Yields) runs at the lowest CPU and IO priority there is: the +// composer gives its runtime process declaration.Yields, and the node-engine writes it into the unit as +// CPUWeight=idle, IOWeight=1, Nice=19 and the best-effort IO class at its lowest level. Interactive and service +// work always comes first; builds take the cycles nothing else wants. +// - **The cap.** The build modules' setting `cpu-cap` (Setting) gives each node the share of its processors its builds +// may take at most, as `=` pairs. The composer turns this node's share into Yields.CPUShare, and the +// node-engine into CPUQuota for the machine's processor count: a hard bound, held by the kernel. +// - **Threads.** Inside that bound a step starts no more threads than its part of it (Threads): GOMAXPROCS, `go +// test -p`, the Node test runner's concurrency. A step with more runnable threads than its quota only waits in the +// kernel's throttle, and its timeouts with it. +package buildcpu + +import ( + "fmt" + "os" + "runtime" + "sort" + "strconv" + "strings" +) + +// Setting is the build modules' setting of each node's processor cap: `=` pairs separated by commas, +// such as `shanks=50,ace=75`; a percent sign after the number is allowed. Each build module declares it with Defaults +// as its default. +const Setting = "cpu-cap" + +// Defaults is the setting's default: the operator's two desktops (shanks, g14) keep half their processors for +// themselves at all times, and ace, a server, builds on three quarters. A node it does not name has DefaultPercent. +const Defaults = "shanks=50,g14=50,ace=75" + +// DefaultPercent is the cap of a node the setting does not name. +const DefaultPercent = 50 + +// Parse reads the setting's pairs. A percent outside 1 to 100 is refused: 0 would stop every build without pausing the +// seat (the capacity setting says that, with 0 asks), and more than 100 is no cap. +func Parse(s string) (map[string]int, error) { + out := map[string]int{} + for _, pair := range strings.Split(s, ",") { + pair = strings.TrimSpace(pair) + if pair == "" { + continue + } + node, value, ok := strings.Cut(pair, "=") + node = strings.TrimSpace(node) + value = strings.TrimSuffix(strings.TrimSpace(value), "%") + n, err := strconv.Atoi(strings.TrimSpace(value)) + if !ok || node == "" || err != nil || n < 1 || n > 100 { + return nil, fmt.Errorf("the processor cap %q is not =", pair) + } + if _, dup := out[node]; dup { + return nil, fmt.Errorf("the processor cap names %s twice", node) + } + out[node] = n + } + return out, nil +} + +// Of is node's cap in percent of its processors: DefaultPercent for a node the setting does not name. +func Of(setting, node string) (int, error) { + caps, err := Parse(setting) + if err != nil { + return 0, err + } + if n, ok := caps[node]; ok { + return n, nil + } + return DefaultPercent, nil +} + +// Format is a cap map in the setting's own words, sorted by node. +func Format(caps map[string]int) string { + nodes := make([]string, 0, len(caps)) + for n := range caps { + nodes = append(nodes, n) + } + sort.Strings(nodes) + parts := make([]string, 0, len(nodes)) + for _, n := range nodes { + parts = append(parts, fmt.Sprintf("%s=%d", n, caps[n])) + } + return strings.Join(parts, ",") +} + +// Quota is the unit's CPUQuota for a share of a machine with cores processors: systemd counts a quota in percent of +// one processor, so half of 24 is 1200%. At least 100%, one processor, so a cap on a small machine never stops a step. +func Quota(percent, cores int) int { + q := percent * cores + if q < 100 { + q = 100 + } + return q +} + +// Threads is how many threads one step may run at once: its part of the cap when slots steps run beside it, at least +// one. Rounded down, so the steps together stay inside the quota. +func Threads(percent, cores, slots int) int { + if slots < 1 { + slots = 1 + } + if percent < 1 || percent > 100 { + percent = 100 + } + t := cores * percent / 100 / slots + if t < 1 { + t = 1 + } + return t +} + +// Limits is what one step is held to on this node, as a status tool says it. +type Limits struct { + // Percent is the node's cap in percent of its processors, and Setting the words it came from. + Percent int `json:"cap-percent"` + Setting string `json:"cap-setting"` + // Cores is the machine's processor count. + Cores int `json:"cores"` + // Slots is the node's build capacity: steps at once. + Slots int `json:"slots"` + // Threads is what each step is given (Threads). + Threads int `json:"threads-per-step"` +} + +// LimitsFor is the limits of a step on node, from the cap setting and the node's capacity. A setting that cannot be read +// gives the default cap and says why in the error, so a step still runs with a bound rather than none. +func LimitsFor(setting, node string, slots int) (Limits, error) { + l := Limits{Setting: setting, Cores: runtime.NumCPU(), Slots: slots} + p, err := Of(setting, node) + if err != nil { + p = DefaultPercent + } + l.Percent = p + l.Threads = Threads(p, l.Cores, slots) + return l, err +} + +// Env is the environment a step's commands are given so they start no more threads than they may: GOMAXPROCS for every +// Go program (the compiler, a test binary, esbuild, buildah), UV_THREADPOOL_SIZE for Node's worker pool. Returned as +// KEY=value words to append to a command's environment. +func Env(threads int) []string { + if threads < 1 { + threads = 1 + } + t := strconv.Itoa(threads) + return []string{"GOMAXPROCS=" + t, "UV_THREADPOOL_SIZE=" + t} +} + +// WithEnv is env with Env(threads) in place of any GOMAXPROCS or UV_THREADPOOL_SIZE it carried. +func WithEnv(env []string, threads int) []string { + out := make([]string, 0, len(env)+2) + for _, kv := range env { + if strings.HasPrefix(kv, "GOMAXPROCS=") || strings.HasPrefix(kv, "UV_THREADPOOL_SIZE=") { + continue + } + out = append(out, kv) + } + return append(out, Env(threads)...) +} + +// InForce is what the kernel holds this process to, read rather than assumed (evidence over assertion): its nice +// level, its cgroup, the cgroup's CPU weight, idle mark and quota. A field the machine does not answer is left empty +// and Unread says which. +type InForce struct { + Nice int `json:"nice"` + Cgroup string `json:"cgroup,omitempty"` + CPUIdle string `json:"cpu-idle,omitempty"` + CPUWeight string `json:"cpu-weight,omitempty"` + // CPUMax is the cgroup's cpu.max: " ", or "max" with no quota. + CPUMax string `json:"cpu-max,omitempty"` + // Quota is CPUMax as a percent of one processor, 0 with none. + Quota int `json:"quota-percent,omitempty"` + IOWeight string `json:"io-weight,omitempty"` + Unread []string `json:"unread,omitempty"` +} + +// cgroupRoot is where the unified hierarchy is mounted. A variable so a test can point it elsewhere. +var cgroupRoot = "/sys/fs/cgroup" + +// procSelfCgroup is this process's cgroup file. A variable so a test can point it elsewhere. +var procSelfCgroup = "/proc/self/cgroup" + +// ReadInForce reads what the kernel holds this process to. +func ReadInForce() InForce { + var f InForce + f.Nice = nice() + raw, err := os.ReadFile(procSelfCgroup) + if err != nil { + f.Unread = append(f.Unread, "cgroup: "+err.Error()) + return f + } + for _, line := range strings.Split(strings.TrimSpace(string(raw)), "\n") { + if strings.HasPrefix(line, "0::") { + f.Cgroup = strings.TrimPrefix(line, "0::") + } + } + if f.Cgroup == "" { + f.Unread = append(f.Unread, "cgroup: not on the unified hierarchy") + return f + } + dir := cgroupRoot + f.Cgroup + read := func(name string) string { + b, err := os.ReadFile(dir + "/" + name) + if err != nil { + f.Unread = append(f.Unread, name+": "+err.Error()) + return "" + } + return strings.TrimSpace(string(b)) + } + f.CPUIdle = read("cpu.idle") + f.CPUWeight = read("cpu.weight") + f.CPUMax = read("cpu.max") + f.IOWeight = read("io.weight") + f.Quota = quotaOf(f.CPUMax) + return f +} + +// quotaOf is a cpu.max value as a percent of one processor: "1200000 100000" is 1200; "max ..." or nothing is 0. +func quotaOf(cpuMax string) int { + fields := strings.Fields(cpuMax) + if len(fields) != 2 || fields[0] == "max" { + return 0 + } + q, err1 := strconv.Atoi(fields[0]) + p, err2 := strconv.Atoi(fields[1]) + if err1 != nil || err2 != nil || p <= 0 { + return 0 + } + return q * 100 / p +} + +// procSelfStat is this process's stat file. A variable so a test can point it elsewhere. +var procSelfStat = "/proc/self/stat" + +// nice is this process's nice level from its stat file (the 19th field, counted after the command's closing +// parenthesis, which may itself hold spaces); 0 when it cannot be read. +func nice() int { + raw, err := os.ReadFile(procSelfStat) + if err != nil { + return 0 + } + s := string(raw) + i := strings.LastIndex(s, ")") + if i < 0 { + return 0 + } + fields := strings.Fields(s[i+1:]) + // After the command: state is field 3, so nice (field 19) is index 16. + if len(fields) < 17 { + return 0 + } + n, _ := strconv.Atoi(fields[16]) + return n +} diff --git a/go/buildcpu/buildcpu_test.go b/go/buildcpu/buildcpu_test.go new file mode 100644 index 0000000..d1d0549 --- /dev/null +++ b/go/buildcpu/buildcpu_test.go @@ -0,0 +1,92 @@ +package buildcpu + +import ( + "os" + "path/filepath" + "reflect" + "testing" +) + +// The cap's words, a node's share, the unit quota and the threads a step is given, in one table (issue 1806). +func TestCapQuotaAndThreads(t *testing.T) { + for _, c := range []struct { + name, setting, node string + cores, slots int + percent, quota int + threads int + refused bool + }{ + {name: "default for shanks", setting: Defaults, node: "shanks", cores: 24, slots: 3, percent: 50, quota: 1200, threads: 4}, + {name: "default for ace", setting: Defaults, node: "ace", cores: 24, slots: 2, percent: 75, quota: 1800, threads: 9}, + {name: "default for g14", setting: Defaults, node: "g14", cores: 16, slots: 1, percent: 50, quota: 800, threads: 8}, + {name: "a node not named", setting: Defaults, node: "novox", cores: 8, slots: 1, percent: DefaultPercent, quota: 400, threads: 4}, + {name: "percent sign and spaces", setting: " shanks = 25% ", node: "shanks", cores: 24, slots: 3, percent: 25, quota: 600, threads: 2}, + {name: "one processor at least", setting: "tiny=10", node: "tiny", cores: 2, slots: 4, percent: 10, quota: 100, threads: 1}, + {name: "zero slots counts as one", setting: "x=100", node: "x", cores: 4, slots: 0, percent: 100, quota: 400, threads: 4}, + {name: "zero refused", setting: "shanks=0", node: "shanks", refused: true}, + {name: "over a hundred refused", setting: "shanks=150", node: "shanks", refused: true}, + {name: "no number refused", setting: "shanks", node: "shanks", refused: true}, + {name: "twice refused", setting: "a=1,a=2", node: "a", refused: true}, + } { + t.Run(c.name, func(t *testing.T) { + p, err := Of(c.setting, c.node) + if c.refused { + if err == nil { + t.Fatalf("Of(%q) = %d, want refused", c.setting, p) + } + return + } + if err != nil { + t.Fatal(err) + } + if p != c.percent { + t.Fatalf("percent = %d, want %d", p, c.percent) + } + if q := Quota(p, c.cores); q != c.quota { + t.Errorf("Quota = %d, want %d", q, c.quota) + } + if th := Threads(p, c.cores, c.slots); th != c.threads { + t.Errorf("Threads = %d, want %d", th, c.threads) + } + }) + } + caps, _ := Parse("ace=75,shanks=50%") + if got := Format(caps); got != "ace=75,shanks=50" { + t.Errorf("Format = %q", got) + } + if got := WithEnv([]string{"A=1", "GOMAXPROCS=24"}, 3); !reflect.DeepEqual(got, []string{"A=1", "GOMAXPROCS=3", "UV_THREADPOOL_SIZE=3"}) { + t.Errorf("WithEnv = %q", got) + } +} + +// What the kernel holds a process to is read from its cgroup and stat files, not assumed. +func TestReadInForce(t *testing.T) { + dir := t.TempDir() + cg := filepath.Join(dir, "cg") + unit := filepath.Join(cg, "system.slice", "mesh-build-go-runtime.service") + if err := os.MkdirAll(unit, 0o755); err != nil { + t.Fatal(err) + } + for name, v := range map[string]string{"cpu.idle": "1\n", "cpu.weight": "0\n", "cpu.max": "1200000 100000\n", "io.weight": "default 1\n"} { + if err := os.WriteFile(filepath.Join(unit, name), []byte(v), 0o644); err != nil { + t.Fatal(err) + } + } + self := filepath.Join(dir, "cgroup") + stat := filepath.Join(dir, "stat") + _ = os.WriteFile(self, []byte("0::/system.slice/mesh-build-go-runtime.service\n"), 0o644) + // A command with a space and a parenthesis in it, then nice 19 in field 19. + _ = os.WriteFile(stat, []byte("42 (mesh (build) go) S 1 42 42 0 -1 4194560 1 0 0 0 0 0 0 0 39 19 8 0 1 2 3\n"), 0o644) + cgroupRoot, procSelfCgroup, procSelfStat = cg, self, stat + t.Cleanup(func() { cgroupRoot, procSelfCgroup, procSelfStat = "/sys/fs/cgroup", "/proc/self/cgroup", "/proc/self/stat" }) + + got := ReadInForce() + want := InForce{Nice: 19, Cgroup: "/system.slice/mesh-build-go-runtime.service", CPUIdle: "1", CPUWeight: "0", + CPUMax: "1200000 100000", Quota: 1200, IOWeight: "default 1"} + if !reflect.DeepEqual(got, want) { + t.Fatalf("ReadInForce = %+v\nwant %+v", got, want) + } + if q := quotaOf("max 100000"); q != 0 { + t.Errorf("no quota read as %d", q) + } +} diff --git a/go/buildstep/capacity_test.go b/go/buildstep/capacity_test.go index 5334226..8f1a7ac 100644 --- a/go/buildstep/capacity_test.go +++ b/go/buildstep/capacity_test.go @@ -36,3 +36,19 @@ func TestNodeCapacity(t *testing.T) { t.Errorf("a missing file = %d, want 0", got) } } + +// Issue 1806: a step's limits come from the cap in the same settings file, read at every call; a cap that cannot be +// read still bounds the step, at the default. +func TestNodeLimits(t *testing.T) { + file := filepath.Join(t.TempDir(), "settings.json") + os.WriteFile(file, []byte(`{"capacity":"shanks=3","cpu-cap":"shanks=25"}`), 0o600) + if l := NodeLimits(file, "shanks", 3, nil); l.Percent != 25 || l.Slots != 3 || l.Threads < 1 { + t.Errorf("from the file: %+v", l) + } + for _, body := range []string{`{"capacity":"shanks=3"}`, `{"cpu-cap":"shanks=0"}`, `not json`} { + os.WriteFile(file, []byte(body), 0o600) + if l := NodeLimits(file, "shanks", 1, nil); l.Percent != 50 { + t.Errorf("%s gives %d%%, want the default 50", body, l.Percent) + } + } +} diff --git a/go/buildstep/cpu.go b/go/buildstep/cpu.go new file mode 100644 index 0000000..754cd3e --- /dev/null +++ b/go/buildstep/cpu.go @@ -0,0 +1,40 @@ +package buildstep + +import ( + "encoding/json" + "os" + + "git.novox.be/novox/mesh-sdk/go/buildcpu" +) + +// NodeLimits is what one step on node is held to (novox/hq issue 1806): the node's processor cap from the module's +// settings file at path (buildcpu.Setting, its default when the file does not set it), the machine's processors, and +// the node's capacity, so the steps that run at once start no more threads together than the cap gives. Read at every +// call, like the capacity, so a change takes effect at the next step. A file or a setting that cannot be read gives the +// default cap and says why: a step still runs with a bound, never without one. +func NodeLimits(path, node string, slots int, logf func(string, ...any)) buildcpu.Limits { + setting := buildcpu.Defaults + if path != "" { + if raw, err := os.ReadFile(path); err != nil { + sayTo(logf, "the processor cap cannot be read, so %s builds under the default cap: %v", node, err) + } else { + var s map[string]any + if err := json.Unmarshal(raw, &s); err != nil { + sayTo(logf, "the settings file %s is not JSON, so %s builds under the default cap: %v", path, node, err) + } else if v, ok := s[buildcpu.Setting].(string); ok { + setting = v + } + } + } + l, err := buildcpu.LimitsFor(setting, node, slots) + if err != nil { + sayTo(logf, "%v, so %s builds under the default cap of %d%%", err, node, buildcpu.DefaultPercent) + } + return l +} + +func sayTo(logf func(string, ...any), format string, args ...any) { + if logf != nil { + logf(format, args...) + } +} diff --git a/go/catalogue/runtime.go b/go/catalogue/runtime.go index b0139c8..7aa9e36 100644 --- a/go/catalogue/runtime.go +++ b/go/catalogue/runtime.go @@ -329,6 +329,14 @@ func (r Resolution) ownRuntimes(with Rendering) ([]map[string]any, error) { process[HealthField] = m.RuntimeHealth healthInto(process, m, with) } + // A build seat's holder gives way to everything else on its node (novox/hq issue 1806). + yields, err := r.yieldsOf(m, with) + if err != nil { + return nil, err + } + if yields != nil { + process["yields"] = yields + } if err := artifactsInto(process, RuntimeModule, with); err != nil { return nil, err } diff --git a/go/catalogue/runtime_test.go b/go/catalogue/runtime_test.go index 16c02b4..40f6526 100644 --- a/go/catalogue/runtime_test.go +++ b/go/catalogue/runtime_test.go @@ -498,6 +498,34 @@ func TestAModuleOfItsOwnAccountIsServedByARuntimeOfItsOwn(t *testing.T) { if _, told := env[RuntimeOperatorAccount]; told { t.Error("a runtime of a module's own account is told the operator's account") } + if _, yields := own["yields"]; yields { + t.Errorf("telegram holds no build seat and yields: %v", own["yields"]) + } + // A build seat's holder yields: idle priority, under this node's share of its cpu-cap, the default where no layer + // sets one (issue 1806). + builder := telegram + builder.Claims = []Claim{{Name: "node-build-go"}} + for _, c := range []struct { + layers []Layer + share int + }{ + {nil, 50}, + {[]Layer{{From: "anchor", Values: map[string]any{"cpu-cap": "anchor=30,other=90"}}}, 30}, + } { + with.Settings = SettingsBy{"telegram": c.layers} + runtimes, err := Resolution{Node: "anchor", Modules: []Manifest{builder, goRuntime}}.ownRuntimes(with) + if err != nil { + t.Fatal(err) + } + if got := fmt.Sprint(runtimes[0]["yields"]); got != fmt.Sprintf("map[cpu-share:%d priority:idle]", c.share) { + t.Errorf("a build seat's holder under %v yields %s", c.layers, got) + } + } + with.Settings = SettingsBy{"telegram": {{From: "anchor", Values: map[string]any{"cpu-cap": "anchor=0"}}}} + if _, err := (Resolution{Node: "anchor", Modules: []Manifest{builder, goRuntime}}).ownRuntimes(with); err == nil { + t.Error("a cap of 0 percent was composed") + } + with.Settings = nil // Without the machine's runtime to run it from, it is refused in words. if _, err := (Resolution{Node: "anchor", Modules: []Manifest{telegram}}).ownRuntimes(with); err == nil { t.Error("a module of its own account composed without a runtime program") diff --git a/go/catalogue/seats.go b/go/catalogue/seats.go index 7337d8a..bc43fe1 100644 --- a/go/catalogue/seats.go +++ b/go/catalogue/seats.go @@ -49,6 +49,11 @@ type Seat struct { // order Accepts lists them (novox/hq ADR 0525 §2, ADR 0536 §4): the build seats'. Compiled, never stored, like // Receives. PulledByKind bool + // Yields says the seat's holder gives way to everything else on its node (novox/hq issue 1806): its runtime runs at + // the lowest CPU and IO priority and under its node's processor cap, the holder's setting `cpu-cap` + // (buildcpu.Setting). The build seats': a build must never slow the operator's desktop. Compiled, never stored, like + // Receives. + Yields bool // Serves carries each verb in full — name, description, schema — because a role's tools are the // mesh's to define and an agent's to call (novox/hq ADR 0132, design 33 §2). Serves []Verb @@ -256,11 +261,11 @@ var defaultSeats = append([]Seat{ // its holder taking nothing, kept on the module's own pause key (issue 1887's design limit 2). They replace // node-build-agent, retired with build-agent (ADR 0524 §5). {Name: "node-build-go", Scope: ScopeNode, Accepts: buildAskKinds(), PulledByKind: true, Serves: buildSeatVerbs("Go"), - Decision: "novox/hq ADR 0525, ADR 0536"}, + Yields: true, Decision: "novox/hq ADR 0525, ADR 0536"}, {Name: "node-build-node", Scope: ScopeNode, Accepts: buildAskKinds(), PulledByKind: true, Serves: buildSeatVerbs("Node"), - Decision: "novox/hq ADR 0525, ADR 0536"}, + Yields: true, Decision: "novox/hq ADR 0525, ADR 0536"}, {Name: "node-build-image", Scope: ScopeNode, Accepts: buildAskKinds(), PulledByKind: true, Serves: buildSeatVerbs("image"), - Decision: "novox/hq ADR 0525, ADR 0536"}, + Yields: true, Decision: "novox/hq ADR 0525, ADR 0536"}, // **The mesh's resolvers** (novox/hq ADR 0194, 0196, 0223): every node's internal domain, and // every node and container asks them and nothing else. Replicated since ADR 0223: held on more // than one machine, each answering the same names from the same roster, and every machine's diff --git a/go/catalogue/yields.go b/go/catalogue/yields.go new file mode 100644 index 0000000..e0d0034 --- /dev/null +++ b/go/catalogue/yields.go @@ -0,0 +1,41 @@ +package catalogue + +import ( + "fmt" + + "git.novox.be/novox/mesh-sdk/go/buildcpu" +) + +// yieldsOf is how the runtime of its own a module is served by gives way on this node (novox/hq issue 1806), or nil +// when it does not: a module holding a seat that yields (Seat.Yields, the build seats) runs at idle priority and under +// this node's share of its setting `cpu-cap` (buildcpu.Setting), its default when no layer sets it. +// +// In the node-engine's words (declaration.Yields), which it writes into the unit: the kernel holds the bound, not the +// module. +func (r Resolution) yieldsOf(m Manifest, with Rendering) (map[string]any, error) { + yields := false + for _, c := range m.Claims { + if s, ok := SeatNamed(c.Name); ok && s.Yields { + yields = true + } + } + if !yields { + return nil, nil + } + setting := buildcpu.Defaults + for _, s := range Effective(m, with.Settings[m.Module]) { + if s.Key == buildcpu.Setting { + v, ok := s.Value.(string) + if !ok { + return nil, fmt.Errorf("%s's %s is %v, not = pairs (novox/hq issue 1806)", + m.Module, buildcpu.Setting, s.Value) + } + setting = v + } + } + share, err := buildcpu.Of(setting, r.Node) + if err != nil { + return nil, fmt.Errorf("%s's %s: %w (novox/hq issue 1806)", m.Module, buildcpu.Setting, err) + } + return map[string]any{"priority": "idle", "cpu-share": share}, nil +} diff --git a/go/declaration/declaration.go b/go/declaration/declaration.go index cec208d..314cd58 100644 --- a/go/declaration/declaration.go +++ b/go/declaration/declaration.go @@ -916,6 +916,48 @@ type Process struct { // Health is how this resource is ready (novox/hq ADR 0240 rule 2, Phase B): one kind and its // timing, judged by the node-engine beside liveness. Absent: judged alive or not, and nothing more. Health *Health `json:"health,omitempty"` + + // Yields says this process gives way to everything else on its machine (novox/hq issue 1806): the + // runtime of a build seat's holder, whose builds must never slow the operator's desktop. Absent: the + // machine's defaults. For a process that stays up. + Yields *Yields `json:"yields,omitempty"` +} + +// YieldIdle is the one priority a process yields at: the lowest CPU and IO priority there is. +const YieldIdle = "idle" + +// Yields is how a process gives way, written into its unit by the node-engine (novox/hq issue 1806). +// +// **Priority** "idle": CPUWeight=idle (its cgroup runs only on cycles nothing else wants), IOWeight=1, +// Nice=19 for every process it starts, and the best-effort IO class at its lowest level (never the idle IO +// class, which a busy disk starves for good). **CPUShare** is the most of the machine's processors it may +// take, in percent: the node-engine writes CPUQuota for the processors the machine has, so the bound is +// the kernel's. 0 is no cap. +type Yields struct { + Priority string `json:"priority,omitempty"` + CPUShare int `json:"cpu-share,omitempty"` +} + +func (y *Yields) problems(where string, staysUp bool) []string { + if y == nil { + return nil + } + var problems []string + if !staysUp { + problems = append(problems, where+": yields is for a process that stays up") + } + switch y.Priority { + case "", YieldIdle: + default: + problems = append(problems, fmt.Sprintf("%s: yields priority %q is not one this host knows: idle", where, y.Priority)) + } + if y.CPUShare < 0 || y.CPUShare > 100 { + problems = append(problems, fmt.Sprintf("%s: yields cpu-share %d is not a percent of the machine, 0 to 100", where, y.CPUShare)) + } + if y.Priority == "" && y.CPUShare == 0 { + problems = append(problems, where+": yields says neither a priority nor a cpu-share") + } + return problems } func (d *Process) Identity() string { return d.ID } @@ -948,6 +990,7 @@ func ProcessNameProblem(name string) string { func (d *Process) validate(where string, _ bool) []string { problems := d.Health.problems(where, false, !d.RunOnce && d.Schedule == "") + problems = append(problems, d.Yields.problems(where, !d.RunOnce && d.Schedule == "")...) if problem := ProcessNameProblem(d.Name); problem != "" { problems = append(problems, where+": "+problem) } diff --git a/go/declaration/process_test.go b/go/declaration/process_test.go index 9a7c7e9..02eaff1 100644 --- a/go/declaration/process_test.go +++ b/go/declaration/process_test.go @@ -92,6 +92,20 @@ func TestTheModesAreExclusive(t *testing.T) { if problems := watching.validate("a process", false); len(problems) == 0 { t.Fatal("a scheduled process was given something to restart on, and it is never running") } + + // A process yields only while it stays up, at a priority the host knows, within the machine (issue 1806). + for name, bad := range map[string]func(*Process){ + "a step that yields": func(p *Process) { p.RunOnce = true; p.Yields = &Yields{Priority: YieldIdle} }, + "an unknown priority": func(p *Process) { p.Yields = &Yields{Priority: "low"} }, + "a share over the whole": func(p *Process) { p.Yields = &Yields{CPUShare: 150} }, + "a yield saying nothing": func(p *Process) { p.Yields = &Yields{} }, + } { + p := aProcess() + bad(p) + if problems := p.validate("a process", false); len(problems) == 0 { + t.Errorf("%s was accepted", name) + } + } } // A cadence that is not a cadence is refused near its author, rather than by a machine at the far @@ -118,6 +132,11 @@ func TestEachModeOnItsOwnIsAccepted(t *testing.T) { if problems := every.validate("a process", false); len(problems) != 0 { t.Fatalf("a scheduled process was refused: %v", problems) } + yielding := aProcess() + yielding.Yields = &Yields{Priority: YieldIdle, CPUShare: 50} + if problems := yielding.validate("a process", false); len(problems) != 0 { + t.Fatalf("a daemon yielding at idle under half the machine was refused: %v", problems) + } } // **The one that cannot be escaped, only refused.** From 76deab6c67c246d274677dc3e62ba53d258ceedb Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 26 Oct 2026 13:49:09 +0100 Subject: [PATCH 2/6] gofmt the cap's test (issue 1806) --- go/buildcpu/buildcpu_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/go/buildcpu/buildcpu_test.go b/go/buildcpu/buildcpu_test.go index d1d0549..8c36ddb 100644 --- a/go/buildcpu/buildcpu_test.go +++ b/go/buildcpu/buildcpu_test.go @@ -78,7 +78,9 @@ func TestReadInForce(t *testing.T) { // A command with a space and a parenthesis in it, then nice 19 in field 19. _ = os.WriteFile(stat, []byte("42 (mesh (build) go) S 1 42 42 0 -1 4194560 1 0 0 0 0 0 0 0 39 19 8 0 1 2 3\n"), 0o644) cgroupRoot, procSelfCgroup, procSelfStat = cg, self, stat - t.Cleanup(func() { cgroupRoot, procSelfCgroup, procSelfStat = "/sys/fs/cgroup", "/proc/self/cgroup", "/proc/self/stat" }) + t.Cleanup(func() { + cgroupRoot, procSelfCgroup, procSelfStat = "/sys/fs/cgroup", "/proc/self/cgroup", "/proc/self/stat" + }) got := ReadInForce() want := InForce{Nice: 19, Cgroup: "/system.slice/mesh-build-go-runtime.service", CPUIdle: "1", CPUWeight: "0", From 852ba81c5af152813588c83e577104fec3adddaa Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 26 Oct 2026 14:20:05 +0100 Subject: [PATCH 3/6] Build seats yield to one shared slice per node; the composer half waits for the engines (ADR 0553, issue 1806) Review round 1: the decoder type ships before any composer can emit it, so a routine SDK bump never sends yields to an engine that refuses them; the composer half moves to a later change. Every yielding process of a node goes into one top-level slice, meshyield.slice, so the three build modules share one quota instead of taking one each; the share is one node value, node-tools' build-cpu-share, and each step's threads divide it by every build slot of the node (words from the composer, a conservative fallback without them). --- go/buildcpu/buildcpu.go | 224 +++++++++++++++------------------- go/buildcpu/buildcpu_test.go | 104 ++++++++-------- go/buildstep/capacity_test.go | 21 ++-- go/buildstep/cpu.go | 37 +----- go/catalogue/holdings_test.go | 2 +- go/catalogue/runtime.go | 8 -- go/catalogue/runtime_test.go | 28 ----- go/catalogue/seats.go | 15 +-- go/catalogue/seats_test.go | 15 +++ go/catalogue/yields.go | 41 ------- go/declaration/declaration.go | 28 +++-- 11 files changed, 205 insertions(+), 318 deletions(-) delete mode 100644 go/catalogue/yields.go diff --git a/go/buildcpu/buildcpu.go b/go/buildcpu/buildcpu.go index 4df094e..06b9dec 100644 --- a/go/buildcpu/buildcpu.go +++ b/go/buildcpu/buildcpu.go @@ -1,94 +1,50 @@ // Package buildcpu is how much of a build node's processors its builds may take, and how they yield to everything else -// there (novox/hq issue 1806; the operator, 2026-10-26: "limit CPU usage on our build nodes so the total cpu usage never -// impacts the normal workings of our nodes"). +// there (novox/hq ADR 0553, issue 1806; the operator, 2026-10-26: "limit CPU usage on our build nodes so the total cpu +// usage never impacts the normal workings of our nodes ... We have 3 build agents so they should share the cpu power"). // // Three parts, each with one owner: // -// - **Priority.** A build seat's holder (catalogue.Seat.Yields) runs at the lowest CPU and IO priority there is: the -// composer gives its runtime process declaration.Yields, and the node-engine writes it into the unit as -// CPUWeight=idle, IOWeight=1, Nice=19 and the best-effort IO class at its lowest level. Interactive and service -// work always comes first; builds take the cycles nothing else wants. -// - **The cap.** The build modules' setting `cpu-cap` (Setting) gives each node the share of its processors its builds -// may take at most, as `=` pairs. The composer turns this node's share into Yields.CPUShare, and the -// node-engine into CPUQuota for the machine's processor count: a hard bound, held by the kernel. -// - **Threads.** Inside that bound a step starts no more threads than its part of it (Threads): GOMAXPROCS, `go -// test -p`, the Node test runner's concurrency. A step with more runnable threads than its quota only waits in the -// kernel's throttle, and its timeouts with it. +// - **Priority.** A build seat's holder (catalogue.Seat.Yields) runs at the lowest priority there is: the composer +// gives its runtime process declaration.Yields, and the node-engine places every yielding process of the node in +// one slice (declaration.YieldSlice) whose CPU weight is idle, with Nice=19 for every process it starts. +// Interactive and service work always comes first; builds take the cycles nothing else wants. +// - **One cap per node, shared.** The node's share of its processors is one value, the node-tools runtime's setting +// `build-cpu-share` (ShareSetting), set per node through the node's settings layer. The node-engine writes it as the +// slice's CPUQuota: one bound the kernel holds for all the node's build modules together, never one per module. +// - **Threads.** Inside that bound a step starts no more threads than its part of it (Threads): the node's share +// divided by every build slot of the node, across all its build modules (the composer hands both as words, +// ShareWord and SlotsWord). GOMAXPROCS sets every go command's -p, the compiler's concurrency, a test binary's +// processors and -parallel. package buildcpu import ( - "fmt" "os" "runtime" - "sort" "strconv" "strings" ) -// Setting is the build modules' setting of each node's processor cap: `=` pairs separated by commas, -// such as `shanks=50,ace=75`; a percent sign after the number is allowed. Each build module declares it with Defaults -// as its default. -const Setting = "cpu-cap" +// ShareSetting is the node-tools runtime's setting of the node's processor share for its builds, in percent of the +// machine's processors, 1 to 100: one value per node, set through the node's settings layer (novox/hq ADR 0553 §3). +const ShareSetting = "build-cpu-share" -// Defaults is the setting's default: the operator's two desktops (shanks, g14) keep half their processors for -// themselves at all times, and ace, a server, builds on three quarters. A node it does not name has DefaultPercent. -const Defaults = "shanks=50,g14=50,ace=75" - -// DefaultPercent is the cap of a node the setting does not name. +// DefaultPercent is the share where nothing sets one: half the machine, so a desktop keeps half its processors for +// itself whatever builds. const DefaultPercent = 50 -// Parse reads the setting's pairs. A percent outside 1 to 100 is refused: 0 would stop every build without pausing the -// seat (the capacity setting says that, with 0 asks), and more than 100 is no cap. -func Parse(s string) (map[string]int, error) { - out := map[string]int{} - for _, pair := range strings.Split(s, ",") { - pair = strings.TrimSpace(pair) - if pair == "" { - continue - } - node, value, ok := strings.Cut(pair, "=") - node = strings.TrimSpace(node) - value = strings.TrimSuffix(strings.TrimSpace(value), "%") - n, err := strconv.Atoi(strings.TrimSpace(value)) - if !ok || node == "" || err != nil || n < 1 || n > 100 { - return nil, fmt.Errorf("the processor cap %q is not =", pair) - } - if _, dup := out[node]; dup { - return nil, fmt.Errorf("the processor cap names %s twice", node) - } - out[node] = n - } - return out, nil -} +// BuildSeats is how many seats yield (catalogue.Seat.Yields): one build module per language. A step that is told no +// node-wide slot count assumes every one of them runs its full capacity beside it. +const BuildSeats = 3 -// Of is node's cap in percent of its processors: DefaultPercent for a node the setting does not name. -func Of(setting, node string) (int, error) { - caps, err := Parse(setting) - if err != nil { - return 0, err - } - if n, ok := caps[node]; ok { - return n, nil - } - return DefaultPercent, nil -} +// The words the composer gives every yielding module (novox/hq ADR 0553 §4): the node's share, and every build slot +// of the node across all its yielding modules. +const ( + ShareWord = "MESH_BUILD_CPU_SHARE" + SlotsWord = "MESH_BUILD_SLOTS" +) -// Format is a cap map in the setting's own words, sorted by node. -func Format(caps map[string]int) string { - nodes := make([]string, 0, len(caps)) - for n := range caps { - nodes = append(nodes, n) - } - sort.Strings(nodes) - parts := make([]string, 0, len(nodes)) - for _, n := range nodes { - parts = append(parts, fmt.Sprintf("%s=%d", n, caps[n])) - } - return strings.Join(parts, ",") -} - -// Quota is the unit's CPUQuota for a share of a machine with cores processors: systemd counts a quota in percent of -// one processor, so half of 24 is 1200%. At least 100%, one processor, so a cap on a small machine never stops a step. +// Quota is a CPUQuota for a share of a machine with cores processors: systemd counts a quota in percent of one +// processor, so half of 24 is 1200%. At least 100%, one processor, so a share on a small machine never stops a step. func Quota(percent, cores int) int { q := percent * cores if q < 100 { @@ -97,14 +53,14 @@ func Quota(percent, cores int) int { return q } -// Threads is how many threads one step may run at once: its part of the cap when slots steps run beside it, at least -// one. Rounded down, so the steps together stay inside the quota. +// Threads is how many threads one step may run at once: its part of the share when slots steps may run on the node +// at once, at least one. Rounded down, so the steps together stay inside the quota. func Threads(percent, cores, slots int) int { if slots < 1 { slots = 1 } if percent < 1 || percent > 100 { - percent = 100 + percent = DefaultPercent } t := cores * percent / 100 / slots if t < 1 { @@ -115,28 +71,36 @@ func Threads(percent, cores, slots int) int { // Limits is what one step is held to on this node, as a status tool says it. type Limits struct { - // Percent is the node's cap in percent of its processors, and Setting the words it came from. - Percent int `json:"cap-percent"` - Setting string `json:"cap-setting"` + // Percent is the node's share in percent of its processors; From says where it was read: the composer's word, + // or the default. + Percent int `json:"share-percent"` + From string `json:"share-from"` // Cores is the machine's processor count. Cores int `json:"cores"` - // Slots is the node's build capacity: steps at once. - Slots int `json:"slots"` + // Slots is every build slot of the node, across all its build modules; SlotsFrom where it was read. + Slots int `json:"node-slots"` + SlotsFrom string `json:"slots-from"` // Threads is what each step is given (Threads). Threads int `json:"threads-per-step"` } -// LimitsFor is the limits of a step on node, from the cap setting and the node's capacity. A setting that cannot be read -// gives the default cap and says why in the error, so a step still runs with a bound rather than none. -func LimitsFor(setting, node string, slots int) (Limits, error) { - l := Limits{Setting: setting, Cores: runtime.NumCPU(), Slots: slots} - p, err := Of(setting, node) - if err != nil { - p = DefaultPercent +// FromWords is the limits of a step from the words the composer gave this process (ShareWord, SlotsWord). Where a word +// is absent or not a number: the default share, and own × BuildSeats slots. A module told nothing assumes every build +// module of the node runs its full capacity beside it, so the steps together never take more than the default share. +func FromWords(getenv func(string) string, own int) Limits { + if own < 1 { + own = 1 } - l.Percent = p - l.Threads = Threads(p, l.Cores, slots) - return l, err + l := Limits{Percent: DefaultPercent, From: "default", Cores: runtime.NumCPU(), Slots: own * BuildSeats, + SlotsFrom: "own capacity × " + strconv.Itoa(BuildSeats) + " build seats"} + if n, err := strconv.Atoi(strings.TrimSpace(getenv(ShareWord))); err == nil && n >= 1 && n <= 100 { + l.Percent, l.From = n, ShareWord + } + if n, err := strconv.Atoi(strings.TrimSpace(getenv(SlotsWord))); err == nil && n >= 1 { + l.Slots, l.SlotsFrom = n, SlotsWord + } + l.Threads = Threads(l.Percent, l.Cores, l.Slots) + return l } // Env is the environment a step's commands are given so they start no more threads than they may: GOMAXPROCS for every @@ -150,32 +114,30 @@ func Env(threads int) []string { return []string{"GOMAXPROCS=" + t, "UV_THREADPOOL_SIZE=" + t} } -// WithEnv is env with Env(threads) in place of any GOMAXPROCS or UV_THREADPOOL_SIZE it carried. -func WithEnv(env []string, threads int) []string { - out := make([]string, 0, len(env)+2) - for _, kv := range env { - if strings.HasPrefix(kv, "GOMAXPROCS=") || strings.HasPrefix(kv, "UV_THREADPOOL_SIZE=") { - continue - } - out = append(out, kv) - } - return append(out, Env(threads)...) +// InForce is what the kernel holds this process to, read rather than assumed (evidence over assertion): its nice +// level, its cgroup (the unit) and the cgroup's parent (the slice the node-engine placed it in), each one's CPU weight, +// idle mark and quota. A field the machine does not answer is left empty and Unread says which. +// +// The IO weight is read but binds only where the disk's scheduler honours it (bfq, or the io.cost controller); under +// the scheduler `none`, the usual one for NVMe, it does nothing, and the CPU bound is what holds. +type InForce struct { + Nice int `json:"nice"` + Cgroup string `json:"cgroup,omitempty"` + Unit Group `json:"unit"` + Slice Group `json:"slice"` + // Quota is the tighter of the two groups' cpu.max as a percent of one processor, 0 with none. + Quota int `json:"quota-percent,omitempty"` + Unread []string `json:"unread,omitempty"` } -// InForce is what the kernel holds this process to, read rather than assumed (evidence over assertion): its nice -// level, its cgroup, the cgroup's CPU weight, idle mark and quota. A field the machine does not answer is left empty -// and Unread says which. -type InForce struct { - Nice int `json:"nice"` - Cgroup string `json:"cgroup,omitempty"` +// Group is one cgroup's controls. +type Group struct { + Path string `json:"path,omitempty"` CPUIdle string `json:"cpu-idle,omitempty"` CPUWeight string `json:"cpu-weight,omitempty"` // CPUMax is the cgroup's cpu.max: " ", or "max" with no quota. - CPUMax string `json:"cpu-max,omitempty"` - // Quota is CPUMax as a percent of one processor, 0 with none. - Quota int `json:"quota-percent,omitempty"` - IOWeight string `json:"io-weight,omitempty"` - Unread []string `json:"unread,omitempty"` + CPUMax string `json:"cpu-max,omitempty"` + IOWeight string `json:"io-weight,omitempty"` } // cgroupRoot is where the unified hierarchy is mounted. A variable so a test can point it elsewhere. @@ -184,6 +146,9 @@ var cgroupRoot = "/sys/fs/cgroup" // procSelfCgroup is this process's cgroup file. A variable so a test can point it elsewhere. var procSelfCgroup = "/proc/self/cgroup" +// procSelfStat is this process's stat file. A variable so a test can point it elsewhere. +var procSelfStat = "/proc/self/stat" + // ReadInForce reads what the kernel holds this process to. func ReadInForce() InForce { var f InForce @@ -202,20 +167,28 @@ func ReadInForce() InForce { f.Unread = append(f.Unread, "cgroup: not on the unified hierarchy") return f } - dir := cgroupRoot + f.Cgroup - read := func(name string) string { - b, err := os.ReadFile(dir + "/" + name) - if err != nil { - f.Unread = append(f.Unread, name+": "+err.Error()) - return "" + read := func(path string) Group { + g := Group{Path: path} + value := func(name string) string { + b, err := os.ReadFile(cgroupRoot + path + "/" + name) + if err != nil { + f.Unread = append(f.Unread, path+"/"+name+": "+err.Error()) + return "" + } + return strings.TrimSpace(string(b)) + } + g.CPUIdle, g.CPUWeight, g.CPUMax, g.IOWeight = value("cpu.idle"), value("cpu.weight"), value("cpu.max"), value("io.weight") + return g + } + f.Unit = read(f.Cgroup) + if i := strings.LastIndex(f.Cgroup, "/"); i > 0 { + f.Slice = read(f.Cgroup[:i]) + } + for _, q := range []int{quotaOf(f.Unit.CPUMax), quotaOf(f.Slice.CPUMax)} { + if q > 0 && (f.Quota == 0 || q < f.Quota) { + f.Quota = q } - return strings.TrimSpace(string(b)) } - f.CPUIdle = read("cpu.idle") - f.CPUWeight = read("cpu.weight") - f.CPUMax = read("cpu.max") - f.IOWeight = read("io.weight") - f.Quota = quotaOf(f.CPUMax) return f } @@ -233,9 +206,6 @@ func quotaOf(cpuMax string) int { return q * 100 / p } -// procSelfStat is this process's stat file. A variable so a test can point it elsewhere. -var procSelfStat = "/proc/self/stat" - // nice is this process's nice level from its stat file (the 19th field, counted after the command's closing // parenthesis, which may itself hold spaces); 0 when it cannot be read. func nice() int { diff --git a/go/buildcpu/buildcpu_test.go b/go/buildcpu/buildcpu_test.go index 8c36ddb..cdf7b91 100644 --- a/go/buildcpu/buildcpu_test.go +++ b/go/buildcpu/buildcpu_test.go @@ -4,77 +4,72 @@ import ( "os" "path/filepath" "reflect" + "runtime" "testing" ) -// The cap's words, a node's share, the unit quota and the threads a step is given, in one table (issue 1806). -func TestCapQuotaAndThreads(t *testing.T) { +// The node's share as a quota, the threads one step is given across every build slot of the node, and the words the +// composer hands, in one table (ADR 0553, issue 1806). +func TestShareQuotaAndThreads(t *testing.T) { for _, c := range []struct { - name, setting, node string - cores, slots int - percent, quota int - threads int - refused bool + name string + percent, cores, slots int + quota, threads int }{ - {name: "default for shanks", setting: Defaults, node: "shanks", cores: 24, slots: 3, percent: 50, quota: 1200, threads: 4}, - {name: "default for ace", setting: Defaults, node: "ace", cores: 24, slots: 2, percent: 75, quota: 1800, threads: 9}, - {name: "default for g14", setting: Defaults, node: "g14", cores: 16, slots: 1, percent: 50, quota: 800, threads: 8}, - {name: "a node not named", setting: Defaults, node: "novox", cores: 8, slots: 1, percent: DefaultPercent, quota: 400, threads: 4}, - {name: "percent sign and spaces", setting: " shanks = 25% ", node: "shanks", cores: 24, slots: 3, percent: 25, quota: 600, threads: 2}, - {name: "one processor at least", setting: "tiny=10", node: "tiny", cores: 2, slots: 4, percent: 10, quota: 100, threads: 1}, - {name: "zero slots counts as one", setting: "x=100", node: "x", cores: 4, slots: 0, percent: 100, quota: 400, threads: 4}, - {name: "zero refused", setting: "shanks=0", node: "shanks", refused: true}, - {name: "over a hundred refused", setting: "shanks=150", node: "shanks", refused: true}, - {name: "no number refused", setting: "shanks", node: "shanks", refused: true}, - {name: "twice refused", setting: "a=1,a=2", node: "a", refused: true}, + {"shanks: half of 24 across 9 slots", 50, 24, 9, 1200, 1}, + {"shanks: half of 24 across 4 slots", 50, 24, 4, 1200, 3}, + {"ace: three quarters of 24 across 6 slots", 75, 24, 6, 1800, 3}, + {"g14: half of 16 across 1 slot", 50, 16, 1, 800, 8}, + {"one processor at least", 10, 2, 4, 100, 1}, + {"no slots counts as one", 100, 4, 0, 400, 4}, + {"a share out of range is the default", 0, 8, 1, 0, 4}, } { - t.Run(c.name, func(t *testing.T) { - p, err := Of(c.setting, c.node) - if c.refused { - if err == nil { - t.Fatalf("Of(%q) = %d, want refused", c.setting, p) - } - return + if c.quota != 0 { + if q := Quota(c.percent, c.cores); q != c.quota { + t.Errorf("%s: Quota = %d, want %d", c.name, q, c.quota) } - if err != nil { - t.Fatal(err) - } - if p != c.percent { - t.Fatalf("percent = %d, want %d", p, c.percent) - } - if q := Quota(p, c.cores); q != c.quota { - t.Errorf("Quota = %d, want %d", q, c.quota) - } - if th := Threads(p, c.cores, c.slots); th != c.threads { - t.Errorf("Threads = %d, want %d", th, c.threads) - } - }) + } + if th := Threads(c.percent, c.cores, c.slots); th != c.threads { + t.Errorf("%s: Threads = %d, want %d", c.name, th, c.threads) + } } - caps, _ := Parse("ace=75,shanks=50%") - if got := Format(caps); got != "ace=75,shanks=50" { - t.Errorf("Format = %q", got) + words := map[string]string{ShareWord: "75", SlotsWord: "6"} + l := FromWords(func(k string) string { return words[k] }, 2) + if l.Percent != 75 || l.From != ShareWord || l.Slots != 6 || l.SlotsFrom != SlotsWord || l.Threads != Threads(75, runtime.NumCPU(), 6) { + t.Errorf("from the words: %+v", l) } - if got := WithEnv([]string{"A=1", "GOMAXPROCS=24"}, 3); !reflect.DeepEqual(got, []string{"A=1", "GOMAXPROCS=3", "UV_THREADPOOL_SIZE=3"}) { - t.Errorf("WithEnv = %q", got) + // Told nothing (or nonsense), a module assumes every build seat beside it at its own capacity, at the default share. + words = map[string]string{ShareWord: "250", SlotsWord: "many"} + if l := FromWords(func(k string) string { return words[k] }, 3); l.Percent != DefaultPercent || l.Slots != 3*BuildSeats || l.From != "default" { + t.Errorf("without usable words: %+v", l) + } + if got := Env(0); !reflect.DeepEqual(got, []string{"GOMAXPROCS=1", "UV_THREADPOOL_SIZE=1"}) { + t.Errorf("Env = %q", got) } } -// What the kernel holds a process to is read from its cgroup and stat files, not assumed. +// What the kernel holds a process to is read from its unit's and its slice's cgroup and its stat file, not assumed. func TestReadInForce(t *testing.T) { dir := t.TempDir() cg := filepath.Join(dir, "cg") - unit := filepath.Join(cg, "system.slice", "mesh-build-go-runtime.service") + slice := filepath.Join(cg, "meshyield.slice") + unit := filepath.Join(slice, "mesh-build-go-runtime.service") if err := os.MkdirAll(unit, 0o755); err != nil { t.Fatal(err) } - for name, v := range map[string]string{"cpu.idle": "1\n", "cpu.weight": "0\n", "cpu.max": "1200000 100000\n", "io.weight": "default 1\n"} { - if err := os.WriteFile(filepath.Join(unit, name), []byte(v), 0o644); err != nil { - t.Fatal(err) + for at, files := range map[string]map[string]string{ + slice: {"cpu.idle": "1\n", "cpu.weight": "0\n", "cpu.max": "1200000 100000\n", "io.weight": "default 1\n"}, + unit: {"cpu.idle": "0\n", "cpu.weight": "100\n", "cpu.max": "max 100000\n", "io.weight": "default 100\n"}, + } { + for name, v := range files { + if err := os.WriteFile(filepath.Join(at, name), []byte(v), 0o644); err != nil { + t.Fatal(err) + } } } self := filepath.Join(dir, "cgroup") stat := filepath.Join(dir, "stat") - _ = os.WriteFile(self, []byte("0::/system.slice/mesh-build-go-runtime.service\n"), 0o644) + _ = os.WriteFile(self, []byte("0::/meshyield.slice/mesh-build-go-runtime.service\n"), 0o644) // A command with a space and a parenthesis in it, then nice 19 in field 19. _ = os.WriteFile(stat, []byte("42 (mesh (build) go) S 1 42 42 0 -1 4194560 1 0 0 0 0 0 0 0 39 19 8 0 1 2 3\n"), 0o644) cgroupRoot, procSelfCgroup, procSelfStat = cg, self, stat @@ -83,12 +78,13 @@ func TestReadInForce(t *testing.T) { }) got := ReadInForce() - want := InForce{Nice: 19, Cgroup: "/system.slice/mesh-build-go-runtime.service", CPUIdle: "1", CPUWeight: "0", - CPUMax: "1200000 100000", Quota: 1200, IOWeight: "default 1"} + want := InForce{Nice: 19, Cgroup: "/meshyield.slice/mesh-build-go-runtime.service", + Unit: Group{Path: "/meshyield.slice/mesh-build-go-runtime.service", CPUIdle: "0", CPUWeight: "100", + CPUMax: "max 100000", IOWeight: "default 100"}, + Slice: Group{Path: "/meshyield.slice", CPUIdle: "1", CPUWeight: "0", CPUMax: "1200000 100000", + IOWeight: "default 1"}, + Quota: 1200} if !reflect.DeepEqual(got, want) { t.Fatalf("ReadInForce = %+v\nwant %+v", got, want) } - if q := quotaOf("max 100000"); q != 0 { - t.Errorf("no quota read as %d", q) - } } diff --git a/go/buildstep/capacity_test.go b/go/buildstep/capacity_test.go index 8f1a7ac..632cdfc 100644 --- a/go/buildstep/capacity_test.go +++ b/go/buildstep/capacity_test.go @@ -37,18 +37,17 @@ func TestNodeCapacity(t *testing.T) { } } -// Issue 1806: a step's limits come from the cap in the same settings file, read at every call; a cap that cannot be -// read still bounds the step, at the default. +// ADR 0553 §4: a step's limits come from the composer's words; without them, every build seat at the module's own +// capacity at the default share. func TestNodeLimits(t *testing.T) { - file := filepath.Join(t.TempDir(), "settings.json") - os.WriteFile(file, []byte(`{"capacity":"shanks=3","cpu-cap":"shanks=25"}`), 0o600) - if l := NodeLimits(file, "shanks", 3, nil); l.Percent != 25 || l.Slots != 3 || l.Threads < 1 { - t.Errorf("from the file: %+v", l) + t.Setenv("MESH_BUILD_CPU_SHARE", "") + t.Setenv("MESH_BUILD_SLOTS", "") + if l := NodeLimits(3); l.Percent != 50 || l.Slots != 9 { + t.Errorf("without words: %+v", l) } - for _, body := range []string{`{"capacity":"shanks=3"}`, `{"cpu-cap":"shanks=0"}`, `not json`} { - os.WriteFile(file, []byte(body), 0o600) - if l := NodeLimits(file, "shanks", 1, nil); l.Percent != 50 { - t.Errorf("%s gives %d%%, want the default 50", body, l.Percent) - } + t.Setenv("MESH_BUILD_CPU_SHARE", "75") + t.Setenv("MESH_BUILD_SLOTS", "5") + if l := NodeLimits(3); l.Percent != 75 || l.Slots != 5 || l.Threads < 1 { + t.Errorf("from the words: %+v", l) } } diff --git a/go/buildstep/cpu.go b/go/buildstep/cpu.go index 754cd3e..6147202 100644 --- a/go/buildstep/cpu.go +++ b/go/buildstep/cpu.go @@ -1,40 +1,15 @@ package buildstep import ( - "encoding/json" "os" "git.novox.be/novox/mesh-sdk/go/buildcpu" ) -// NodeLimits is what one step on node is held to (novox/hq issue 1806): the node's processor cap from the module's -// settings file at path (buildcpu.Setting, its default when the file does not set it), the machine's processors, and -// the node's capacity, so the steps that run at once start no more threads together than the cap gives. Read at every -// call, like the capacity, so a change takes effect at the next step. A file or a setting that cannot be read gives the -// default cap and says why: a step still runs with a bound, never without one. -func NodeLimits(path, node string, slots int, logf func(string, ...any)) buildcpu.Limits { - setting := buildcpu.Defaults - if path != "" { - if raw, err := os.ReadFile(path); err != nil { - sayTo(logf, "the processor cap cannot be read, so %s builds under the default cap: %v", node, err) - } else { - var s map[string]any - if err := json.Unmarshal(raw, &s); err != nil { - sayTo(logf, "the settings file %s is not JSON, so %s builds under the default cap: %v", path, node, err) - } else if v, ok := s[buildcpu.Setting].(string); ok { - setting = v - } - } - } - l, err := buildcpu.LimitsFor(setting, node, slots) - if err != nil { - sayTo(logf, "%v, so %s builds under the default cap of %d%%", err, node, buildcpu.DefaultPercent) - } - return l -} - -func sayTo(logf func(string, ...any), format string, args ...any) { - if logf != nil { - logf(format, args...) - } +// NodeLimits is what one step of a build module is held to (novox/hq ADR 0553 §4): the node's processor share and every +// build slot of the node, from the words the composer gives the module (buildcpu.ShareWord, buildcpu.SlotsWord), and the +// machine's processors. A module told neither assumes every build seat beside it at its own capacity, own, so its steps +// stay within the default share with the others'. +func NodeLimits(own int) buildcpu.Limits { + return buildcpu.FromWords(os.Getenv, own) } diff --git a/go/catalogue/holdings_test.go b/go/catalogue/holdings_test.go index c8a15b5..dedc566 100644 --- a/go/catalogue/holdings_test.go +++ b/go/catalogue/holdings_test.go @@ -172,7 +172,7 @@ func TestAStoreRowWithoutAProtocolKeepsTheCompiledOne(t *testing.T) { t.Cleanup(func() { UseSeats(was) }) UseSeats([]Seat{{Name: "node-build-go", Scope: ScopeNode, Decision: "row"}}) got, ok := SeatNamed("node-build-go") - if !ok || len(got.Accepts) == 0 || !got.PulledByKind { + if !ok || len(got.Accepts) == 0 || !got.PulledByKind || !got.Yields { t.Fatalf("the Go build seat lost what it accepts when loaded from the store: %+v", got) } if got.Decision != "row" { diff --git a/go/catalogue/runtime.go b/go/catalogue/runtime.go index 7aa9e36..b0139c8 100644 --- a/go/catalogue/runtime.go +++ b/go/catalogue/runtime.go @@ -329,14 +329,6 @@ func (r Resolution) ownRuntimes(with Rendering) ([]map[string]any, error) { process[HealthField] = m.RuntimeHealth healthInto(process, m, with) } - // A build seat's holder gives way to everything else on its node (novox/hq issue 1806). - yields, err := r.yieldsOf(m, with) - if err != nil { - return nil, err - } - if yields != nil { - process["yields"] = yields - } if err := artifactsInto(process, RuntimeModule, with); err != nil { return nil, err } diff --git a/go/catalogue/runtime_test.go b/go/catalogue/runtime_test.go index 40f6526..16c02b4 100644 --- a/go/catalogue/runtime_test.go +++ b/go/catalogue/runtime_test.go @@ -498,34 +498,6 @@ func TestAModuleOfItsOwnAccountIsServedByARuntimeOfItsOwn(t *testing.T) { if _, told := env[RuntimeOperatorAccount]; told { t.Error("a runtime of a module's own account is told the operator's account") } - if _, yields := own["yields"]; yields { - t.Errorf("telegram holds no build seat and yields: %v", own["yields"]) - } - // A build seat's holder yields: idle priority, under this node's share of its cpu-cap, the default where no layer - // sets one (issue 1806). - builder := telegram - builder.Claims = []Claim{{Name: "node-build-go"}} - for _, c := range []struct { - layers []Layer - share int - }{ - {nil, 50}, - {[]Layer{{From: "anchor", Values: map[string]any{"cpu-cap": "anchor=30,other=90"}}}, 30}, - } { - with.Settings = SettingsBy{"telegram": c.layers} - runtimes, err := Resolution{Node: "anchor", Modules: []Manifest{builder, goRuntime}}.ownRuntimes(with) - if err != nil { - t.Fatal(err) - } - if got := fmt.Sprint(runtimes[0]["yields"]); got != fmt.Sprintf("map[cpu-share:%d priority:idle]", c.share) { - t.Errorf("a build seat's holder under %v yields %s", c.layers, got) - } - } - with.Settings = SettingsBy{"telegram": {{From: "anchor", Values: map[string]any{"cpu-cap": "anchor=0"}}}} - if _, err := (Resolution{Node: "anchor", Modules: []Manifest{builder, goRuntime}}).ownRuntimes(with); err == nil { - t.Error("a cap of 0 percent was composed") - } - with.Settings = nil // Without the machine's runtime to run it from, it is refused in words. if _, err := (Resolution{Node: "anchor", Modules: []Manifest{telegram}}).ownRuntimes(with); err == nil { t.Error("a module of its own account composed without a runtime program") diff --git a/go/catalogue/seats.go b/go/catalogue/seats.go index bc43fe1..805d0be 100644 --- a/go/catalogue/seats.go +++ b/go/catalogue/seats.go @@ -49,10 +49,10 @@ type Seat struct { // order Accepts lists them (novox/hq ADR 0525 §2, ADR 0536 §4): the build seats'. Compiled, never stored, like // Receives. PulledByKind bool - // Yields says the seat's holder gives way to everything else on its node (novox/hq issue 1806): its runtime runs at - // the lowest CPU and IO priority and under its node's processor cap, the holder's setting `cpu-cap` - // (buildcpu.Setting). The build seats': a build must never slow the operator's desktop. Compiled, never stored, like - // Receives. + // Yields says the seat's holder gives way to everything else on its node (novox/hq ADR 0553): its runtime runs at + // idle priority in the node's one yielding slice, under the node's one processor share (buildcpu.ShareSetting) that + // every yielding holder of the node shares. The build seats': a build must never slow the operator's desktop. + // Compiled, never stored, like Receives. Yields bool // Serves carries each verb in full — name, description, schema — because a role's tools are the // mesh's to define and an agent's to call (novox/hq ADR 0132, design 33 §2). @@ -261,11 +261,11 @@ var defaultSeats = append([]Seat{ // its holder taking nothing, kept on the module's own pause key (issue 1887's design limit 2). They replace // node-build-agent, retired with build-agent (ADR 0524 §5). {Name: "node-build-go", Scope: ScopeNode, Accepts: buildAskKinds(), PulledByKind: true, Serves: buildSeatVerbs("Go"), - Yields: true, Decision: "novox/hq ADR 0525, ADR 0536"}, + Yields: true, Decision: "novox/hq ADR 0525, ADR 0536, ADR 0553"}, {Name: "node-build-node", Scope: ScopeNode, Accepts: buildAskKinds(), PulledByKind: true, Serves: buildSeatVerbs("Node"), - Yields: true, Decision: "novox/hq ADR 0525, ADR 0536"}, + Yields: true, Decision: "novox/hq ADR 0525, ADR 0536, ADR 0553"}, {Name: "node-build-image", Scope: ScopeNode, Accepts: buildAskKinds(), PulledByKind: true, Serves: buildSeatVerbs("image"), - Yields: true, Decision: "novox/hq ADR 0525, ADR 0536"}, + Yields: true, Decision: "novox/hq ADR 0525, ADR 0536, ADR 0553"}, // **The mesh's resolvers** (novox/hq ADR 0194, 0196, 0223): every node's internal domain, and // every node and container asks them and nothing else. Replicated since ADR 0223: held on more // than one machine, each answering the same names from the same roster, and every machine's @@ -502,6 +502,7 @@ func UseSeats(s []Seat) { row.Paths = d.Paths row.Replicated = d.Replicated row.PulledByKind = d.PulledByKind + row.Yields = d.Yields // never stored (ADR 0553) row.Serves = optionalAsCompiled(row.Serves, d.Serves) } merged = append(merged, row) diff --git a/go/catalogue/seats_test.go b/go/catalogue/seats_test.go index caa5a25..4e65fca 100644 --- a/go/catalogue/seats_test.go +++ b/go/catalogue/seats_test.go @@ -4,6 +4,8 @@ import ( "regexp" "strings" "testing" + + "git.novox.be/novox/mesh-sdk/go/buildcpu" ) // Defends novox/hq ADR 0110: a seat is a module assignment from a closed set. @@ -324,6 +326,9 @@ func TestNodeBuildAgentIsRetiredForTheBuildSeats(t *testing.T) { if !ok || s.Scope != ScopeNode || !s.PulledByKind || strings.Join(s.Accepts, ",") != "delivery,check,recheck" { t.Fatalf("%s = %+v", name, s) } + if !s.Yields { + t.Errorf("%s does not yield (ADR 0553)", name) + } var verbs []string for _, v := range s.Serves { verbs = append(verbs, v.Name) @@ -332,4 +337,14 @@ func TestNodeBuildAgentIsRetiredForTheBuildSeats(t *testing.T) { t.Errorf("%s serves %v", name, verbs) } } + // The build seats are the seats that yield, as many as a step told nothing assumes beside it (ADR 0553 §4). + yielding := 0 + for _, s := range Seats() { + if s.Yields { + yielding++ + } + } + if yielding != buildcpu.BuildSeats { + t.Errorf("%d seats yield, and buildcpu.BuildSeats says %d", yielding, buildcpu.BuildSeats) + } } diff --git a/go/catalogue/yields.go b/go/catalogue/yields.go deleted file mode 100644 index e0d0034..0000000 --- a/go/catalogue/yields.go +++ /dev/null @@ -1,41 +0,0 @@ -package catalogue - -import ( - "fmt" - - "git.novox.be/novox/mesh-sdk/go/buildcpu" -) - -// yieldsOf is how the runtime of its own a module is served by gives way on this node (novox/hq issue 1806), or nil -// when it does not: a module holding a seat that yields (Seat.Yields, the build seats) runs at idle priority and under -// this node's share of its setting `cpu-cap` (buildcpu.Setting), its default when no layer sets it. -// -// In the node-engine's words (declaration.Yields), which it writes into the unit: the kernel holds the bound, not the -// module. -func (r Resolution) yieldsOf(m Manifest, with Rendering) (map[string]any, error) { - yields := false - for _, c := range m.Claims { - if s, ok := SeatNamed(c.Name); ok && s.Yields { - yields = true - } - } - if !yields { - return nil, nil - } - setting := buildcpu.Defaults - for _, s := range Effective(m, with.Settings[m.Module]) { - if s.Key == buildcpu.Setting { - v, ok := s.Value.(string) - if !ok { - return nil, fmt.Errorf("%s's %s is %v, not = pairs (novox/hq issue 1806)", - m.Module, buildcpu.Setting, s.Value) - } - setting = v - } - } - share, err := buildcpu.Of(setting, r.Node) - if err != nil { - return nil, fmt.Errorf("%s's %s: %w (novox/hq issue 1806)", m.Module, buildcpu.Setting, err) - } - return map[string]any{"priority": "idle", "cpu-share": share}, nil -} diff --git a/go/declaration/declaration.go b/go/declaration/declaration.go index 314cd58..3d70e00 100644 --- a/go/declaration/declaration.go +++ b/go/declaration/declaration.go @@ -917,22 +917,30 @@ type Process struct { // timing, judged by the node-engine beside liveness. Absent: judged alive or not, and nothing more. Health *Health `json:"health,omitempty"` - // Yields says this process gives way to everything else on its machine (novox/hq issue 1806): the - // runtime of a build seat's holder, whose builds must never slow the operator's desktop. Absent: the - // machine's defaults. For a process that stays up. + // Yields says this process gives way to everything else on its machine (novox/hq ADR 0553, issue 1806): the + // runtime of a build seat's holder, whose builds must never slow the operator's desktop. Absent: the machine's + // defaults. For a process that stays up. Yields *Yields `json:"yields,omitempty"` } -// YieldIdle is the one priority a process yields at: the lowest CPU and IO priority there is. +// YieldIdle is the one priority a process yields at: the lowest there is. const YieldIdle = "idle" -// Yields is how a process gives way, written into its unit by the node-engine (novox/hq issue 1806). +// YieldSlice is the one slice the node-engine places every yielding process of its node in (novox/hq ADR 0553 §2): a +// slice at the top of the hierarchy, beside the operator's user slice and the system's, so its idle weight and its +// quota bind against everything else on the machine. No dash in its name: a dash makes a slice a child of the slice +// named before it, where its weight would count only against its siblings. +const YieldSlice = "meshyield.slice" + +// Yields is how a process gives way, written by the node-engine (novox/hq ADR 0553). // -// **Priority** "idle": CPUWeight=idle (its cgroup runs only on cycles nothing else wants), IOWeight=1, -// Nice=19 for every process it starts, and the best-effort IO class at its lowest level (never the idle IO -// class, which a busy disk starves for good). **CPUShare** is the most of the machine's processors it may -// take, in percent: the node-engine writes CPUQuota for the processors the machine has, so the bound is -// the kernel's. 0 is no cap. +// **Priority** "idle": the process runs in YieldSlice, whose CPUWeight is idle (it runs only on cycles nothing else +// wants) and whose IOWeight is 1, with Nice=19 and the lowest best-effort IO level for every process it starts. The IO +// controls bind only where the disk's scheduler honours them (bfq, or io.cost for the weight); under `none`, the +// usual one for NVMe, they do nothing, and the CPU bound is what holds. **CPUShare** is the most of the machine's +// processors the slice may take, in percent, shared by every yielding process of the node together: the node-engine +// writes it as the slice's CPUQuota for the processors the machine has, so the bound is the kernel's. Every yielding +// process of one node carries the same share: the composer gives the node's one value (ADR 0553 §3). 0 is no cap. type Yields struct { Priority string `json:"priority,omitempty"` CPUShare int `json:"cpu-share,omitempty"` From 527d87613eb23ee0f3b9404f0a10fbb824ab4ee8 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 26 Oct 2026 14:39:39 +0100 Subject: [PATCH 4/6] Without the composer's share a step has no thread limit; with it, the share over the module's own capacity (ADR 0553 round 2) Review round 2: change A must not slow builds before any slice exists, so a module given no share sets no GOMAXPROCS. The slice's quota bounds the total; threads only size one step, from the node's share over the module's own capacity (shanks: 24 x 50% / 3 = 4). The share is the node-engine's own config. --- go/buildcpu/buildcpu.go | 59 ++++++++++++++--------------------- go/buildcpu/buildcpu_test.go | 30 +++++++++--------- go/buildstep/capacity_test.go | 13 +++----- go/buildstep/cpu.go | 7 ++--- go/catalogue/seats_test.go | 12 +++---- 5 files changed, 51 insertions(+), 70 deletions(-) diff --git a/go/buildcpu/buildcpu.go b/go/buildcpu/buildcpu.go index 06b9dec..dcd2f36 100644 --- a/go/buildcpu/buildcpu.go +++ b/go/buildcpu/buildcpu.go @@ -8,13 +8,14 @@ // gives its runtime process declaration.Yields, and the node-engine places every yielding process of the node in // one slice (declaration.YieldSlice) whose CPU weight is idle, with Nice=19 for every process it starts. // Interactive and service work always comes first; builds take the cycles nothing else wants. -// - **One cap per node, shared.** The node's share of its processors is one value, the node-tools runtime's setting +// - **One cap per node, shared.** The node's share of its processors is one value, the node-engine's own config // `build-cpu-share` (ShareSetting), set per node through the node's settings layer. The node-engine writes it as the // slice's CPUQuota: one bound the kernel holds for all the node's build modules together, never one per module. -// - **Threads.** Inside that bound a step starts no more threads than its part of it (Threads): the node's share -// divided by every build slot of the node, across all its build modules (the composer hands both as words, -// ShareWord and SlotsWord). GOMAXPROCS sets every go command's -p, the compiler's concurrency, a test binary's -// processors and -parallel. +// - **Threads.** The quota bounds the total; threads only size each step. A step gets the share divided by its own +// module's capacity (Threads), from the share the composer hands as a word (ShareWord): GOMAXPROCS sets every go +// command's -p, the compiler's concurrency, a test binary's processors and -parallel. A module given no word sets +// no thread limit at all: without the slice there is nothing to size a step to, and a limit then would only slow +// builds. package buildcpu import ( @@ -24,7 +25,7 @@ import ( "strings" ) -// ShareSetting is the node-tools runtime's setting of the node's processor share for its builds, in percent of the +// ShareSetting is the node-engine's config value of the node's processor share for its builds, in percent of the // machine's processors, 1 to 100: one value per node, set through the node's settings layer (novox/hq ADR 0553 §3). const ShareSetting = "build-cpu-share" @@ -32,16 +33,8 @@ const ShareSetting = "build-cpu-share" // itself whatever builds. const DefaultPercent = 50 -// BuildSeats is how many seats yield (catalogue.Seat.Yields): one build module per language. A step that is told no -// node-wide slot count assumes every one of them runs its full capacity beside it. -const BuildSeats = 3 - -// The words the composer gives every yielding module (novox/hq ADR 0553 §4): the node's share, and every build slot -// of the node across all its yielding modules. -const ( - ShareWord = "MESH_BUILD_CPU_SHARE" - SlotsWord = "MESH_BUILD_SLOTS" -) +// ShareWord is the word the composer gives every yielding module (novox/hq ADR 0553 §4): the node's share, in percent. +const ShareWord = "MESH_BUILD_CPU_SHARE" // Quota is a CPUQuota for a share of a machine with cores processors: systemd counts a quota in percent of one // processor, so half of 24 is 1200%. At least 100%, one processor, so a share on a small machine never stops a step. @@ -53,8 +46,8 @@ func Quota(percent, cores int) int { return q } -// Threads is how many threads one step may run at once: its part of the share when slots steps may run on the node -// at once, at least one. Rounded down, so the steps together stay inside the quota. +// Threads is how many threads one step may run at once: its part of the share when its own module runs slots steps at +// once, at least one. The slice's quota bounds the steps of all the node's build modules together; this only sizes one. func Threads(percent, cores, slots int) int { if slots < 1 { slots = 1 @@ -71,44 +64,38 @@ func Threads(percent, cores, slots int) int { // Limits is what one step is held to on this node, as a status tool says it. type Limits struct { - // Percent is the node's share in percent of its processors; From says where it was read: the composer's word, - // or the default. - Percent int `json:"share-percent"` + // Percent is the node's share in percent of its processors, and From where it was read: ShareWord, or "none" when + // the composer gave no share and a step runs without a thread limit. + Percent int `json:"share-percent,omitempty"` From string `json:"share-from"` // Cores is the machine's processor count. Cores int `json:"cores"` - // Slots is every build slot of the node, across all its build modules; SlotsFrom where it was read. - Slots int `json:"node-slots"` - SlotsFrom string `json:"slots-from"` - // Threads is what each step is given (Threads). + // Slots is this module's own capacity on the node: steps at once. + Slots int `json:"slots"` + // Threads is what each step is given (Threads); 0 is no limit. Threads int `json:"threads-per-step"` } -// FromWords is the limits of a step from the words the composer gave this process (ShareWord, SlotsWord). Where a word -// is absent or not a number: the default share, and own × BuildSeats slots. A module told nothing assumes every build -// module of the node runs its full capacity beside it, so the steps together never take more than the default share. +// FromWords is the limits of a step from the share the composer gave this process (ShareWord) and the module's own +// capacity. Without a usable share there is no limit: Threads is 0. func FromWords(getenv func(string) string, own int) Limits { if own < 1 { own = 1 } - l := Limits{Percent: DefaultPercent, From: "default", Cores: runtime.NumCPU(), Slots: own * BuildSeats, - SlotsFrom: "own capacity × " + strconv.Itoa(BuildSeats) + " build seats"} + l := Limits{From: "none", Cores: runtime.NumCPU(), Slots: own} if n, err := strconv.Atoi(strings.TrimSpace(getenv(ShareWord))); err == nil && n >= 1 && n <= 100 { l.Percent, l.From = n, ShareWord + l.Threads = Threads(n, l.Cores, own) } - if n, err := strconv.Atoi(strings.TrimSpace(getenv(SlotsWord))); err == nil && n >= 1 { - l.Slots, l.SlotsFrom = n, SlotsWord - } - l.Threads = Threads(l.Percent, l.Cores, l.Slots) return l } // Env is the environment a step's commands are given so they start no more threads than they may: GOMAXPROCS for every // Go program (the compiler, a test binary, esbuild, buildah), UV_THREADPOOL_SIZE for Node's worker pool. Returned as -// KEY=value words to append to a command's environment. +// KEY=value words to append to a command's environment; nothing for 0 threads, no limit. func Env(threads int) []string { if threads < 1 { - threads = 1 + return nil } t := strconv.Itoa(threads) return []string{"GOMAXPROCS=" + t, "UV_THREADPOOL_SIZE=" + t} diff --git a/go/buildcpu/buildcpu_test.go b/go/buildcpu/buildcpu_test.go index cdf7b91..6740687 100644 --- a/go/buildcpu/buildcpu_test.go +++ b/go/buildcpu/buildcpu_test.go @@ -8,18 +8,17 @@ import ( "testing" ) -// The node's share as a quota, the threads one step is given across every build slot of the node, and the words the -// composer hands, in one table (ADR 0553, issue 1806). +// The node's share as a quota, the threads one step is given for its own module's capacity, and the word the composer +// hands, in one table (ADR 0553, issue 1806). func TestShareQuotaAndThreads(t *testing.T) { for _, c := range []struct { name string percent, cores, slots int quota, threads int }{ - {"shanks: half of 24 across 9 slots", 50, 24, 9, 1200, 1}, - {"shanks: half of 24 across 4 slots", 50, 24, 4, 1200, 3}, - {"ace: three quarters of 24 across 6 slots", 75, 24, 6, 1800, 3}, - {"g14: half of 16 across 1 slot", 50, 16, 1, 800, 8}, + {"shanks: half of 24, three slots", 50, 24, 3, 1200, 4}, + {"ace: three quarters of 24, two slots", 75, 24, 2, 1800, 9}, + {"g14: half of 16, one slot", 50, 16, 1, 800, 8}, {"one processor at least", 10, 2, 4, 100, 1}, {"no slots counts as one", 100, 4, 0, 400, 4}, {"a share out of range is the default", 0, 8, 1, 0, 4}, @@ -33,17 +32,18 @@ func TestShareQuotaAndThreads(t *testing.T) { t.Errorf("%s: Threads = %d, want %d", c.name, th, c.threads) } } - words := map[string]string{ShareWord: "75", SlotsWord: "6"} - l := FromWords(func(k string) string { return words[k] }, 2) - if l.Percent != 75 || l.From != ShareWord || l.Slots != 6 || l.SlotsFrom != SlotsWord || l.Threads != Threads(75, runtime.NumCPU(), 6) { - t.Errorf("from the words: %+v", l) + l := FromWords(func(k string) string { return map[string]string{ShareWord: "75"}[k] }, 2) + if l.Percent != 75 || l.From != ShareWord || l.Slots != 2 || l.Threads != Threads(75, runtime.NumCPU(), 2) { + t.Errorf("from the word: %+v", l) } - // Told nothing (or nonsense), a module assumes every build seat beside it at its own capacity, at the default share. - words = map[string]string{ShareWord: "250", SlotsWord: "many"} - if l := FromWords(func(k string) string { return words[k] }, 3); l.Percent != DefaultPercent || l.Slots != 3*BuildSeats || l.From != "default" { - t.Errorf("without usable words: %+v", l) + // Told nothing usable, a step has no thread limit: nothing in its environment. + for _, word := range []string{"", "250", "lots"} { + l := FromWords(func(string) string { return word }, 3) + if l.Threads != 0 || l.From != "none" || Env(l.Threads) != nil { + t.Errorf("share %q: %+v, env %v", word, l, Env(l.Threads)) + } } - if got := Env(0); !reflect.DeepEqual(got, []string{"GOMAXPROCS=1", "UV_THREADPOOL_SIZE=1"}) { + if got := Env(3); !reflect.DeepEqual(got, []string{"GOMAXPROCS=3", "UV_THREADPOOL_SIZE=3"}) { t.Errorf("Env = %q", got) } } diff --git a/go/buildstep/capacity_test.go b/go/buildstep/capacity_test.go index 632cdfc..a9d1a60 100644 --- a/go/buildstep/capacity_test.go +++ b/go/buildstep/capacity_test.go @@ -37,17 +37,14 @@ func TestNodeCapacity(t *testing.T) { } } -// ADR 0553 §4: a step's limits come from the composer's words; without them, every build seat at the module's own -// capacity at the default share. +// ADR 0553 §4: a step's threads are the composer's share over the module's own capacity; without the word, none. func TestNodeLimits(t *testing.T) { t.Setenv("MESH_BUILD_CPU_SHARE", "") - t.Setenv("MESH_BUILD_SLOTS", "") - if l := NodeLimits(3); l.Percent != 50 || l.Slots != 9 { - t.Errorf("without words: %+v", l) + if l := NodeLimits(3); l.Threads != 0 || l.Slots != 3 { + t.Errorf("without the word: %+v", l) } t.Setenv("MESH_BUILD_CPU_SHARE", "75") - t.Setenv("MESH_BUILD_SLOTS", "5") - if l := NodeLimits(3); l.Percent != 75 || l.Slots != 5 || l.Threads < 1 { - t.Errorf("from the words: %+v", l) + if l := NodeLimits(3); l.Percent != 75 || l.Slots != 3 || l.Threads < 1 { + t.Errorf("from the word: %+v", l) } } diff --git a/go/buildstep/cpu.go b/go/buildstep/cpu.go index 6147202..39b5e3e 100644 --- a/go/buildstep/cpu.go +++ b/go/buildstep/cpu.go @@ -6,10 +6,9 @@ import ( "git.novox.be/novox/mesh-sdk/go/buildcpu" ) -// NodeLimits is what one step of a build module is held to (novox/hq ADR 0553 §4): the node's processor share and every -// build slot of the node, from the words the composer gives the module (buildcpu.ShareWord, buildcpu.SlotsWord), and the -// machine's processors. A module told neither assumes every build seat beside it at its own capacity, own, so its steps -// stay within the default share with the others'. +// NodeLimits is what one step of a build module is held to (novox/hq ADR 0553 §4): the node's processor share from the +// word the composer gives the module (buildcpu.ShareWord), divided by the module's own capacity, own, over the machine's +// processors. Without the word, no thread limit: Threads is 0. func NodeLimits(own int) buildcpu.Limits { return buildcpu.FromWords(os.Getenv, own) } diff --git a/go/catalogue/seats_test.go b/go/catalogue/seats_test.go index 4e65fca..62043e1 100644 --- a/go/catalogue/seats_test.go +++ b/go/catalogue/seats_test.go @@ -4,8 +4,6 @@ import ( "regexp" "strings" "testing" - - "git.novox.be/novox/mesh-sdk/go/buildcpu" ) // Defends novox/hq ADR 0110: a seat is a module assignment from a closed set. @@ -337,14 +335,14 @@ func TestNodeBuildAgentIsRetiredForTheBuildSeats(t *testing.T) { t.Errorf("%s serves %v", name, verbs) } } - // The build seats are the seats that yield, as many as a step told nothing assumes beside it (ADR 0553 §4). - yielding := 0 + // The build seats are the seats that yield, and no other (ADR 0553 §1). + var yielding []string for _, s := range Seats() { if s.Yields { - yielding++ + yielding = append(yielding, s.Name) } } - if yielding != buildcpu.BuildSeats { - t.Errorf("%d seats yield, and buildcpu.BuildSeats says %d", yielding, buildcpu.BuildSeats) + if strings.Join(yielding, ",") != "node-build-go,node-build-node,node-build-image" { + t.Errorf("the seats that yield are %v", yielding) } } From 36061b5c56ae7c90a676c22d774ed910a1d0ad18 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 26 Oct 2026 15:01:57 +0100 Subject: [PATCH 5/6] Start commands in a directory that exists, and decline an ask whose toolchain could not be read (issue 734) A build module runs in the directory its build was unpacked in. When the node-engine replaced that build, every command the module started inherited a deleted working directory, and the toolchain probe's failure became a final refusal of the check that asked it (three checks on ace on 2026-10-26). forgekey: ExecRunnerClean starts its command in the HOME it is given, or the temporary directory; the other runners keep the process's directory while it exists and use the temporary directory once it is gone. buildstep: a guard whose probe failed answers a Refusal with Again set; the puller declines such an ask (delivery.StepDeclined) so its asker puts it again for another node or a later pull, instead of ending it refused. A version mismatch stays a refusal. --- go/buildstep/guard.go | 6 ++++-- go/buildstep/guard_test.go | 9 +++++---- go/buildstep/pull.go | 35 ++++++++++++++++++++++++++--------- go/buildstep/pull_test.go | 18 +++++++++++++++++- go/buildstep/refusal.go | 5 +++++ go/forgekey/runner.go | 27 ++++++++++++++++++++++++++- go/forgekey/runner_test.go | 18 +++++++++++++++++- 7 files changed, 100 insertions(+), 18 deletions(-) diff --git a/go/buildstep/guard.go b/go/buildstep/guard.go index a9dc49c..6d2678b 100644 --- a/go/buildstep/guard.go +++ b/go/buildstep/guard.go @@ -27,11 +27,13 @@ type Guard struct { // Check probes the installed version, writes it to . of the toolchains state every time it runs, // and answers nil when it is the recipe's. Otherwise it answers a *Refusal naming both versions; the installed // version is written even then, so the indexer sees that builders disagree. A probe that fails is a *Refusal -// naming its failure, and nothing is written. A write that fails is an error that is not a Refusal. +// naming its failure with Again set, and nothing is written: a toolchain that could not be read is not a toolchain +// known to be wrong, so the ask goes to another node or a later pull instead of ending refused (novox/hq issue 734). +// A write that fails is an error that is not a Refusal. func (g Guard) Check(ctx context.Context) error { installed, err := g.Probe(ctx) if err != nil { - return &Refusal{Text: fmt.Sprintf("the installed %s version could not be read: %v", g.Language, err), Want: g.Want} + return &Refusal{Text: fmt.Sprintf("the installed %s version could not be read: %v", g.Language, err), Want: g.Want, Again: true} } key, err := delivery.ToolchainKey(g.Node, g.Language) if err != nil { diff --git a/go/buildstep/guard_test.go b/go/buildstep/guard_test.go index 30514b1..a22d11b 100644 --- a/go/buildstep/guard_test.go +++ b/go/buildstep/guard_test.go @@ -46,7 +46,7 @@ func TestCheckRefusesAnotherVersionAndWritesTheInstalledOne(t *testing.T) { if !errors.As(err, &ref) { t.Fatalf("Check = %v, want a Refusal", err) } - if ref.Text != "go 1.27.1 installed, the recipe builds with go 1.27.0" || ref.Want != "1.27.0" || ref.Have != "1.27.1" { + if ref.Text != "go 1.27.1 installed, the recipe builds with go 1.27.0" || ref.Want != "1.27.0" || ref.Have != "1.27.1" || ref.Again { t.Errorf("refusal = %+v", ref) } if k["ace.go"].Version != "1.27.1" { @@ -54,13 +54,14 @@ func TestCheckRefusesAnotherVersionAndWritesTheInstalledOne(t *testing.T) { } } -// 1473/AC3: a probe that fails: a Refusal naming the failure, and nothing put. +// 1473/AC3: a probe that fails: a Refusal naming the failure, and nothing put. Issue 734: one to ask again (Again), not a +// toolchain known to be wrong. func TestCheckRefusesAFailedProbeAndPutsNothing(t *testing.T) { k := toolchainKeys{} err := guardFor(k, "", errors.New("go: not found")).Check(context.Background()) var ref *Refusal - if !errors.As(err, &ref) || !strings.Contains(ref.Text, "go: not found") { - t.Fatalf("Check = %v, want a Refusal naming the probe's failure", err) + if !errors.As(err, &ref) || !strings.Contains(ref.Text, "go: not found") || !ref.Again { + t.Fatalf("Check = %+v, want a Refusal to ask again naming the probe's failure", err) } if len(k) != 0 { t.Errorf("put %v", k) diff --git a/go/buildstep/pull.go b/go/buildstep/pull.go index 960d02e..587d3cf 100644 --- a/go/buildstep/pull.go +++ b/go/buildstep/pull.go @@ -320,22 +320,33 @@ func (p *Puller) next(ctx context.Context, wait time.Duration) (*Pulled, error) // back, as one whose start cannot be. func (p *Puller) decline(msg *Pulled, ask delivery.StepAsk, lacks []string) { step, key, err := begin(p.Put, p.Node, ask.Kind+" ask", ask.ID, AskWhat(ask)) + if err != nil { + p.logf("the ask %s cannot be declined on %s, so it is given back: %v", ask.ID, p.Node, err) + _ = msg.Nak() + return + } + why := fmt.Sprintf("the ask %s needs %s, and %s lacks %s: it is left to a node that has it", ask.ID, + strings.Join(ask.Needs, ", "), p.Node, strings.Join(lacks, ", ")) + p.endDeclined(msg, ask, key, step, why, lacks) +} + +// endDeclined ends a begun step of an ask refused as declined (delivery.StepDeclined) and drops its message, for its +// asker to put again; the worker it came from is not looked at again until the next pull's wait. A message whose +// decline cannot be written is handed back. +func (p *Puller) endDeclined(msg *Pulled, ask delivery.StepAsk, key string, step delivery.Step, why string, lacks []string) { + step.Ended = now().UTC() + step.State, step.Reason = delivery.StepRefused, why + var err error + step.Result, err = json.Marshal(delivery.StepDeclined{Declined: true, Needs: ask.Needs, Lacks: lacks}) if err == nil { - why := Refuse("the ask %s needs %s, and %s lacks %s: it is left to a node that has it", ask.ID, - strings.Join(ask.Needs, ", "), p.Node, strings.Join(lacks, ", ")) - step.Ended = now().UTC() - step.State, step.Reason = delivery.StepRefused, why.Error() - step.Result, err = json.Marshal(delivery.StepDeclined{Declined: true, Needs: ask.Needs, Lacks: lacks}) - if err == nil { - err = writeEnd(p.Put, key, step) - } + err = writeEnd(p.Put, key, step) } if err != nil { p.logf("the ask %s cannot be declined on %s, so it is given back: %v", ask.ID, p.Node, err) _ = msg.Nak() return } - p.logf("the ask %s is declined on %s: it needs %s, and this node lacks %s", ask.ID, p.Node, strings.Join(ask.Needs, ", "), strings.Join(lacks, ", ")) + p.logf("the ask %s is declined on %s: %s", ask.ID, p.Node, why) _ = msg.Term() p.mu.Lock() if p.declined == nil { @@ -431,6 +442,12 @@ func (p *Puller) run(ctx context.Context, msg *Pulled, ask delivery.StepAsk, key // The process stops: the ask is neither ended nor acknowledged here, so the stream gives it to a node that runs. return } + if ref := (*Refusal)(nil); errors.As(err, &ref) && ref.Again { + // The node cannot run it at this moment (its toolchain could not be read): declined, not refused, so the + // asker puts it again for another node or a later pull (issue 734). + p.endDeclined(msg, ask, key, step, fmt.Sprintf("%s cannot run the ask %s now: %v", p.Node, ask.ID, err), nil) + return + } if ferr := finish(p.Put, key, step, result, err, p.Logf); ferr != nil { // No end was written: the ask is not acknowledged as done, so the stream gives it again and a run that can // write its end (or the poison rule, issue 1907) ends it where its asker sees it (issue 1930). diff --git a/go/buildstep/pull_test.go b/go/buildstep/pull_test.go index 902a460..72e2cb4 100644 --- a/go/buildstep/pull_test.go +++ b/go/buildstep/pull_test.go @@ -139,7 +139,8 @@ func TestPullHoldsAtMostItsCapacity(t *testing.T) { // An ask that is not one, or not this seat's, is dropped and runs nowhere. And issue 890/AC1: a check ask needing // node-build-node is declined on a node that lacks it (its step refused as declined, naming what it lacks, the message -// dropped for the asker to put again), and runs on a node that has it. +// dropped for the asker to put again), and runs on a node that has it. And issue 734: an ask whose run is refused to +// ask again (a toolchain that could not be read) is declined the same way, not refused for good. func TestPullDropsWhatIsNoAsk(t *testing.T) { src, st := NewFakeAsks(), newSteps() src.Publish(delivery.AskDelivery, []byte(`{"v":1}`)) @@ -173,6 +174,21 @@ func TestPullDropsWhatIsNoAsk(t *testing.T) { Run: func(ctx context.Context, a delivery.StepAsk) (any, error) { return nil, nil }} go q.Pull(ctx) waitFor(t, "the placed ask acked where its need is met", func() bool { a, _, _ := has.Fates(); return len(a) == 1 }) + + probe, probeSteps := NewFakeAsks(), newSteps() + unready := testAsk(delivery.AskCheck) + _ = probe.PublishAsk(unready) + r := &Puller{Source: probe, Language: delivery.AskGo, Node: "ace", Put: probeSteps.put, Capacity: func() int { return 1 }, + PullWait: 10 * time.Millisecond, Run: func(ctx context.Context, a delivery.StepAsk) (any, error) { + return nil, &Refusal{Text: "the installed go version could not be read: getwd: no such file or directory", Again: true} + }} + go r.Pull(ctx) + waitFor(t, "the unready ask termed", func() bool { _, tm, _ := probe.Fates(); return len(tm) == 1 }) + key, _ = delivery.AskStepKey("ace", unready.ID) + s, _ = probeSteps.get(key) + if _, ok := delivery.DeclinedOf(s); !ok || !strings.Contains(s.Reason, "getwd") { + t.Fatalf("the step of an ask its node could not run now = %+v, want it declined naming why", s) + } } // load reads the recorded steps state's raw values: the pause key is not a step, so it is kept apart. diff --git a/go/buildstep/refusal.go b/go/buildstep/refusal.go index dcf0c48..c5256a7 100644 --- a/go/buildstep/refusal.go +++ b/go/buildstep/refusal.go @@ -10,6 +10,11 @@ type Refusal struct { Text string // Want is the version the recipe asks for and Have the one found; either may be empty. Want, Have string + // Again says the node cannot run the step at this moment, not that it never should: its toolchain could not even + // be read (novox/hq issue 734: a probe started in a directory deleted under it). A puller declines such an ask + // rather than refusing it (delivery.StepDeclined), so its asker puts it again for another node or a later pull; a + // step run as a tool ends refused, as any Refusal. + Again bool } func (r *Refusal) Error() string { return r.Text } diff --git a/go/forgekey/runner.go b/go/forgekey/runner.go index 14bba8a..422e868 100644 --- a/go/forgekey/runner.go +++ b/go/forgekey/runner.go @@ -43,7 +43,8 @@ func ExecRunnerFor(bound time.Duration) Runner { // ExecRunnerClean is ExecRunnerFor whose commands get exactly the env argument as their environment, and nothing of // this process's (issue 1497; novox/hq ADR 0466 §4: no registry password in a fetch's environment). The caller -// gives PATH and HOME itself. A directory is the command's own business (`git -C`, `go -C`). +// gives PATH and HOME itself. A directory is the command's own business (`git -C`, `go -C`): it is started in the HOME +// it is given, or the machine's temporary directory, never in this process's (stableDir). func ExecRunnerClean(bound time.Duration) Runner { return func(ctx context.Context, env []string, name string, args ...string) Ran { return execRun(ctx, bound, true, env, name, args...) @@ -62,6 +63,7 @@ func execRun(ctx context.Context, bound time.Duration, clean bool, env []string, } else { cmd.Env = append(os.Environ(), env...) } + cmd.Dir = stableDir(clean, env) var out, errb bytes.Buffer cmd.Stdout, cmd.Stderr = &out, &errb err := cmd.Run() @@ -83,6 +85,29 @@ func execRun(ctx context.Context, bound time.Duration, clean bool, env []string, return r } +// stableDir is the directory a command starts in when it names none (novox/hq issue 734). **Never one that may be +// deleted under it**: a module runs in the directory its build was unpacked in, which a new build replaces, and a +// command started there inherits it — `go version` then fails "getwd: no such file or directory" before it reads +// anything. A clean command starts in the HOME it is given, or the temporary directory; any other keeps this +// process's directory while it exists, and starts in the temporary directory once it is gone. "" is this process's. +func stableDir(clean bool, env []string) string { + if !clean { + if _, err := os.Getwd(); err == nil { + return "" + } + return os.TempDir() + } + for i := len(env) - 1; i >= 0; i-- { + if home, ok := strings.CutPrefix(env[i], "HOME="); ok { + if info, err := os.Stat(home); err == nil && info.IsDir() { + return home + } + break + } + } + return os.TempDir() +} + func firstLine(s string) string { for _, l := range strings.Split(s, "\n") { if l = strings.TrimSpace(l); l != "" { diff --git a/go/forgekey/runner_test.go b/go/forgekey/runner_test.go index 13f3669..2dc09b4 100644 --- a/go/forgekey/runner_test.go +++ b/go/forgekey/runner_test.go @@ -48,13 +48,29 @@ func TestExecRunnerForZeroIsCutByItsContext(t *testing.T) { } } -// 1497/AC4: ExecRunnerClean(0) runs env with only the given words: PATH and HOME, not the process's secret. +// 1497/AC4: ExecRunnerClean(0) runs env with only the given words: PATH and HOME, not the process's secret. And issue +// 734: run from a process whose own directory was deleted under it (a module whose build a new one replaced), a clean +// command starts in its HOME, and any other in the temporary directory, rather than failing to find where it is. func TestExecRunnerCleanGivesTheCommandOnlyTheEnvArgument(t *testing.T) { t.Setenv("MESH_TEST_SECRET", "x") r := ExecRunnerClean(0)(context.Background(), []string{"PATH=" + os.Getenv("PATH"), "HOME=/h"}, "env") if r.Status != 0 || !strings.Contains(r.Stdout, "HOME=/h") || !strings.Contains(r.Stdout, "PATH=") || strings.Contains(r.Stdout, "MESH_TEST_SECRET") { t.Errorf("ran = %+v", r) } + + home, gone := t.TempDir(), t.TempDir() + t.Chdir(gone) + if err := os.Remove(gone); err != nil { + t.Fatal(err) + } + r = ExecRunnerClean(0)(context.Background(), []string{"PATH=" + os.Getenv("PATH"), "HOME=" + home}, "pwd") + if r.Status != 0 || strings.TrimSpace(r.Stdout) != home { + t.Errorf("a clean command run from a deleted directory = %+v, want it started in its HOME %s", r, home) + } + r = ExecRunnerFor(0)(context.Background(), nil, "pwd") + if r.Status != 0 || strings.TrimSpace(r.Stdout) != os.TempDir() { + t.Errorf("a command run from a deleted directory = %+v, want it started in %s", r, os.TempDir()) + } } // 1497/AC5: ExecRunnerFor(0) is unchanged: the env argument is added to the process's environment. From fd7d93333e6b2540936536393009a9865e81e4a1 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 26 Oct 2026 15:11:04 +0100 Subject: [PATCH 6/6] Keep a failed toolchain probe a loud refusal; only the command directory is fixed (issue 734, review round 1) Declining an ask whose probe failed would turn a truly broken toolchain into a wait of up to the decline bound that then fails with an empty "needs a node with" reason. With the node-engine no longer deleting a running build and commands started in a directory that exists, the getwd cause is gone, so the guard's refusal stays as it was. The decline refactor goes with it. --- go/buildstep/guard.go | 6 ++---- go/buildstep/guard_test.go | 9 ++++----- go/buildstep/pull.go | 35 +++++++++-------------------------- go/buildstep/pull_test.go | 18 +----------------- go/buildstep/refusal.go | 5 ----- 5 files changed, 16 insertions(+), 57 deletions(-) diff --git a/go/buildstep/guard.go b/go/buildstep/guard.go index 6d2678b..a9dc49c 100644 --- a/go/buildstep/guard.go +++ b/go/buildstep/guard.go @@ -27,13 +27,11 @@ type Guard struct { // Check probes the installed version, writes it to . of the toolchains state every time it runs, // and answers nil when it is the recipe's. Otherwise it answers a *Refusal naming both versions; the installed // version is written even then, so the indexer sees that builders disagree. A probe that fails is a *Refusal -// naming its failure with Again set, and nothing is written: a toolchain that could not be read is not a toolchain -// known to be wrong, so the ask goes to another node or a later pull instead of ending refused (novox/hq issue 734). -// A write that fails is an error that is not a Refusal. +// naming its failure, and nothing is written. A write that fails is an error that is not a Refusal. func (g Guard) Check(ctx context.Context) error { installed, err := g.Probe(ctx) if err != nil { - return &Refusal{Text: fmt.Sprintf("the installed %s version could not be read: %v", g.Language, err), Want: g.Want, Again: true} + return &Refusal{Text: fmt.Sprintf("the installed %s version could not be read: %v", g.Language, err), Want: g.Want} } key, err := delivery.ToolchainKey(g.Node, g.Language) if err != nil { diff --git a/go/buildstep/guard_test.go b/go/buildstep/guard_test.go index a22d11b..30514b1 100644 --- a/go/buildstep/guard_test.go +++ b/go/buildstep/guard_test.go @@ -46,7 +46,7 @@ func TestCheckRefusesAnotherVersionAndWritesTheInstalledOne(t *testing.T) { if !errors.As(err, &ref) { t.Fatalf("Check = %v, want a Refusal", err) } - if ref.Text != "go 1.27.1 installed, the recipe builds with go 1.27.0" || ref.Want != "1.27.0" || ref.Have != "1.27.1" || ref.Again { + if ref.Text != "go 1.27.1 installed, the recipe builds with go 1.27.0" || ref.Want != "1.27.0" || ref.Have != "1.27.1" { t.Errorf("refusal = %+v", ref) } if k["ace.go"].Version != "1.27.1" { @@ -54,14 +54,13 @@ func TestCheckRefusesAnotherVersionAndWritesTheInstalledOne(t *testing.T) { } } -// 1473/AC3: a probe that fails: a Refusal naming the failure, and nothing put. Issue 734: one to ask again (Again), not a -// toolchain known to be wrong. +// 1473/AC3: a probe that fails: a Refusal naming the failure, and nothing put. func TestCheckRefusesAFailedProbeAndPutsNothing(t *testing.T) { k := toolchainKeys{} err := guardFor(k, "", errors.New("go: not found")).Check(context.Background()) var ref *Refusal - if !errors.As(err, &ref) || !strings.Contains(ref.Text, "go: not found") || !ref.Again { - t.Fatalf("Check = %+v, want a Refusal to ask again naming the probe's failure", err) + if !errors.As(err, &ref) || !strings.Contains(ref.Text, "go: not found") { + t.Fatalf("Check = %v, want a Refusal naming the probe's failure", err) } if len(k) != 0 { t.Errorf("put %v", k) diff --git a/go/buildstep/pull.go b/go/buildstep/pull.go index 587d3cf..960d02e 100644 --- a/go/buildstep/pull.go +++ b/go/buildstep/pull.go @@ -320,33 +320,22 @@ func (p *Puller) next(ctx context.Context, wait time.Duration) (*Pulled, error) // back, as one whose start cannot be. func (p *Puller) decline(msg *Pulled, ask delivery.StepAsk, lacks []string) { step, key, err := begin(p.Put, p.Node, ask.Kind+" ask", ask.ID, AskWhat(ask)) - if err != nil { - p.logf("the ask %s cannot be declined on %s, so it is given back: %v", ask.ID, p.Node, err) - _ = msg.Nak() - return - } - why := fmt.Sprintf("the ask %s needs %s, and %s lacks %s: it is left to a node that has it", ask.ID, - strings.Join(ask.Needs, ", "), p.Node, strings.Join(lacks, ", ")) - p.endDeclined(msg, ask, key, step, why, lacks) -} - -// endDeclined ends a begun step of an ask refused as declined (delivery.StepDeclined) and drops its message, for its -// asker to put again; the worker it came from is not looked at again until the next pull's wait. A message whose -// decline cannot be written is handed back. -func (p *Puller) endDeclined(msg *Pulled, ask delivery.StepAsk, key string, step delivery.Step, why string, lacks []string) { - step.Ended = now().UTC() - step.State, step.Reason = delivery.StepRefused, why - var err error - step.Result, err = json.Marshal(delivery.StepDeclined{Declined: true, Needs: ask.Needs, Lacks: lacks}) if err == nil { - err = writeEnd(p.Put, key, step) + why := Refuse("the ask %s needs %s, and %s lacks %s: it is left to a node that has it", ask.ID, + strings.Join(ask.Needs, ", "), p.Node, strings.Join(lacks, ", ")) + step.Ended = now().UTC() + step.State, step.Reason = delivery.StepRefused, why.Error() + step.Result, err = json.Marshal(delivery.StepDeclined{Declined: true, Needs: ask.Needs, Lacks: lacks}) + if err == nil { + err = writeEnd(p.Put, key, step) + } } if err != nil { p.logf("the ask %s cannot be declined on %s, so it is given back: %v", ask.ID, p.Node, err) _ = msg.Nak() return } - p.logf("the ask %s is declined on %s: %s", ask.ID, p.Node, why) + p.logf("the ask %s is declined on %s: it needs %s, and this node lacks %s", ask.ID, p.Node, strings.Join(ask.Needs, ", "), strings.Join(lacks, ", ")) _ = msg.Term() p.mu.Lock() if p.declined == nil { @@ -442,12 +431,6 @@ func (p *Puller) run(ctx context.Context, msg *Pulled, ask delivery.StepAsk, key // The process stops: the ask is neither ended nor acknowledged here, so the stream gives it to a node that runs. return } - if ref := (*Refusal)(nil); errors.As(err, &ref) && ref.Again { - // The node cannot run it at this moment (its toolchain could not be read): declined, not refused, so the - // asker puts it again for another node or a later pull (issue 734). - p.endDeclined(msg, ask, key, step, fmt.Sprintf("%s cannot run the ask %s now: %v", p.Node, ask.ID, err), nil) - return - } if ferr := finish(p.Put, key, step, result, err, p.Logf); ferr != nil { // No end was written: the ask is not acknowledged as done, so the stream gives it again and a run that can // write its end (or the poison rule, issue 1907) ends it where its asker sees it (issue 1930). diff --git a/go/buildstep/pull_test.go b/go/buildstep/pull_test.go index 72e2cb4..902a460 100644 --- a/go/buildstep/pull_test.go +++ b/go/buildstep/pull_test.go @@ -139,8 +139,7 @@ func TestPullHoldsAtMostItsCapacity(t *testing.T) { // An ask that is not one, or not this seat's, is dropped and runs nowhere. And issue 890/AC1: a check ask needing // node-build-node is declined on a node that lacks it (its step refused as declined, naming what it lacks, the message -// dropped for the asker to put again), and runs on a node that has it. And issue 734: an ask whose run is refused to -// ask again (a toolchain that could not be read) is declined the same way, not refused for good. +// dropped for the asker to put again), and runs on a node that has it. func TestPullDropsWhatIsNoAsk(t *testing.T) { src, st := NewFakeAsks(), newSteps() src.Publish(delivery.AskDelivery, []byte(`{"v":1}`)) @@ -174,21 +173,6 @@ func TestPullDropsWhatIsNoAsk(t *testing.T) { Run: func(ctx context.Context, a delivery.StepAsk) (any, error) { return nil, nil }} go q.Pull(ctx) waitFor(t, "the placed ask acked where its need is met", func() bool { a, _, _ := has.Fates(); return len(a) == 1 }) - - probe, probeSteps := NewFakeAsks(), newSteps() - unready := testAsk(delivery.AskCheck) - _ = probe.PublishAsk(unready) - r := &Puller{Source: probe, Language: delivery.AskGo, Node: "ace", Put: probeSteps.put, Capacity: func() int { return 1 }, - PullWait: 10 * time.Millisecond, Run: func(ctx context.Context, a delivery.StepAsk) (any, error) { - return nil, &Refusal{Text: "the installed go version could not be read: getwd: no such file or directory", Again: true} - }} - go r.Pull(ctx) - waitFor(t, "the unready ask termed", func() bool { _, tm, _ := probe.Fates(); return len(tm) == 1 }) - key, _ = delivery.AskStepKey("ace", unready.ID) - s, _ = probeSteps.get(key) - if _, ok := delivery.DeclinedOf(s); !ok || !strings.Contains(s.Reason, "getwd") { - t.Fatalf("the step of an ask its node could not run now = %+v, want it declined naming why", s) - } } // load reads the recorded steps state's raw values: the pause key is not a step, so it is kept apart. diff --git a/go/buildstep/refusal.go b/go/buildstep/refusal.go index c5256a7..dcf0c48 100644 --- a/go/buildstep/refusal.go +++ b/go/buildstep/refusal.go @@ -10,11 +10,6 @@ type Refusal struct { Text string // Want is the version the recipe asks for and Have the one found; either may be empty. Want, Have string - // Again says the node cannot run the step at this moment, not that it never should: its toolchain could not even - // be read (novox/hq issue 734: a probe started in a directory deleted under it). A puller declines such an ask - // rather than refusing it (delivery.StepDeclined), so its asker puts it again for another node or a later pull; a - // step run as a tool ends refused, as any Refusal. - Again bool } func (r *Refusal) Error() string { return r.Text }