[release/1.1] cherry-pick: Increase reaper buffer size#2762
[release/1.1] cherry-pick: Increase reaper buffer size#2762estesp merged 2 commits intocontainerd:release/1.1from
Conversation
Codecov Report
@@ Coverage Diff @@
## release/1.1 #2762 +/- ##
============================================
Coverage 48.99% 48.99%
============================================
Files 85 85
Lines 7603 7603
============================================
Hits 3725 3725
Misses 3203 3203
Partials 675 675
Continue to review full report at Codecov.
|
|
LGTM |
|
Enlarging the channel size seems fine to me, I'm a bit uncomfortable with discarding events. Init/exec process exit events come only once, and are very important for the container lifecycle. Discarding the event feels unsafe to me, and will cause state mismatch. I think the sender should either block, or cache the event and retry. I think #2765 to fix the deadlock seems a better fix. |
|
any opinions @crosbymichael? We already have the events change in 1.2/master, so whatever we decide, if we change anything we should remember to "fix" master as well. |
|
At least for CRI plugin, if the container exit event is discarded, the container won't be stopped any more. Only when user restarts containerd, CRI can recover state by listing/inspecting all the containers. Our current assumption in CRI is that event is reliable, and we can purely rely on it. If that is not the case, we'll probably need to add periodically relisting to sync state for each container. |
|
We could revert the change for dropping the send and keep the larger buffer size in addition to the #2765 change |
SGTM. :) Thanks a lot! |
|
If so, I'll send partial fix to revert the event discard change. For 1.1, it seems that we have all changes we want. |
|
@Random-Liu go ahead and send a patch. I'm working on a fix for the pause and state issues right now that some have been seeing, the weird bug |
|
Should I update this PR with just the buffer size change? |
Fixes containerd#2709 This increases the buffer size for process exit subscribers. It also implements a non-blocking send on the subscriber channel. It is better to drop an exit even than it is to block a shim for one slow subscriber. Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
Signed-off-by: Lantao Liu <lantaol@google.com> Signed-off-by: Phil Estes <estesp@linux.vnet.ibm.com>
e2ac712 to
966fc1a
Compare
|
@Random-Liu updated! |
|
LGTM |
1 similar comment
|
LGTM |
|
Looks like GitHub is a bit too eager hiding comments; here it's hiding "LGTM"'s
|
Cherry-pick #2748
Cherry-pick #2770
This increases the buffer size for process exit subscribers.
It alsoimplements a non-blocking send on the subscriber channel. It is better
to drop an exit even than it is to block a shim for one slow subscriber.
Signed-off-by: Michael Crosby crosbymichael@gmail.com
Signed-off-by: Lantao Liu lantaol@google.com
Signed-off-by: Phil Estes estesp@linux.vnet.ibm.com