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
The Ready event is missed in RetriableStream. Sublistener::onReady is called but masterListener.onReady() is not. #6817
Comments
@bjxutj since you are saying it's hard to reproduce I am assuming there is no immediate impact or not looking for an urgent fix. If you have any code snippet/example to share that will be good. |
@dapengzhang0 might have some comments... |
I can agree the current implementation ( @bjxutj Is this causing failure for your application or any performance problem? |
@dapengzhang0 Yes, in our test, some times the request was blocked because it didn't receive onReady event. I want to know if we have a solution to avoid this problem? |
@dapengzhang0 I am confusing on |
@bjxutj Are you just doing the normal retry (without hedging)? If that's the case, calling master listener is fare, but it will break hedging more. In the hedging case, we just know that |
No, in fact, I have switched to no-retry stream, because we don't need the
retry mechanism.
When we use this retriable stream, in my test, it didn't retry, just normal
case, maybe it is too fast or too slow, some thread problem to cause
this issue.
Thanks. Regards
ZHANG Dapeng <notifications@github.com> 于2020年6月2日周二 下午2:37写道:
… @dapengzhang0 <https://github.com/dapengzhang0> I am confusing on
Sublistener.onReady(), does it really need to check if
(state.drainedSubstreams.contains(substream)) ? In which case, we should
check this? Can we just pass through the onReady event to the master
listener, they will handle if the stream is cancelled or closed, or retry
...
@bjxutj <https://github.com/bjxutj> Are you just doing the normal retry
(without hedging)? If that's the case, calling master listener is fare, but
it will break hedging more. In the hedging case, we just know that
onReady() has already broken concurrency. So we might be okay with that
in short term. We just want to make sure that will fix your problem.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#6817 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AE57BZ27GYFFZ3UWGPOJNCDRUVBFDANCNFSM4LF2EI4A>
.
|
What version of gRPC-Java are you using?
I am using 1.26.0, and I have tried 1.27.2 and latest code 1.27.3 (compiled by myself).
What is your environment?
Windows 10, with openjdk 11.0.2
What did you expect to see?
I expect to see the onReady is called, then trigger masterListener.onReady().
What did you see instead?
onReady is called, but masterListener.onReady() is not called.
Steps to reproduce the bug
Sorry, it is hard to reproduce, but I can analyze the code to explain.
In drain(Substream) of RetriableStream, the new state was generated when all the entries were runWith the substream. but in the start(), a StartEntry was added, then the stream started after runWith. There is a possibility that the stream has sent the data successfully and call onReady to notify (another thread), but the new state which include the substream in the drainedSubstreams was not generated, then masterListener.onReady() was not called.
The text was updated successfully, but these errors were encountered: