refactor(#2754): make the container weights Option, not a 0 sentinel
Encoding "not configured" as weight 0 worked (the writer omitted the line) but the type lied: 0 is not a legal cgroup v2 weight, and every reader had to know the sentinel. Use Option<u32> end to end instead — wire type, priv_client, HiveEnv, drop-in writer — so "unset" is a state of the type rather than a magic value. The nix options become nullOr, keeping their default of 80; null now expresses "leave the setting out of the drop-in entirely" declaratively, which is the useful shape on a host whose IO scheduler ignores io.weight anyway. Backward compat is unchanged: the fields stay #[serde(default)], so a request from an older hive-c0re deserialises to None and reproduces the pre-weights drop-in byte for byte. The test that pins that now passes None instead of 0.
This commit is contained in:
parent
e407fa93df
commit
5d3f2af75e
7 changed files with 59 additions and 51 deletions
|
|
@ -87,12 +87,14 @@ pub struct Coordinator {
|
||||||
pub agent_memory_max: String,
|
pub agent_memory_max: String,
|
||||||
/// Per-agent systemd `CPUWeight=` (cgroup v2 `cpu.weight`, 1..=10000).
|
/// Per-agent systemd `CPUWeight=` (cgroup v2 `cpu.weight`, 1..=10000).
|
||||||
/// Same drop-in. A *relative share* under contention, not a cap — see
|
/// Same drop-in. A *relative share* under contention, not a cap — see
|
||||||
/// the `agentCpuWeight` NixOS option for the full semantics.
|
/// the `agentCpuWeight` NixOS option for the full semantics. `None`
|
||||||
pub agent_cpu_weight: u32,
|
/// (option set to `null`) omits the setting: kernel default, and the
|
||||||
|
/// drop-in is byte-identical to the one written before weights existed.
|
||||||
|
pub agent_cpu_weight: Option<u32>,
|
||||||
/// Per-agent systemd `IOWeight=` (cgroup v2 `io.weight`, 1..=10000).
|
/// Per-agent systemd `IOWeight=` (cgroup v2 `io.weight`, 1..=10000).
|
||||||
/// Same drop-in, same relative-share semantics as
|
/// Same drop-in, same relative-share and `None` semantics as
|
||||||
/// [`Self::agent_cpu_weight`].
|
/// [`Self::agent_cpu_weight`].
|
||||||
pub agent_io_weight: u32,
|
pub agent_io_weight: Option<u32>,
|
||||||
/// Operator-tunable model→price table backing the hive-wide cost
|
/// Operator-tunable model→price table backing the hive-wide cost
|
||||||
/// estimate on the ST4TS tab. Set via `services.hyperhive.modelPrices`
|
/// estimate on the ST4TS tab. Set via `services.hyperhive.modelPrices`
|
||||||
/// and passed to `hive-c0re serve --model-prices <json>`. Models not
|
/// and passed to `hive-c0re serve --model-prices <json>`. Models not
|
||||||
|
|
@ -220,10 +222,11 @@ pub struct HiveEnv {
|
||||||
pub agent_memory_max: String,
|
pub agent_memory_max: String,
|
||||||
/// Per-agent systemd `CPUWeight=` — cgroup v2 `cpu.weight`, 1..=10000.
|
/// Per-agent systemd `CPUWeight=` — cgroup v2 `cpu.weight`, 1..=10000.
|
||||||
/// Relative share under contention, not a cap; see `agentCpuWeight`.
|
/// Relative share under contention, not a cap; see `agentCpuWeight`.
|
||||||
pub agent_cpu_weight: u32,
|
/// `None` = leave the setting out of the drop-in (kernel default).
|
||||||
|
pub agent_cpu_weight: Option<u32>,
|
||||||
/// Per-agent systemd `IOWeight=` — cgroup v2 `io.weight`, 1..=10000.
|
/// Per-agent systemd `IOWeight=` — cgroup v2 `io.weight`, 1..=10000.
|
||||||
/// Same semantics as [`Self::agent_cpu_weight`]; see `agentIoWeight`.
|
/// Same semantics as [`Self::agent_cpu_weight`]; see `agentIoWeight`.
|
||||||
pub agent_io_weight: u32,
|
pub agent_io_weight: Option<u32>,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl Default for HiveEnv {
|
impl Default for HiveEnv {
|
||||||
|
|
@ -244,8 +247,8 @@ impl Default for HiveEnv {
|
||||||
// Slightly below the kernel default of 100, so agent
|
// Slightly below the kernel default of 100, so agent
|
||||||
// containers yield to host services + the infra containers
|
// containers yield to host services + the infra containers
|
||||||
// (which carry no drop-in and stay at 100) under contention.
|
// (which carry no drop-in and stay at 100) under contention.
|
||||||
agent_cpu_weight: 80,
|
agent_cpu_weight: Some(80),
|
||||||
agent_io_weight: 80,
|
agent_io_weight: Some(80),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -45,12 +45,14 @@ pub async fn write_dropins(name: &str, hive: &HiveEnv, paths: &AgentPaths) -> Re
|
||||||
/// The weights are hive-wide (`services.hyperhive.agentCpuWeight` /
|
/// The weights are hive-wide (`services.hyperhive.agentCpuWeight` /
|
||||||
/// `agentIoWeight`) — unlike the caps they have no per-agent override in
|
/// `agentIoWeight`) — unlike the caps they have no per-agent override in
|
||||||
/// `meta/resource-limits.json`, so they come straight off [`HiveEnv`].
|
/// `meta/resource-limits.json`, so they come straight off [`HiveEnv`].
|
||||||
|
/// `None` (the nix option set to `null`) means the weight line is left out
|
||||||
|
/// and the container keeps the kernel default.
|
||||||
async fn set_resource_limits(
|
async fn set_resource_limits(
|
||||||
container: &str,
|
container: &str,
|
||||||
cpu_quota: &str,
|
cpu_quota: &str,
|
||||||
memory_max: &str,
|
memory_max: &str,
|
||||||
cpu_weight: u32,
|
cpu_weight: Option<u32>,
|
||||||
io_weight: u32,
|
io_weight: Option<u32>,
|
||||||
) -> Result<()> {
|
) -> Result<()> {
|
||||||
crate::priv_client::write_resource_limits(
|
crate::priv_client::write_resource_limits(
|
||||||
container, memory_max, cpu_quota, cpu_weight, io_weight,
|
container, memory_max, cpu_quota, cpu_weight, io_weight,
|
||||||
|
|
|
||||||
|
|
@ -197,11 +197,15 @@ async fn main() -> Result<()> {
|
||||||
if let Some(v) = agent_memory_max {
|
if let Some(v) = agent_memory_max {
|
||||||
sc.env.agent_memory_max = v;
|
sc.env.agent_memory_max = v;
|
||||||
}
|
}
|
||||||
|
// Passing the flag sets a weight; omitting it keeps whatever the
|
||||||
|
// config file says (including `null` = don't emit the setting).
|
||||||
|
// There's deliberately no flag spelling for "clear it" — that's
|
||||||
|
// what the nix option's `null` is for.
|
||||||
if let Some(v) = agent_cpu_weight {
|
if let Some(v) = agent_cpu_weight {
|
||||||
sc.env.agent_cpu_weight = v;
|
sc.env.agent_cpu_weight = Some(v);
|
||||||
}
|
}
|
||||||
if let Some(v) = agent_io_weight {
|
if let Some(v) = agent_io_weight {
|
||||||
sc.env.agent_io_weight = v;
|
sc.env.agent_io_weight = Some(v);
|
||||||
}
|
}
|
||||||
if let Some(v) = model_prices {
|
if let Some(v) = model_prices {
|
||||||
sc.model_prices =
|
sc.model_prices =
|
||||||
|
|
|
||||||
|
|
@ -174,8 +174,8 @@ pub async fn write_resource_limits(
|
||||||
container: &str,
|
container: &str,
|
||||||
memory_max: &str,
|
memory_max: &str,
|
||||||
cpu_quota: &str,
|
cpu_quota: &str,
|
||||||
cpu_weight: u32,
|
cpu_weight: Option<u32>,
|
||||||
io_weight: u32,
|
io_weight: Option<u32>,
|
||||||
) -> Result<()> {
|
) -> Result<()> {
|
||||||
ok(call(&PrivRequest::WriteResourceLimits {
|
ok(call(&PrivRequest::WriteResourceLimits {
|
||||||
container: container.to_owned(),
|
container: container.to_owned(),
|
||||||
|
|
|
||||||
|
|
@ -343,15 +343,17 @@ pub enum PrivRequest {
|
||||||
container: String,
|
container: String,
|
||||||
memory_max: String,
|
memory_max: String,
|
||||||
cpu_quota: String,
|
cpu_quota: String,
|
||||||
/// cgroup v2 `cpu.weight`, 1..=10000. `#[serde(default)]` so a
|
/// cgroup v2 `cpu.weight`, 1..=10000. `None` means "not
|
||||||
/// hive-priv built before this field still deserialises new
|
/// configured" — the writer omits the line entirely, leaving the
|
||||||
/// requests; 0 is treated as "omit the line" by the writer.
|
/// kernel default. `#[serde(default)]` so a request from a
|
||||||
|
/// hive-c0re built before this field existed deserialises to
|
||||||
|
/// `None` and reproduces the pre-weights drop-in.
|
||||||
#[serde(default)]
|
#[serde(default)]
|
||||||
cpu_weight: u32,
|
cpu_weight: Option<u32>,
|
||||||
/// cgroup v2 `io.weight`, 1..=10000. Same default/omit rule as
|
/// cgroup v2 `io.weight`, 1..=10000. Same `None` = omit rule as
|
||||||
/// `cpu_weight`.
|
/// `cpu_weight`.
|
||||||
#[serde(default)]
|
#[serde(default)]
|
||||||
io_weight: u32,
|
io_weight: Option<u32>,
|
||||||
},
|
},
|
||||||
|
|
||||||
/// Remove `/run/systemd/system/container@<container>.service.d/` if present.
|
/// Remove `/run/systemd/system/container@<container>.service.d/` if present.
|
||||||
|
|
|
||||||
|
|
@ -537,8 +537,8 @@ fn chmod_socket_dir(agent_name: &str, mode: u32) -> Result<(String, String)> {
|
||||||
/// `CPUQuota=` are hard caps that throttle even on an idle host;
|
/// `CPUQuota=` are hard caps that throttle even on an idle host;
|
||||||
/// `CPUWeight=` / `IOWeight=` are cgroup v2 relative shares that only
|
/// `CPUWeight=` / `IOWeight=` are cgroup v2 relative shares that only
|
||||||
/// decide who yields *under contention*. A zero weight means "not
|
/// decide who yields *under contention*. A zero weight means "not
|
||||||
/// configured" and omits the line, so a hive-c0re built before the weights
|
/// configured" (`None`) and omits the line, so a hive-c0re built before the
|
||||||
/// existed keeps producing the old two-line drop-in.
|
/// weights existed keeps producing the old two-line drop-in.
|
||||||
///
|
///
|
||||||
/// The condition causes systemd to *skip* (not *fail*) the unit when the
|
/// The condition causes systemd to *skip* (not *fail*) the unit when the
|
||||||
/// bind-mount source dir is absent — result is `condition`, which does not
|
/// bind-mount source dir is absent — result is `condition`, which does not
|
||||||
|
|
@ -550,8 +550,8 @@ fn write_resource_limits(
|
||||||
container: &str,
|
container: &str,
|
||||||
memory_max: &str,
|
memory_max: &str,
|
||||||
cpu_quota: &str,
|
cpu_quota: &str,
|
||||||
cpu_weight: u32,
|
cpu_weight: Option<u32>,
|
||||||
io_weight: u32,
|
io_weight: Option<u32>,
|
||||||
) -> Result<(String, String)> {
|
) -> Result<(String, String)> {
|
||||||
validate_container_system_name(container)?;
|
validate_container_system_name(container)?;
|
||||||
// Derive the logical agent name (strip h- prefix) to form the runtime
|
// Derive the logical agent name (strip h- prefix) to form the runtime
|
||||||
|
|
@ -572,29 +572,21 @@ fn write_resource_limits(
|
||||||
/// `[Unit]`: the condition is checked at start time — it skips (not fails)
|
/// `[Unit]`: the condition is checked at start time — it skips (not fails)
|
||||||
/// the unit when the MCP socket dir is absent, avoiding restart loops.
|
/// the unit when the MCP socket dir is absent, avoiding restart loops.
|
||||||
/// `[Service]`: the hard caps first, then the relative weights. A weight of
|
/// `[Service]`: the hard caps first, then the relative weights. A weight of
|
||||||
/// `0` means "not configured" and omits its line entirely, so a request from
|
/// `None` means "not configured" and omits its line entirely, so a request
|
||||||
/// a hive-c0re built before the weights existed reproduces the old
|
/// from a hive-c0re built before the weights existed — or one whose nix
|
||||||
/// two-setting drop-in byte for byte.
|
/// option is `null` — reproduces the old two-setting drop-in byte for byte.
|
||||||
fn limits_dropin_body(
|
fn limits_dropin_body(
|
||||||
runtime_dir: &str,
|
runtime_dir: &str,
|
||||||
memory_max: &str,
|
memory_max: &str,
|
||||||
cpu_quota: &str,
|
cpu_quota: &str,
|
||||||
cpu_weight: u32,
|
cpu_weight: Option<u32>,
|
||||||
io_weight: u32,
|
io_weight: Option<u32>,
|
||||||
) -> String {
|
) -> String {
|
||||||
// Built as two possibly-empty lines rather than pushed onto the
|
// Built as two possibly-empty lines rather than pushed onto the
|
||||||
// string: `format!` appended to a `String` trips clippy::pedantic's
|
// string: `format!` appended to a `String` trips clippy::pedantic's
|
||||||
// `format_push_string`, and a `write!` would need an unwrap.
|
// `format_push_string`, and a `write!` would need an unwrap.
|
||||||
let cpu_weight_line = if cpu_weight > 0 {
|
let cpu_weight_line = cpu_weight.map_or_else(String::new, |w| format!("CPUWeight={w}\n"));
|
||||||
format!("CPUWeight={cpu_weight}\n")
|
let io_weight_line = io_weight.map_or_else(String::new, |w| format!("IOWeight={w}\n"));
|
||||||
} else {
|
|
||||||
String::new()
|
|
||||||
};
|
|
||||||
let io_weight_line = if io_weight > 0 {
|
|
||||||
format!("IOWeight={io_weight}\n")
|
|
||||||
} else {
|
|
||||||
String::new()
|
|
||||||
};
|
|
||||||
format!(
|
format!(
|
||||||
"[Unit]\n\
|
"[Unit]\n\
|
||||||
ConditionPathIsDirectory={runtime_dir}\n\
|
ConditionPathIsDirectory={runtime_dir}\n\
|
||||||
|
|
@ -2231,13 +2223,13 @@ mod tests {
|
||||||
use std::path::PathBuf;
|
use std::path::PathBuf;
|
||||||
use std::sync::atomic::{AtomicU32, Ordering};
|
use std::sync::atomic::{AtomicU32, Ordering};
|
||||||
|
|
||||||
/// A zero weight is "not configured": the drop-in must come out
|
/// An unset weight is "not configured": the drop-in must come out
|
||||||
/// byte-identical to the pre-weights two-setting body, so a hive-c0re
|
/// byte-identical to the pre-weights two-setting body, so a hive-c0re
|
||||||
/// older than this field can't change what lands on disk.
|
/// older than this field can't change what lands on disk.
|
||||||
#[test]
|
#[test]
|
||||||
fn zero_weights_reproduce_the_pre_weights_dropin() {
|
fn unset_weights_reproduce_the_pre_weights_dropin() {
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
limits_dropin_body("/run/hyperhive/agents/iris", "4G", "200%", 0, 0),
|
limits_dropin_body("/run/hyperhive/agents/iris", "4G", "200%", None, None),
|
||||||
"[Unit]\n\
|
"[Unit]\n\
|
||||||
ConditionPathIsDirectory=/run/hyperhive/agents/iris\n\
|
ConditionPathIsDirectory=/run/hyperhive/agents/iris\n\
|
||||||
\n\
|
\n\
|
||||||
|
|
@ -2248,23 +2240,23 @@ mod tests {
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Weights are appended to the `[Service]` section, each omitted
|
/// Weights are appended to the `[Service]` section, each omitted
|
||||||
/// independently when zero.
|
/// independently when `None`.
|
||||||
#[test]
|
#[test]
|
||||||
fn weights_are_emitted_only_when_set() {
|
fn weights_are_emitted_only_when_set() {
|
||||||
let both = limits_dropin_body("/rt/x", "4G", "200%", 80, 80);
|
let both = limits_dropin_body("/rt/x", "4G", "200%", Some(80), Some(80));
|
||||||
assert!(
|
assert!(
|
||||||
both.ends_with("CPUQuota=200%\nCPUWeight=80\nIOWeight=80\n"),
|
both.ends_with("CPUQuota=200%\nCPUWeight=80\nIOWeight=80\n"),
|
||||||
"{both}"
|
"{both}"
|
||||||
);
|
);
|
||||||
|
|
||||||
let cpu_only = limits_dropin_body("/rt/x", "4G", "200%", 80, 0);
|
let cpu_only = limits_dropin_body("/rt/x", "4G", "200%", Some(80), None);
|
||||||
assert!(
|
assert!(
|
||||||
cpu_only.ends_with("CPUQuota=200%\nCPUWeight=80\n"),
|
cpu_only.ends_with("CPUQuota=200%\nCPUWeight=80\n"),
|
||||||
"{cpu_only}"
|
"{cpu_only}"
|
||||||
);
|
);
|
||||||
assert!(!cpu_only.contains("IOWeight"), "{cpu_only}");
|
assert!(!cpu_only.contains("IOWeight"), "{cpu_only}");
|
||||||
|
|
||||||
let io_only = limits_dropin_body("/rt/x", "4G", "200%", 0, 80);
|
let io_only = limits_dropin_body("/rt/x", "4G", "200%", None, Some(80));
|
||||||
assert!(
|
assert!(
|
||||||
io_only.ends_with("CPUQuota=200%\nIOWeight=80\n"),
|
io_only.ends_with("CPUQuota=200%\nIOWeight=80\n"),
|
||||||
"{io_only}"
|
"{io_only}"
|
||||||
|
|
|
||||||
|
|
@ -302,9 +302,9 @@
|
||||||
};
|
};
|
||||||
|
|
||||||
agentCpuWeight = lib.mkOption {
|
agentCpuWeight = lib.mkOption {
|
||||||
type = lib.types.ints.between 1 10000;
|
type = lib.types.nullOr (lib.types.ints.between 1 10000);
|
||||||
default = 80;
|
default = 80;
|
||||||
example = 50;
|
example = null;
|
||||||
description = ''
|
description = ''
|
||||||
systemd `CPUWeight=` applied to every agent container via the
|
systemd `CPUWeight=` applied to every agent container via the
|
||||||
same drop-in as `agentCpuQuota`. This is the cgroup v2
|
same drop-in as `agentCpuQuota`. This is the cgroup v2
|
||||||
|
|
@ -321,13 +321,17 @@
|
||||||
`hive-matrix`), which stay at `100`. Note this is a hive-wide
|
`hive-matrix`), which stay at `100`. Note this is a hive-wide
|
||||||
value, so it does not rank agents against *each other*: they all
|
value, so it does not rank agents against *each other*: they all
|
||||||
share one weight.
|
share one weight.
|
||||||
|
|
||||||
|
Set to `null` to leave `CPUWeight=` out of the drop-in entirely —
|
||||||
|
the container then inherits the kernel default and the generated
|
||||||
|
unit file is identical to one from before this option existed.
|
||||||
'';
|
'';
|
||||||
};
|
};
|
||||||
|
|
||||||
agentIoWeight = lib.mkOption {
|
agentIoWeight = lib.mkOption {
|
||||||
type = lib.types.ints.between 1 10000;
|
type = lib.types.nullOr (lib.types.ints.between 1 10000);
|
||||||
default = 80;
|
default = 80;
|
||||||
example = 50;
|
example = null;
|
||||||
description = ''
|
description = ''
|
||||||
systemd `IOWeight=` applied to every agent container via the
|
systemd `IOWeight=` applied to every agent container via the
|
||||||
same drop-in as `agentCpuQuota` — the block-IO counterpart of
|
same drop-in as `agentCpuQuota` — the block-IO counterpart of
|
||||||
|
|
@ -340,7 +344,8 @@
|
||||||
scheduler. On a host running `none`/`mq-deadline`/`kyber` without
|
scheduler. On a host running `none`/`mq-deadline`/`kyber` without
|
||||||
iocost QoS configured, systemd writes the value and the kernel
|
iocost QoS configured, systemd writes the value and the kernel
|
||||||
ignores it — harmless, but it will measure as a no-op. Check with
|
ignores it — harmless, but it will measure as a no-op. Check with
|
||||||
`cat /sys/fs/cgroup/io.cost.qos` on the host.
|
`cat /sys/fs/cgroup/io.cost.qos` on the host, and set this to
|
||||||
|
`null` to omit the setting rather than write one nothing reads.
|
||||||
'';
|
'';
|
||||||
};
|
};
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue