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.

Sign up for GitHub

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

Jump to bottom

feat: close listeners in registration rundown #3823

Merged
merged 7 commits into from
Aug 28, 2023
Merged

feat: close listeners in registration rundown #3823

merged 7 commits into from
Aug 28, 2023

Conversation

qzhuyan
Copy link
Contributor

@qzhuyan qzhuyan commented Aug 22, 2023

Description

fix #3818

Testing

Do any existing tests cover this change? Are new tests needed?

Documentation

Is there any documentation impact for this change?

nibanks
nibanks reviewed Aug 22, 2023
View reviewed changes
src/core/registration.c Outdated Show resolved Hide resolved
nibanks
nibanks reviewed Aug 22, 2023
View reviewed changes
src/core/registration.c Outdated Show resolved Hide resolved
qzhuyan
qzhuyan commented Aug 22, 2023
View reviewed changes
src/core/listener.c Outdated Show resolved Hide resolved
@qzhuyan qzhuyan force-pushed the dev/william/close-listn-in-registr-rundown branch from 4376cbb to 50969b3 Compare August 25, 2023 15:14
qzhuyan
qzhuyan commented Aug 25, 2023
View reviewed changes
src/core/listener.c Outdated Show resolved Hide resolved
nibanks
nibanks reviewed Aug 25, 2023
View reviewed changes
src/core/registration.c Outdated Show resolved Hide resolved
nibanks
nibanks reviewed Aug 25, 2023
View reviewed changes
src/core/registration.h Outdated Show resolved Hide resolved
nibanks
nibanks reviewed Aug 25, 2023
View reviewed changes
src/core/listener.c Outdated Show resolved Hide resolved
nibanks
nibanks reviewed Aug 25, 2023
View reviewed changes
src/core/listener.c Outdated Show resolved Hide resolved
nibanks
nibanks reviewed Aug 25, 2023
View reviewed changes
src/core/listener.c Outdated Show resolved Hide resolved
@qzhuyan
Copy link
Contributor Author

qzhuyan commented Aug 25, 2023

two TCs are failing at local run, will check later...

Handshake_WithHandshakeArgs10.HandshakeSpecificLossPatterns_2
Handshake_WithHandshakeArgs10.HandshakeSpecificLossPatterns_3

@qzhuyan
Copy link
Contributor Author

qzhuyan commented Aug 28, 2023

two TCs are failing at local run, will check later...

Handshake_WithHandshakeArgs10.HandshakeSpecificLossPatterns_2
Handshake_WithHandshakeArgs10.HandshakeSpecificLossPatterns_3

Are these two flaky test cases? Now I cannot reproduce them.

@nibanks
Copy link
Member

nibanks commented Aug 28, 2023

@qzhuyan do you want to mark this "Ready for review"?

@qzhuyan
Copy link
Contributor Author

qzhuyan commented Aug 28, 2023 •
edited
Loading

@qzhuyan do you want to mark this "Ready for review"?

I am waiting for CI jobs to finish to see if there are any regressions.
I see windows kernel mode perf regression -25%, is it something we should worry about?

Performance regression in ThroughputDown_Winkernel_x64_schannel_Default. -22.07% < -10

@nibanks
Copy link
Member

nibanks commented Aug 28, 2023

@qzhuyan do you want to mark this "Ready for review"?

I am waiting for CI jobs to finish to see if there are any regressions. I see windows kernel mode perf regression -25%, is it something we should worry about?

Performance regression in ThroughputDown_Winkernel_x64_schannel_Default. -22.07% < -10

No, the results are inconsistent. You can ignore it.

@nibanks
Copy link
Member

nibanks commented Aug 28, 2023

I am waiting for CI jobs to finish to see if there are any regressions.

Generally, everything looks fine.

@qzhuyan qzhuyan marked this pull request as ready for review August 28, 2023 17:54
@qzhuyan qzhuyan requested a review from a team as a code owner August 28, 2023 17:54
@qzhuyan qzhuyan changed the title WIP: feat: close listeners in registration rundown feat: close listeners in registration rundown Aug 28, 2023
@nibanks nibanks added Area: API Area: Core Related to the shared, core protocol logic labels Aug 28, 2023
@nibanks nibanks merged commit f91ccc2 into microsoft:main Aug 28, 2023
418 checks passed
@qzhuyan qzhuyan deleted the dev/william/close-listn-in-registr-rundown branch August 29, 2023 07:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nibanks nibanks nibanks approved these changes

Assignees
No one assigned
Labels
Area: API Area: Core Related to the shared, core protocol logic
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support Listener stop via RegistrationShutdown API
2 participants
@qzhuyan @nibanks