Allocate a conhost during Windows service startup#3450
Merged
crosbymichael merged 1 commit intocontainerd:masterfrom Jul 25, 2019
Merged
Allocate a conhost during Windows service startup#3450crosbymichael merged 1 commit intocontainerd:masterfrom
crosbymichael merged 1 commit intocontainerd:masterfrom
Conversation
Creating a console for containerd causes it to be inherited by any child processes, which gives us performance and reliability improvements. See comment in code for more information. Another option considered here would be to invoke each child process with the DETACHED_PROCESS flag. This would save us the containerd console allocation. The difficulty of this approach would be ensuring that all process invocation points have had this flag added, and that any future invocations also use the flag. Signed-off-by: Kevin Parsons <[email protected]>
Contributor
|
@crosbymichael - Never seen the vendor script timeout before :) |
jterry75
approved these changes
Jul 25, 2019
Contributor
jterry75
left a comment
jterry75
left a comment
There was a problem hiding this comment.
LGTM. I think this is the right approach over adding this to every cmd invocation. Containerd is a distributed model based on process activation for many things so simply inheriting the conhost seems a good approach which your comment makes clear. Thanks
Member
Author
|
FYI: We're still investigating the underlying issue that is causing these conhost launches to fail. But even if that were fixed, this is still a righteous change as we won't waste time creating conhosts when we don't actually want them. |
Member
|
LGTM |
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
3 participants
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.
Creating a console for containerd causes it to be inherited by any child
processes, which gives us performance and reliability improvements. See
comment in code for more information.
Another option considered here would be to invoke each child process
with the DETACHED_PROCESS flag. This would save us the containerd
console allocation. The difficulty of this approach would be ensuring
that all process invocation points have had this flag added, and that
any future invocations also use the flag.
Signed-off-by: Kevin Parsons [email protected]