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

fix(input_messenger) client side retry policy #1680

Merged
merged 2 commits into from
Mar 4, 2022
Merged

Conversation

ehds
Copy link
Contributor

@ehds ehds commented Jan 22, 2022

Client Ptotocol Rule: Ptotocol in client-side is fixed except baidu_std, because of streaming_rpc need create by baidu_std ptotocol.

But in current implementation, there is a chance that client will accept message but not match with current protocol.

Case:

Client issue two RPC names A (use hulu protocol) and B(use baidu_std protocol) to server in the same connection, If A is sent before B, client will set preferred protocol to baidu_std, server will accept these two request and response. Then if response of B received by client before A, B will be parsed success, because it is matched with current protocol(baidu_std). When parse response of A by current preferred ptotocol(baidu_std) will fail, according to Client Ptotocol Rule, client think this response is streaming_rpc, and try other ptotocols, then response of B will be parsed ok by hulu ptotocol.

That's the point: Shall we just retry streaming_rpc when parse failed if current protocol is baidu_std? instead of trying all other protocols, or other ptotocol mixed with baidu_std will be accepted successfully.

client retry parse message when baidu_std fall to streaming_rpc
@wwbmmm
Copy link
Contributor

wwbmmm commented Jan 24, 2022

@ehds Thanks for you contribution!

@ehds ehds requested a review from lorinlee March 4, 2022 07:24
@lorinlee lorinlee merged commit e9d12ea into apache:master Mar 4, 2022
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