mirror of
https://github.com/containerd/containerd.git
synced 2026-11-06 05:17:02 +00:00
Md Tahsin Rahman
1130a1f4c6
Init.Delete takes p.mu and holds it across initState.Delete, whose
delete() shuts the terminal console down and waits up to 10s for stdio
to drain, execs `runc delete`, closes the process IO and console and
unmounts the rootfs. Init.Status, ExitStatus and
ExitedAt take the same mutex, so the shim's task.State RPC, which calls
all three, queues behind a Delete. containerd gives State a 2s deadline
(io.containerd.timeout.task.state), so the call fails. Depending on the
containerd version that is logged as "get state for <id>: context
deadline exceeded" and the task reported as UNKNOWN, or the error is
propagated, making Get fail and List drop the task. execProcess has the
identical shape with e.mu.
This is easy to hit when the unmount is slow. On a node with tens of GB
of dirty page cache, umount of the overlayfs rootfs calls
sync_filesystem() on the upper filesystem and was measured at 17.6s with
12.9 GB dirty (0.22s with 1.0 GB). That node logged 4,661 State timeouts
across 284 containers over a window of heavy writes.
This change lets State answer while Delete runs. Delete now releases mu
for the slow cleanup, and every method that is not a plain reader still
waits for the cleanup to end:
- Init and execProcess get a `deleting` channel, read and written only
with mu held. delete() is entered and left with mu held; it sets
`deleting`, unlocks, runs the old body unchanged, and a deferred
relock clears `deleting` and closes the channel, whether the cleanup
succeeded, failed or panicked.
- A new lock() takes mu and, while `deleting` is set, unlocks, waits for
the channel and locks again. Every method that
changes the process (Init and exec alike) now calls lock() instead of
mu.Lock(), so the cleanup is still exclusive with all of them, exactly
as when Delete held mu throughout. They then run
against deletedState if Delete succeeded, or the previous state if it
failed, which is what they saw before.
- Status, ExitStatus and ExitedAt keep taking mu directly and so no
longer wait for the cleanup. They report the state from before the
Delete, since nothing is changed until it ends: a created process
stays created with a zero exit status and exit time, and a stopped
one keeps its real exit status and exit time. There is no
created -> stopped -> created flicker if the Delete fails.
The state machine (init_state.go, exec_state.go, deleted_state.go) is
unchanged, and there is no second lock: readers still take mu, so Status
during an in-flight `runc start` still waits and returns "running",
which CRI recovery relies on (a task seen as created is killed).
Behavior change: State/Status/ExitStatus/ExitedAt no longer wait for the
cleanup part of Delete and report the state from before the Delete. If
the Delete fails nothing changes, and an exit that arrived meanwhile is
applied afterwards, as it was on main. One consequence: while the
Delete of a created task with a non-zero pid runs, a second plain
task.Delete from the client (no WithProcessKill) sees "created" and
fails at once with "task must be stopped before deletion", where it
used to wait for the first Delete, see "stopped" and get NotFound.
Everything else is unchanged: other calls wait for Delete as before, a
second Delete waits and gets NotFound, exit events wait, and the task
service still removes the process only after Delete returns.
Because mu is now released inside delete(), any new method that changes
a process must take mu through lock(); this is documented on mu.
This does not make Delete faster. It still takes as long as before, and
the dirty page cache that makes umount slow is a separate problem.
Timeouts are unchanged.
Tests: Delete is blocked inside an io.Closer; State must return and
report the pre-Delete state, mutating methods must wait, and readers
must take mu.
Assisted-by: Claude Code (Claude Sonnet 5.5)
Signed-off-by: Md Tahsin Rahman <tahsin8243@gmail.com>
(cherry picked from commit a71f15225d)