Make kill, pause, and unpause parallel. #1737
Conversation
|
Do we have perf numbers on this? I have nothing really against it, but I feel like we're buying almost nothing here, there are not expensive operations. |
|
yes it is faster |
docs/podman-unpause.1.md
Outdated
|
|
||
| **--all, -a** | ||
|
|
||
| Unpause all running containers. |
| for _, ctr := range unpauseContainers { | ||
| con := ctr | ||
| f := func() error { | ||
| return con.Unpause() |
There was a problem hiding this comment.
This won't work for all I think - it'll throw an error on every container that is not paused
There was a problem hiding this comment.
note that the unpauseContainers are containers in the state of paused ... so we should be ok
There was a problem hiding this comment.
Could still be racy - but probably fine, given that we guarantee they are paused.
There was a problem hiding this comment.
Wait, where are you verifying this? I don't see any code to filter the containers in question
cmd/podman/unpause.go
Outdated
| unpauseFlags = []cli.Flag{ | ||
| cli.BoolFlag{ | ||
| Name: "all, a", | ||
| Usage: "stop all running containers", |
There was a problem hiding this comment.
Thought @mheon pinged this one. "stop all running" to "unpause all paused"
| @@ -9,11 +9,28 @@ podman\-pause - Pause one or more containers | |||
| ## DESCRIPTION | |||
There was a problem hiding this comment.
The synopsis for each of these man pages needs to be updated. the "CONTAINER ..." now needs to be in brackets too as it's not always required due to the --all switch. See https://github.com/containers/buildah/blob/master/docs/buildah-rm.md
baude
force-pushed
the
parakill
branch
2 times, most recently
from
ab4ae10
to
be34c60
Compare
|
bot, retest this please |
|
Ah, it's the return of the papr timeout-after-successful-test |
|
/approve |
|
LGTM. @TomSweeneyRedHat @umohnani8 @rhatdan PTAL |
|
/lgtm |
|
☔ The latest upstream changes (presumably #1738) made this pull request unmergeable. Please resolve the merge conflicts. |
Operations like kill, pause, and unpause -- which can operation on one or more containers -- can greatly benefit from parallizing its main job (eq kill). In the case of pauseand unpause, an --all option as was added. pause --all will pause all **running** containers. And unpause --all will unpause all **paused** containers. Signed-off-by: baude <bbaude@redhat.com>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mheon The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci-robot
added
the
approved
|
/lgtm |
github-actions
bot
added
the
locked - please file new issue/PR
Operations like kill, pause, and unpause -- which can operation on one or
more containers -- can greatly benefit from parallizing its main job (eq kill).
In the case of pauseand unpause, an --all option as was added. pause --all will
pause all running containers. And unpause --all will unpause all paused
containers.
Signed-off-by: baude bbaude@redhat.com