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

thrift_proxy: integration test is flakey #4416

Closed
zuercher opened this issue Sep 13, 2018 · 1 comment · Fixed by #4418
Closed

thrift_proxy: integration test is flakey #4416

zuercher opened this issue Sep 13, 2018 · 1 comment · Fixed by #4418
Assignees
Labels

Comments

@zuercher
Copy link
Member

zuercher commented Sep 13, 2018

I’ve reproduced it locally.

@zuercher zuercher changed the title thrift_proxy: integration test is flakes thrift_proxy: integration test is flakey Sep 13, 2018
@zuercher zuercher self-assigned this Sep 13, 2018
@zuercher
Copy link
Member Author

Turns out this is actually a bug. If the downstream client closes after a partial request, but after an upstream connection is formed, it hangs.

zuercher added a commit to turbinelabs/envoy that referenced this issue Sep 14, 2018
In this case, the early close fix only worked if the upstream connection
hadn't yet been formed. Instead, handle early close as long as there
isn't a pending oneway request.

Risk Level: low
Testing: added tests, ran 2000x each
Doc Changes: n/a
Release Notes: n/a
Fixes: envoyproxy#4416

Signed-off-by: Stephan Zuercher <[email protected]>
zuercher added a commit that referenced this issue Sep 14, 2018
In this case, the early close fix only worked if the upstream connection
hadn't yet been formed. Instead, handle early close as long as there
isn't a pending oneway request. Also discovered a case where the
Unframed transport causes an RPC to start on a zero-length buffer with
end_stream set.

Risk Level: low
Testing: added tests, ran 2000x each
Doc Changes: n/a
Release Notes: n/a
Fixes: #4416

Signed-off-by: Stephan Zuercher <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant