Skip to content

Use wait_for_ready in goaway_server_test.#14121

Merged
markdroth merged 1 commit intogrpc:masterfrom
markdroth:goaway_server_test_fix
Jan 22, 2018
Merged

Use wait_for_ready in goaway_server_test.#14121
markdroth merged 1 commit intogrpc:masterfrom
markdroth:goaway_server_test_fix

Conversation

@markdroth
Copy link
Copy Markdown
Member

@markdroth markdroth commented Jan 22, 2018

Fixes #14115.

Previously, the test would fail if the resolver happened to return an error between the time that the second request was started and when the resolver returned the real result. Changing the RPCs to set wait_for_ready should fix this.

@ncteisen, can you please test this on your mac? Thanks!

@markdroth markdroth requested a review from dgquintas January 22, 2018 20:32
@grpc-testing
Copy link
Copy Markdown

grpc-testing commented Jan 22, 2018 

****************************************************************

libgrpc.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]


****************************************************************

libgrpc++.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]

@ncteisen
Copy link
Copy Markdown
Contributor

ncteisen commented Jan 22, 2018

Just ran 500 and there were no failures, so this looks good

@markdroth
Copy link
Copy Markdown
Member Author

markdroth commented Jan 22, 2018

Awesome, thanks, Noah!

@grpc-testing
Copy link
Copy Markdown

grpc-testing commented Jan 22, 2018 

[trickle] No significant performance differences

@grpc-testing
Copy link
Copy Markdown

grpc-testing commented Jan 22, 2018 

[microbenchmarks] No significant performance differences

@markdroth
Copy link
Copy Markdown
Member Author

markdroth commented Jan 22, 2018

Known failures: #13429 #13477

@markdroth markdroth merged commit 6827d44 into grpc:master Jan 22, 2018
@markdroth markdroth deleted the goaway_server_test_fix branch January 22, 2018 23:09
@markdroth markdroth mentioned this pull request Jan 22, 2018
Closed
@mehrdada
Copy link
Copy Markdown
Contributor

mehrdada commented Jan 22, 2018

Does this affect 1.9.x as well? If so, can we get it backported right away before cutting an rc2?

@mehrdada
Copy link
Copy Markdown
Contributor

mehrdada commented Jan 23, 2018

Was this just a problem with the test? Could these issues be related by any chance and this was a symptom treatment? #14088 #14109

@markdroth
Copy link
Copy Markdown
Member Author

markdroth commented Jan 23, 2018

It was just a problem with the test, so no changes are needed in 1.9.x.

I don't think either of the issues you mentioned are related.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@dgquintas dgquintas dgquintas approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

goaway_server_test flaky on macos

5 participants

@markdroth @grpc-testing @ncteisen @mehrdada @dgquintas