Cleanup after restored shim dies unexpectedly#2083
Cleanup after restored shim dies unexpectedly#2083mlaventure wants to merge 1 commit intocontainerd:masterfrom
Conversation
Signed-off-by: Kenfe-Mickael Laventure <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #2083 +/- ##
=======================================
Coverage 45.35% 45.35%
=======================================
Files 96 96
Lines 9453 9453
=======================================
Hits 4287 4287
Misses 4455 4455
Partials 711 711
Continue to review full report at Codecov.
|
|
@estesp Yep, witholding my LGTM until I get a chance to test. |
|
LGTM |
| go func() { | ||
| ctx = namespaces.WithNamespace(context.Background(), ns) | ||
| for { | ||
| _, err := s.Wait(ctx, &shim.WaitRequest{id}) |
There was a problem hiding this comment.
Instead of having a blocking connection for every shim, could we change ttprc to have a shutdown hook. ttrpcConn.OnShutdown(func() {}) that any call that gets an ErrClosed runs the shutdown function to cleanup the connection? @stevvooe
There was a problem hiding this comment.
I was thinking a goroutine + connection for every shim is heavy.
There was a problem hiding this comment.
Currently it's the only way I can see, unless we go with @crosbymichael suggestion, which would definitely be better on resources usage
There was a problem hiding this comment.
Looking closer, an OnShutdown method in ttrpc would help here, assuming we are tying everything to the lifecycle of the client, and not the connection. If we want better control over the connection lifecyle, we'll have to introduce a pool. In this case, we are clearly using the client lifecycle, so this change would work well.
6 participants
Signed-off-by: Kenfe-Mickael Laventure [email protected]
--
Fixes #2078