Node health: report /nix/store free disk in the heartbeat, and floor the lower #35

Open

kris wants to merge 2 commits from nixstore-free-disk into main

pull from: nixstore-free-disk

merge into: kris:main

kris:main

kris:kb-queue/main

kris:breaker-last-healthy-node

kris:worktree-repo-task-stats

kris:worktree-ui-usability-placement

kris:auto-attic-tokens

kris:mq-closed-pr

kris:nixos-module-batteries

kris:web-auth

kris:attic-tokens

kris:lu/run-supervisor

kris:lu/acked-delivery

kris:lu/cp-restart

kris:failure-comments

kris:lu/protocol-handshake

kris:lu/forge-followups

kris:lu/forge-outbox

kris:lu/eval-push-fatal

kris:nixdrv-usage

kris:test-threads

kris:setcredential-fallback

kris:faster-test-waits

kris:node-health

kris:agent-enrollment

kris:fix-flaky-pr-closed

kris:eval-drv-verify

kris:faster-tests

kris:lu/verdict-integrity

kris:git-proxy

kris:lu/design-decisions

kris:mergeq-ux

kris:worktree-live-upgrades-design

Conversation 1 Commits 2 Files changed 13 +283 -51

kris commented 2026-10-01 00:17:18 +02:00

Owner

Copy link

What. A node with the nix feature now also reports free space on the filesystem that holds /nix/store, as store_free_disk_mb in its heartbeat. The field is optional in kb-core's Heartbeat (serde(default, skip_serializing_if = "Option::is_none")). The heartbeat is not hashed, so there is no kbN- bump.

When the store is reported:

• Only when it is on a different filesystem from the work dir, compared by device of the nearest existing ancestor. If they share a filesystem, the work dir figure already covers it.
• If the store can't be measured, the field is left out rather than reported as 0, so a failed probe doesn't read as a full store. The work dir figure still falls back to 0 on a failed probe, as before.

How the control plane uses it:

• The min_free_disk_mb floor applies to the lower of the work dir and the store.
• The reason for holding a node names the short one: "low disk on /nix/store (N MB free, floor F MB)" or "low disk on work dir (…)".
• /ui/nodes shows "X MB work dir, Y MB /nix/store" when the store is reported.
• GET /api/nodes passes the field through in heartbeat, and its unschedulable reason names the filesystem.

Docs: SPEC §6.2 (heartbeat listing), §6.3 (pseudocode min(free_disk, store_free_disk) ≥ floor and the "Taking new work" text) and §7.1 (heartbeat line) are updated, along with the min_free_disk_mb doc comment, the forgejo-setup config comment and docs/TODO.md (entry struck through as done).

Why. free_disk_mb measured only the work dir. If /nix/store was on another filesystem and filled up, nothing showed it until builds failed and the circuit breaker opened. This was the gap left open in the node-health TODO after the 2026-09-27 full-disk incident.

Tests.

• kb-core heartbeat_store_free_disk_is_optional: a heartbeat from an old agent (no field) still parses; when the field is None it is left out of the JSON; a heartbeat with it round-trips.
• Agent:
  • the_store_is_reported_only_when_it_is_a_filesystem_of_its_own (tempdir): store on the same filesystem as the work dir gives None; no nix feature gives None; a store on another device (procfs) is reported.
  • a_store_that_cannot_be_measured_is_not_reported_full: an unmeasurable store gives None, while the work dir still reads 0.
  • The existing heartbeat test now also checks that a node without nix reports no store figure.
• Health:
  • free_disk_floor_judges_the_fuller_filesystem: the floor uses the lower figure and the reason names the filesystem.
  • A store lower than the work dir but at the floor holds nothing.
  • free_disk_floor is adapted to the new gate signature.
• Integration node_health::a_node_low_on_store_disk_gets_no_work: a node with plenty of room on its work dir but 100 MB on its store gets no work. /api/nodes names /nix/store, and /ui/nodes shows "100000 MB work dir, 100 MB /nix/store". The node gets work again once the store has room.
• cargo clippy --workspace --all-targets -- -D warnings and cargo test --workspace pass. The nix build .#ci.* gate was not run locally (no nix daemon in the dev VM). No files were added, so the filesets are unchanged.

Deploy notes. Either side can be upgraded first:

• An old agent leaves the field out, and the control plane floors on the work dir alone, as it does today.
• An old control plane ignores the field.

The store check takes effect once both the control plane and an agent whose store is on its own filesystem are upgraded. A node whose /nix/store filesystem is already below min_free_disk_mb (default 4096) stops taking new work at that point. Check df /nix/store on nuxbox and ares before switching.

**What.** A node with the `nix` feature now also reports free space on the filesystem that holds /nix/store, as `store_free_disk_mb` in its heartbeat. The field is optional in kb-core's `Heartbeat` (`serde(default, skip_serializing_if = "Option::is_none")`). The heartbeat is not hashed, so there is no `kbN-` bump. When the store is reported: - Only when it is on a different filesystem from the work dir, compared by device of the nearest existing ancestor. If they share a filesystem, the work dir figure already covers it. - If the store can't be measured, the field is left out rather than reported as 0, so a failed probe doesn't read as a full store. The work dir figure still falls back to 0 on a failed probe, as before. How the control plane uses it: - The `min_free_disk_mb` floor applies to the lower of the work dir and the store. - The reason for holding a node names the short one: "low disk on /nix/store (N MB free, floor F MB)" or "low disk on work dir (…)". - /ui/nodes shows "X MB work dir, Y MB /nix/store" when the store is reported. - GET /api/nodes passes the field through in `heartbeat`, and its `unschedulable` reason names the filesystem. Docs: SPEC §6.2 (heartbeat listing), §6.3 (pseudocode `min(free_disk, store_free_disk) ≥ floor` and the "Taking new work" text) and §7.1 (heartbeat line) are updated, along with the `min_free_disk_mb` doc comment, the forgejo-setup config comment and docs/TODO.md (entry struck through as done). **Why.** `free_disk_mb` measured only the work dir. If /nix/store was on another filesystem and filled up, nothing showed it until builds failed and the circuit breaker opened. This was the gap left open in the node-health TODO after the 2026-09-27 full-disk incident. **Tests.** - kb-core `heartbeat_store_free_disk_is_optional`: a heartbeat from an old agent (no field) still parses; when the field is None it is left out of the JSON; a heartbeat with it round-trips. - Agent: - `the_store_is_reported_only_when_it_is_a_filesystem_of_its_own` (tempdir): store on the same filesystem as the work dir gives None; no `nix` feature gives None; a store on another device (procfs) is reported. - `a_store_that_cannot_be_measured_is_not_reported_full`: an unmeasurable store gives None, while the work dir still reads 0. - The existing heartbeat test now also checks that a node without `nix` reports no store figure. - Health: - `free_disk_floor_judges_the_fuller_filesystem`: the floor uses the lower figure and the reason names the filesystem. - A store lower than the work dir but at the floor holds nothing. - `free_disk_floor` is adapted to the new gate signature. - Integration `node_health::a_node_low_on_store_disk_gets_no_work`: a node with plenty of room on its work dir but 100 MB on its store gets no work. `/api/nodes` names /nix/store, and `/ui/nodes` shows "100000 MB work dir, 100 MB /nix/store". The node gets work again once the store has room. - `cargo clippy --workspace --all-targets -- -D warnings` and `cargo test --workspace` pass. **The `nix build .#ci.*` gate was not run locally** (no nix daemon in the dev VM). No files were added, so the filesets are unchanged. **Deploy notes.** Either side can be upgraded first: - An old agent leaves the field out, and the control plane floors on the work dir alone, as it does today. - An old control plane ignores the field. The store check takes effect once both the control plane and an agent whose store is on its own filesystem are upgraded. A node whose /nix/store filesystem is already below `min_free_disk_mb` (default 4096) stops taking new work at that point. Check `df /nix/store` on nuxbox and ares before switching.

kris added 1 commit 2026-10-01 00:17:18 +02:00

Node health: report /nix/store free disk in the heartbeat, and floor the lower ... 4c7353daee

An agent with the `nix` feature now also reports the free space on the
filesystem holding /nix/store (`store_free_disk_mb`, optional in kb-core's
Heartbeat). The control plane's `min_free_disk_mb` floor judges the lower
of the work dir and the store, and the reason names which one is short
("low disk on /nix/store (…)"). /ui/nodes shows both; GET /api/nodes
carries the field in the heartbeat as reported.

A full store on its own filesystem was invisible until builds failed and
the breaker caught it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

kris commented 2026-10-01 00:23:40 +02:00

Author

Owner

Copy link

Automated review (reviewer subagent) of 4c7353daee

Verdict: approve, with one SPEC fix before merge.

Gates, run on the commit: cargo clippy --workspace --all-targets -j2 -- -D warnings clean; cargo test --workspace -j2 all pass. nix build .#ci.* not run (no nix in the VM; no files added).

Correctness: every free-disk decision goes through least_free: health::blocked_nodes (scheduler and queue view), load_nodes for /api/nodes, and the /ui/nodes badge. Mutation checks: reverting either blocked_nodes or http.rs to the work dir alone fails the new integration test. Compatibility: the field is additive, uses serde(default, skip_serializing_if), and is not hashed, so there is no kbN- bump.

Findings:

1. (fix before merge) SPEC.md:599 §6.3 pseudocode still says free_disk ≥ floor. Change it to min(free_disk, store_free_disk) ≥ floor.
2. (non-blocking) agent.rs:527-531 and 582-600: free_disk_mb returns 0 on a failed statvfs, so a failed probe of the store reads as a full store and holds the node. Suggest a fallible free_disk(path) -> Option<u64> that maps an error to None for the store.
3. (non-blocking) health.rs:343-349: the tie-break assumes one filesystem measured twice ties exactly, but two statvfs calls can differ. Suggest comparing metadata().dev() of the work dir and /nix/store on the agent, and sending None when they match.
4. (non-blocking, tests) agent.rs:1194-1200: the test would still pass if the work dir were measured instead of the store. Suggest a store_free_disk_mb(features, store: &Path) helper tested with a tempdir. The /ui/nodes "X MB work dir, Y MB /nix/store" format has no assertion.
5. (non-blocking, tests) health.rs:742-766: missing the case where the store is lower than the work dir but above the floor (should give None).
6. (deploy) Run df /nix/store on nuxbox and ares before switching. A node gated for low disk is not logged; a one-time warn! could be a follow-up.

## Automated review (reviewer subagent) of 4c7353daee63cd03e26941e4be3d982e0b73b0b1 **Verdict: approve, with one SPEC fix before merge.** Gates, run on the commit: `cargo clippy --workspace --all-targets -j2 -- -D warnings` clean; `cargo test --workspace -j2` all pass. `nix build .#ci.*` not run (no nix in the VM; no files added). Correctness: every free-disk decision goes through `least_free`: `health::blocked_nodes` (scheduler and queue view), `load_nodes` for `/api/nodes`, and the /ui/nodes badge. Mutation checks: reverting either `blocked_nodes` or `http.rs` to the work dir alone fails the new integration test. Compatibility: the field is additive, uses `serde(default, skip_serializing_if)`, and is not hashed, so there is no `kbN-` bump. Findings: 1. **(fix before merge)** SPEC.md:599 §6.3 pseudocode still says `free_disk ≥ floor`. Change it to `min(free_disk, store_free_disk) ≥ floor`. 2. (non-blocking) agent.rs:527-531 and 582-600: `free_disk_mb` returns 0 on a failed statvfs, so a failed probe of the store reads as a full store and holds the node. Suggest a fallible `free_disk(path) -> Option<u64>` that maps an error to `None` for the store. 3. (non-blocking) health.rs:343-349: the tie-break assumes one filesystem measured twice ties exactly, but two statvfs calls can differ. Suggest comparing `metadata().dev()` of the work dir and `/nix/store` on the agent, and sending `None` when they match. 4. (non-blocking, tests) agent.rs:1194-1200: the test would still pass if the work dir were measured instead of the store. Suggest a `store_free_disk_mb(features, store: &Path)` helper tested with a tempdir. The /ui/nodes "X MB work dir, Y MB /nix/store" format has no assertion. 5. (non-blocking, tests) health.rs:742-766: missing the case where the store is lower than the work dir but above the floor (should give `None`). 6. (deploy) Run `df /nix/store` on nuxbox and ares before switching. A node gated for low disk is not logged; a one-time `warn!` could be a follow-up.

kris referenced this pull request 2026-10-01 00:30:59 +02:00

Node health: don't quarantine the last node able to take work when others tripped with it #36

kris added 1 commit 2026-10-02 17:55:39 +02:00

Node health: leave out a store that shares the work dir's filesystem or cannot be measured a6f36373d5

kris referenced this pull request 2026-10-02 17:58:59 +02:00

Node health: don't quarantine the last node able to take work when others tripped with it #36

All checks were successful

Hide checks

krisbuild/kris/krisbuild/nix/hello cached

Details

krisbuild/kris/krisbuild/nix/workspace-deps cached

Details

krisbuild/kris/krisbuild/nix/test-deps cached

Details

krisbuild/kris/krisbuild/nix/world cached

Details

krisbuild/kris/krisbuild/nix/clippy succeeded

Details

krisbuild/kris/krisbuild/nix/test succeeded

Details

krisbuild/kris/krisbuild/nix/build succeeded

Details

krisbuild/kris/krisbuild/nix/deployed succeeded

Details

krisbuild/kris/krisbuild/nix/kb-check succeeded

Details

krisbuild/kris/krisbuild krisbuild kris/krisbuild: all tasks succeeded

Details

This pull request can be merged automatically.

This branch is out-of-date with the base branch

You are not authorized to merge this pull request.

View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u origin nixstore-free-disk:nixstore-free-disk

git switch nixstore-free-disk

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

kris/krisbuild!35

No description provided.