-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
UPSTREAM: google.golang.org/grpc: 1005: fix logical race in flow control #17735
UPSTREAM: google.golang.org/grpc: 1005: fix logical race in flow control #17735
Conversation
@mfojtik you have an extra zero in the PR number: grpc/grpc-go#1005 |
fc0ccf8
to
1b84e0b
Compare
@enj thanks, fixed. |
/cherrypick release-3.6 |
@mfojtik: once the present PR merges, I will cherry-pick it on top of release-3.6 In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/cherrypick release-3.8 @Kargakis does the bot don't recognize multiple cherrypick comments? |
@mfojtik: once the present PR merges, I will cherry-pick it on top of release-3.8 In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
/retest |
/retest |
@smarterclayton do we need/want this? |
/retest |
Yes, I expect us to deliver this to 3.7. Have we had reproducer yet? It’s still happening on west-1 |
No luck on the reproducer, I've tried several different approaches with this tweaking numbers in many different directions and was never able to reproduce it :/ |
One way may be to add a print statement to the connect function that shows
the current quota number and verify if your test suite can cause the race
to happen. Also run with stress -c 8 or whatever in the background.
us-west-1 is still hitting this. This is p0
…On Wed, Dec 20, 2017 at 8:07 AM, Maciej Szulik ***@***.***> wrote:
Yes, I expect us to deliver this to 3.7. Have we had reproducer yet? It’s
still happening on west-1
No luck on the reproducer, I've tried several different approaches with
this
<https://github.com/soltysh/origin-tester/blob/master/cmd/tester/main.go>
tweaking numbers in many different directions and was never able to
reproduce it :/
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#17735 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABG_p5rx9xsRP_uAecM6Tiq66e_Karm-ks5tCQargaJpZM4Q_gq1>
.
|
/retest @smarterclayton if we can't reliably reproduce this in isolated environment, can we ship this and see if it helps? I'm 85% sure the queue is causing the API server being stuck and the tests in CI for this PR did not revealed any regression AFAIK. |
@smarterclayton @derekwaynecarr merge? |
@smarterclayton this should be automatically cherry-picked for 3.8... if not, it means we already have this fix and the grpc bumped. |
Or the alternative #18071? |
Discussed this with @mfojtik and @smarterclayton we're going with this |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mfojtik, soltysh The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
/retest |
/test all [submit-queue is verifying that this PR is safe to merge] |
Automatic merge from submit-queue. |
@openshift-merge-robot: new pull request created: #18270 In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
@openshift-merge-robot: #17735 failed to apply on top of branch "release-3.8":
In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
No description provided.