Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This comment has been minimized.
This comment has been minimized.
rohnnyjoy
force-pushed
the
worker-session-sampling
branch
from
October 11, 2026 17:10
a785a0a to
4b91566
Compare
The worker started each action as its own process-group leader, sampled
the group's memory and CPU, and killed the group. Processes leave a
group freely: a job-control shell moves every job to a group of its own
(Bazel's test-setup.sh runs `set -m` before every test), and a detached
spawn (Node's `spawn(..., { detached: true })`, a daemonizing server)
calls `setsid`, which leaves the session too. So the sampler measured
the shell waiting for the test (a two-second busy test read as 0 ms of
CPU and 194 KiB peak; a web e2e test whose harness detaches its server
read as 60 MB while using gigabytes), soft memory and disk enforcement
could not see the real work, and a kill left it running.
Run every action under a stub, as Bazel's process-wrapper does for a
local action. The spawned child calls `setsid` and forks: the child
execs the action, the parent stays as its stub. The stub is a child
subreaper, so a process the action orphans is reparented to it, never
to the worker, whatever session or group it moved to: the action's
processes are exactly the stub's descendants. The sampler walks that
tree in /proc each tick (not the stub, a fork of the worker whose pages
are the worker's). The SIGTERM grace and the hard kill signal every
descendant, through a pidfd after checking the parent again, so a
reused pid is never signalled, and the hard kill repeats until none is
left alive. When the action's own process exits the stub kills what is
left, reaps it, and exits with the action's status, as process-wrapper's
TerminateAndWaitForAll does, so nothing outlives its action to hold a
port or memory the next action is charged for. Before exiting it writes
the kernel's CPU total of everything it reaped (getrusage
RUSAGE_CHILDREN) to a pipe the worker reads, which replaces the sampled
CPU: exact, including processes too short-lived for any sample.
With namespaces on, the same stub sits outside the PID namespace the
action is the init of; it replaces the namespace's own fork. Pooled
persistent workers are sampled from their own pid. If the stub itself
is killed, its orphans go to the worker, and the session it leads is
still signalled. Without /proc (macOS) there is no stub and the session's
group is signalled, as before.
Tests: the action runs under a stub that leads its group and session; a
busy loop in a `set -m` job and in a `setsid` job is the action's CPU;
256 MiB held by a `setsid` job is the action's memory; a job left in a
group of its own and one left in a session of its own are killed when
the action ends. The three detached tests fail on the code before this
change.
rohnnyjoy
force-pushed
the
worker-session-sampling
branch
from
October 12, 2026 03:06
4b91566 to
b982f6d
Compare
`bazel run //nativelink-proto:update_protos` on this branch; the checked-in file had been formatted by hand and `//nativelink-proto:update_protos_test` refused it.
The two tests spawn their job with util-linux's `setsid` and hold its memory from python3. The coverage build runs the tests in a nix sandbox with neither on PATH, so the job never started and the tests measured an action that finished in two milliseconds. Skip them where the tools are missing, as the other tests here do with `which`.
rohnnyjoy
force-pushed
the
worker-session-sampling
branch
from
October 16, 2026 20:47
fba85fe to
13a4494
Compare
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What and why
The worker treats an action's process group as the action: it samples the
group's memory and CPU, enforces against it, and kills it. Processes leave a
group freely. A job-control shell moves each job to a group of its own, and
Bazel's test wrapper (
test-setup.sh) runsset -mbefore starting everytest. A detached spawn (Node's
spawn(..., { detached: true }), adaemonizing server) calls
setsid, which leaves the session as well. So atest measures as its waiting shell (a two-second busy test: 0 ms CPU, 194 KiB
peak; a web e2e test whose harness detaches its server: 60 MB while using
gigabytes), soft memory and disk enforcement never see it, and a kill leaves
it running.
This runs every action under a stub, as Bazel's
process-wrapperdoes for alocal action (
src/main/tools/process-tools-linux.cc). The stub is a childsubreaper, so anything the action orphans is reparented to it, never to the
worker, whatever session or group it moved to. The action's processes are
exactly the stub's descendants.
Fixes #2921.
action_stub.rs(new):fork_stub, forpre_exec. Aftersetsidthespawned child forks. The child (with
PR_SET_PDEATHSIG) execs the actionand the parent stays as the stub. The stub passes SIGTERM on and reaps as
children exit. When the action's own process exits, it kills and reaps
everything left, reading
/proc/thread-self/childrenuntilwaitpidreports
ECHILD(process-wrapper'sTerminateAndWaitForAll). Then itwrites a
StubReportto a pipe and exits with the action's status. Thereport holds
getrusage(RUSAGE_CHILDREN)(the kernel's exact CPU total ofeverything reaped) and the straggler count. All of it is
async-signal-safe: system calls only, no allocation.
process_tree.rs(wasprocess_session.rs):tree_members(root): one pass over/procthat builds the parent map.signal_descendants(root, signal): signals through a pidfd after readingthe parent again, so a reaped and reused pid is never signalled. SIGKILL
repeats until no descendant is alive, bounded at 16 passes.
killed, and on macOS.
running_actions_manager.rs:is a fork of the worker and its pages are the worker's.
killed stub.
namespace_utils.rs: with namespaces on, the same stub replaces thenamespace's own fork and sits outside the PID namespace the action is the
init of.
MaybeNamespacedChildbecomesActionStubChild. Its kill endsthe stub's descendants first and then the stub, so nothing is orphaned to
the worker by the stub's death.
persistent_worker/live_worker.rs: a pooled worker is sampled from its ownpid. Under namespaces it gets a stub with no report.
MemoryEnforcement::Soft), theActionResourceUsage.cpu_time_msproto doc and the generated config docs.How was this verified?
New tests in
nativelink-worker/tests/running_actions_manager_test.rs:no_namespace_action_runs_under_its_stub(replacesno_namespace_action_is_process_group_leader): the action's shell readsits ppid, pgrp and session from
/proc/$$/stat. All three are equal, andthe parent is not the worker.
job_control_job_is_measured:set -mand a two-second busy loop in abackground job. Asserts
cpu_time_ms >= 500.detached_job_is_measured: the same loop undersetsid.detached_job_memory_is_measured: 256 MiB held for two seconds undersetsid. Assertspeak_memory_kb >= 200 MiB.job_control_straggler_is_killedanddetached_straggler_is_killed: asleep 300left in its own group, and in its own session. After the actionreturns, the sleep is gone or a zombie within 5 s.
parse_ppid_from_stat_*,parse_session_from_stat_*andis_zombie_stat_*.The three detached tests fail on main without the change:
With it,
cargo test -p nativelink-workerpasses every suite: 80 inrunning_actions_manager_test, 12 innamespace_utils_test.cargo clippy -p nativelink-worker --all-targets -- -D warningsis clean andthe crate is formatted with nightly rustfmt.
We run this on a production fleet as a carried patch on v1.7.5 (the same
change, cut against that tag).
Risk
worker that touches almost none of its pages. Namespaced actions already
had one.
before the action completes. The worker already did this to the session's
stragglers, and a detached process now cannot escape it. An action that
relied on a daemon outliving it would notice.
/procis scanned per sample as before; the filter is the parent mapinstead of the session field. Kills scan it once or a few times.
by the session, as before this change. A cgroup per action would close
that, where the worker may create cgroups.
AI assistance
Written by an AI agent (Claude Code). I reviewed every line and ran the tests myself.