Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Backport 1.19] Enable Windows workers (#17555) #17813

Merged
merged 7 commits into from
Aug 31, 2021

Conversation

davinci26
Copy link
Member

Commit Message:

Backport 1260c5c to 1.19

Fixing an issue where Envoy workers are not picking up connections on Windows.

The root cause of the issue is in the Windows kernel.

There are two issues that we need to consider:

If we want to listen from a duplicated socket then we need to duplicate it after we call listen on the original socket.
If duplicated sockets try to accept at the same time, then one of the accept calls might block. Even if the sockets are non-blocking.
The best way to work around that issue is to only listen/accept connections from the first worker thread and then use ExactConnectionBalance to dispatch the connection to another worker thread. This is a temporary solution until the underlying issue is fixed on Windows.

Signed-off-by: Sotiris Nanopoulos sonanopo@microsoft.com

Risk Level: Low
Testing: Manual testing and tests
Docs Changes: Added
Release Notes: Added a bug fix note
Platform Specific Features: N/A

Sotiris Nanopoulos added 2 commits August 23, 2021 11:54
Fixing an issue where Envoy workers are not picking up connections on Windows.

The root cause of the issue is in the Windows kernel.

There are two issues that we need to consider:

If we want to listen from a duplicated socket then we need to duplicate it after we call listen on the original socket.
If duplicated sockets try to accept at the same time, then one of the accept calls might block. Even if the sockets are non-blocking.
The best way to work around that issue is to only listen/accept connections from the first worker thread and then use ExactConnectionBalance to dispatch the connection to another worker thread. This is a temporary solution until the underlying issue is fixed on Windows.

Signed-off-by: Sotiris Nanopoulos <sonanopo@microsoft.com>
… setting resuse_port on linux

Signed-off-by: Sotiris Nanopoulos <sonanopo@microsoft.com>
@davinci26
Copy link
Member Author

cc @wrowe who is doing the release and @ggreenway / @mattklein123 who did the initial review

cc @envoyproxy/windows-dev

@davinci26 davinci26 added backport/review Request to backport to stable releases area/windows labels Aug 23, 2021
@davinci26
Copy link
Member Author

/retest

@repokitteh-read-only
Copy link

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #17813 (comment) was created by @davinci26.

see: more, trace.

Signed-off-by: Sotiris Nanopoulos <sonanopo@microsoft.com>
@wrowe
Copy link
Contributor

wrowe commented Aug 24, 2021

/retest

@repokitteh-read-only
Copy link

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #17813 (comment) was created by @wrowe.

see: more, trace.

Sotiris Nanopoulos added 2 commits August 25, 2021 10:20
Signed-off-by: Sotiris Nanopoulos <sonanopo@microsoft.com>
@mathetake
Copy link
Member

please wait merge until #17867 lands and then main merge

@ggreenway
Copy link
Contributor

/wait

Signed-off-by: Sotiris Nanopoulos <sonanopo@microsoft.com>
Copy link
Contributor

@wrowe wrowe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, it would be nice to have several working examples to compare between 1.18/1.19/main evolution of the rest of the code. TYVM for the backport submission.

@wrowe wrowe merged commit 7f2d69f into envoyproxy:release/v1.19 Aug 31, 2021
@alyssawilk alyssawilk removed the backport/review Request to backport to stable releases label Nov 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants