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: do not close delegate rs in callback runnable #425
fix: do not close delegate rs in callback runnable #425
Conversation
Codecov Report
@@ Coverage Diff @@
## master #425 +/- ##
=========================================
Coverage 82.16% 82.16%
Complexity 2455 2455
=========================================
Files 136 136
Lines 13589 13589
Branches 1307 1307
=========================================
Hits 11166 11166
Misses 1895 1895
Partials 528 528
Continue to review full report at Codecov.
|
I would like to understand a bit more about this before approving. Could you point me to where the infinite loop would be? |
Yeah, I should have included that in the initial commit. The problem is as follows:
This problem was introduced by fb26abe#diff-df5cefcf33d6f54a3dbfc8ac52ee50f3. Before that change, the loop would also finish if the |
Thanks for the explanation @olavloite, I think I understand it a bit better now. |
Maybe unrelated to this PR, but shouldn't we be updating the |
No, I would say that that would be superfluous. It has always been updated almost directly before that:
The worst that could happen is that one extra row is fetched and buffered. The |
This is an auto-generated regeneration of the .pb.go files by cloud.google.com/go/internal/gapicgen. Once this PR is submitted, genbot will update the corresponding PR to depend on the newer version of go-genproto, and assign reviewers. Whilst this or any regen PR is open in go-genproto, genbot will not create any more regeneration PRs. If all regen PRs are closed, gapicgen will create a new set of regeneration PRs once per night. If you have been assigned to review this PR, please: - Ensure that CI is passing. If it's failing, it requires your manual attention. - Approve and submit this PR if you believe it's ready to ship. That will prompt genbot to assign reviewers to the google-cloud-go PR. Corresponding google-cloud-go PR: googleapis/google-cloud-go#2695
AsyncResultSetImpl
should not close its delegateResultSet
in theCallbackRunnable
, but should leave that to theProduceRowsCallable
and instead only set the flagcursorReturnedDoneOrException
. Otherwise, the main loop that fetches and produces rows could get stuck in an infinite loop.