Skip to content

server: skip lifecycle notifications on early envoy shutdown#7585

Merged
htuch merged 2 commits intoenvoyproxy:masterfrom
eziskind:lifecycle
Jul 17, 2019
Merged

server: skip lifecycle notifications on early envoy shutdown#7585
htuch merged 2 commits intoenvoyproxy:masterfrom
eziskind:lifecycle

Conversation

@eziskind
Copy link
Copy Markdown
Contributor

Description: Skip lifecycle notifications which take a completion callback when envoy shuts down early before starting worker threads. This is needed because these registrations are typically implemented using Slot::runOnAllThreads which will not work correctly if worker threads have not been started.
Risk Level: low
Testing: unit tests

Signed-off-by: Elisha Ziskind eziskind@google.com

Signed-off-by: Elisha Ziskind <eziskind@google.com>
@zuercher
Copy link
Copy Markdown
Member

@htuch can you have a look?

/assign @htuch

Copy link
Copy Markdown
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

Copy link
Copy Markdown
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

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

Actually, before merging, I'm wondering if we need some documentation changes or something to make clear that the server lifecycle notifications only begin after all workers are started?

Signed-off-by: Elisha Ziskind <eziskind@google.com>
@eziskind
Copy link
Copy Markdown
Contributor Author

Actually, before merging, I'm wondering if we need some documentation changes or something to make clear that the server lifecycle notifications only begin after all workers are started?

Added comments to the header file

Copy link
Copy Markdown
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

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

Thanks!

@htuch htuch merged commit d63befe into envoyproxy:master Jul 17, 2019
@eziskind eziskind deleted the lifecycle branch July 17, 2019 21:44
TAOXUY pushed a commit to TAOXUY/envoy that referenced this pull request Jul 22, 2019
…oxy#7585)

Skip lifecycle notifications which take a completion callback when envoy shuts down early before starting worker threads. This is needed because these registrations are typically implemented using Slot::runOnAllThreads which will not work correctly if worker threads have not been started.

Risk Level: low
Testing: unit tests

Signed-off-by: Elisha Ziskind <eziskind@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants