Skip to content

Sweep a killed request's tree from a supervisor-forked sweeper - #63

Merged
flavorjones merged 4 commits into
masterfrom
scratch-maintenance-worker
Sep 9, 2026
Merged

flavorjones merged 4 commits into
masterfrom
scratch-maintenance-worker

Conversation

@flavorjones

Copy link
Copy Markdown
Member

The supervisor renamed a killed worker's home aside and left the unlinking to
the next worker that answered on that slot. A worker killed at its deadline
never reached that sweep, so a slot whose every request was killed stacked
one tree per kill until the scratch was full.

Fork a Sweeper every sweep_interval seconds when a slot holds a discarded
tree, hold it to the cell's deadline, and run one at a time. The worker's
own sweep stays as the fast path; Filesystem.remove_tree now counts a tree
the other sweeper removed first as removed. See HotCell::Sweeper and the
sweep_interval row in docs/DEPLOYMENT.md.

🤖 Generated with Claude Code

The supervisor renamed a killed worker's home aside and left the unlinking to
the next worker that answered on that slot. A worker killed at its deadline
never reached that sweep, so a slot whose every request was killed stacked
one tree per kill until the scratch was full.

Fork a `Sweeper` every `sweep_interval` seconds when a slot holds a discarded
tree, hold it to the cell's `deadline`, and run one at a time. The worker's
own sweep stays as the fast path; `Filesystem.remove_tree` now counts a tree
the other sweeper removed first as removed. See `HotCell::Sweeper` and the
`sweep_interval` row in docs/DEPLOYMENT.md.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings September 9, 2026 17:49

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Comment thread docs/DEPLOYMENT.md Outdated
Comment thread hotcell-server/lib/hot_cell/filesystem.rb Outdated
Comment thread hotcell-server/lib/hot_cell/supervisor.rb
Comment thread hotcell-server/test/sweep_test.rb
Comment thread CHANGELOG.md Outdated
Comment thread CHANGELOG.md Outdated
flavorjones and others added 2 commits September 9, 2026 14:18
`Filesystem.remove_tree` read a path it could not stat as gone, so a tree
behind a slot directory a tool made unsearchable was counted as swept. The
supervisor listed and sorted every slot directory in its loop to see whether
a sweep was due, and a sweeper that died by signal left no line saying so.

Only ENOENT means gone. Stream the slot directory and stop at the first
discarded entry. Apply the cell's limits in the sweeper, log `sweeper.died`
from the reap, and give `slot.unswept` the WARN level docs/LOGS.md already
promised.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The sweeper took every worker limit, and RLIMIT_FSIZE killed it on its
first log line whenever the log was a regular file already past
`file_size`. A slot reported early was then the last one ever swept.

Apply `memory` alone; the sweeper writes nothing but log lines. The
comment on `Sweeper#run` now says what that limit does and does not bound.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@flavorjones

Copy link
Copy Markdown
Member Author

🤖 Adversarial review by Codex, three rounds, converged.

Fixed (commits eec09ad and c50f6d1)

  • Filesystem.remove_tree read a path it could not stat as gone, so a tree behind a slot directory a tool made unsearchable was counted as swept. present? now uses lstat; only ENOENT means gone.
  • The supervisor's due-check listed and sorted every slot directory in its loop. Slot#discarded? streams the directory and stops at the first match.
  • A sweeper that died by signal left no line. reap logs sweeper.died with signal and exit code unless the deadline kill already logged it.
  • Applying every worker limit to the sweeper put RLIMIT_FSIZE on a process whose only writes are log lines; a regular-file log past file_size killed it on its first line. It applies memory alone now.
  • slot.unswept was documented WARN but missing from Log::LEVELS; added. hotcell.swept reworded in docs/LOGS.md.
  • Tests: EACCES sweep, discarded?, signal death, file_size regression, and both signal tests now assert a replacement sweeper forked.

Declined, with reasons

  • Rewrite Filesystem as a streaming remover so a directory wider than the memory limit makes progress: a rewrite for an attacker-shaped case the pre-existing worker sweep shares. Risk recorded in the Sweeper#run comment.
  • Reject Float::INFINITY for sweep_interval: queue_wait and control_deadline take the identical path; a fix belongs to all three, outside this PR.
  • Replaced-slot-directory symlink TOCTOU during a sweep: same-uid actor can already delete anything this uid owns; Codex rated it correct but not worth doing.
  • A test-only pause hook in Sweeper for deterministic signal tests: the tests fail loudly rather than pass falsely, and the timing margin is hundreds of ms against a 20ms poll. Will revisit if CI flakes.

The sweeper carried its own copy of the worker's deadline timer and its
own kill, `present?` said nothing about what it answered, and the
`sweep_interval` row and changelog entries ran long.

Share the timer as `Timed` between `Child` and `Sweep`, latch both with
`killed_for`, and kill both through `kill_group`. Rename `present?` to
`gone?`. Rewrite the setting's row, trim the changelog, and say in
`sweep_test.rb` what in it is real.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@flavorjones
flavorjones merged commit 849f358 into master Sep 9, 2026
16 checks passed
@flavorjones
flavorjones deleted the scratch-maintenance-worker branch September 9, 2026 18:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants