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

[retry-middleware] Stream is finished when retry timeout ends #636

Closed
DavyJohnes opened this issue Sep 7, 2023 · 2 comments
Closed

[retry-middleware] Stream is finished when retry timeout ends #636

DavyJohnes opened this issue Sep 7, 2023 · 2 comments

Comments

@DavyJohnes
Copy link
Contributor

Go version used: 1.21

What happened: Current implementation of StreamClientInterceptor uses perCallContext which creates context with deadline, then streamer() is called with this context. This leads to stream closing from client side when context deadline (which is actually retry timeout) ends. I don't think this is good behavior when stream is closed automatically by retry middleware.

What you expected to happen: I expect to be able to close stream by myself or not close it at all. I see two possible solutions:

  1. Do not pass context with deadline to stream initialization (streamer() call) at all. So we will have retries with timeouts on Recv() only.
  2. Do not pass context with deadline to stream initialization, but somehow track stream success initialization and retry if it was not happen for defined retry timeout.

I can create PR with 1 solution, but I need approve from maintainers.

How to reproduce it (as minimally and precisely as possible):

-->

@johanbrandhorst
Copy link
Collaborator

I like option 1, I agree we shouldn't use the retry timeout for the stream initialization call.

@DavyJohnes
Copy link
Contributor Author

#645

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

No branches or pull requests

2 participants