{"action":"created","issue":{"url":"https://api.github.com/repos/grpc/grpc-go/issues/674","repository_url":"https://api.github.com/repos/grpc/grpc-go","labels_url":"https://api.github.com/repos/grpc/grpc-go/issues/674/labels{/name}","comments_url":"https://api.github.com/repos/grpc/grpc-go/issues/674/comments","events_url":"https://api.github.com/repos/grpc/grpc-go/issues/674/events","html_url":"https://github.com/grpc/grpc-go/issues/674","id":154317473,"number":674,"title":"Stream Quota Leak following SettingMaxConcurrentStreams","user":{"login":"stxmendez","id":10625379,"avatar_url":"https://avatars.githubusercontent.com/u/10625379?v=3","gravatar_id":"","url":"https://api.github.com/users/stxmendez","html_url":"https://github.com/stxmendez","followers_url":"https://api.github.com/users/stxmendez/followers","following_url":"https://api.github.com/users/stxmendez/following{/other_user}","gists_url":"https://api.github.com/users/stxmendez/gists{/gist_id}","starred_url":"https://api.github.com/users/stxmendez/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/stxmendez/subscriptions","organizations_url":"https://api.github.com/users/stxmendez/orgs","repos_url":"https://api.github.com/users/stxmendez/repos","events_url":"https://api.github.com/users/stxmendez/events{/privacy}","received_events_url":"https://api.github.com/users/stxmendez/received_events","type":"User","site_admin":false},"labels":[],"state":"closed","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":4,"created_at":"2016-05-11T19:01:21Z","updated_at":"2016-07-08T16:53:17Z","closed_at":"2016-05-12T18:48:33Z","body":"I'm happy to submit a patch if the following explanation holds.\r\n\r\n## Background\r\nWe’ve experienced issues where gRPC clients would seem to hang after some amount of invocations have been made against the Go Bigtable API.  The problem seems to randomly occur in about 1:3 runs.  If you add logging you can make the problem go away.\r\n\r\nWe have been able to reproduce the issue via the following horrible code [snippet](https://gist.github.com/akhenakh/adb03aa05ac1a6e85611422128d2a004) and running it with 2k-3k routines.  The code creates a WithTimeout context off of the Background context.  It then proceeds to cancel the WithTimeout context and then use it in invocations to the BT API.  As expected, the BT API calls fail due to the canceled context.  However, if you try to make one more call against BT using the parent context of the WithTimeout context the call hangs on line https://github.com/grpc/grpc-go/blob/master/transport/http2_client.go#L282.\r\n\r\nIn this hang, the code is waiting for stream quota that will never become available.  The system will sit there for about 4 minutes until the client transport is torn down which causes the BT API invocation to conclude with an error.\r\n\r\n## Debugging\r\nAs part of tracking this issue down, logging statements were added at several locations.  It was observed that adding log lines at specific points seemed to decrease or eliminate the likelihood of occurrence.  Regardless, every time the hang occurred, the goroutine invoking BT API was stuck on https://github.com/grpc/grpc-go/blob/master/transport/http2_client.go#L282.\r\n\r\nFurther inspection revealed that when the test starts to run the client transport is running without any restrictions to max concurrent streams.  As the code makes more and more concurrent requests the system eventually receives a request to use http2.SettingMaxConcurrentStreams setting of 100 (https://tools.ietf.org/html/rfc7540#page-22).  This places the client code into a quota driven mode which attempts to limit the max concurrent streams in use to that new value by leveraging a channel to pass the current quota value back and forth.\r\n\r\nWhen the code is running in this modality, it invokes a wait method https://github.com/grpc/grpc-go/blob/master/transport/transport.go#L499 which receives a channel (for quota among other reasons) which it monitors in a select statement.  Per the [Go Language Select Definition](https://golang.org/ref/spec#Select_statements) if multiple channels are ready, the channel is: “chosen via a uniform pseudo-random selection”.\r\n\r\nSo if you have a context that is canceled AND you have quota available, you may consume quota OR detect cancellation in a pseudo random manner.  So you could get quota via https://github.com/grpc/grpc-go/blob/master/transport/http2_client.go#L282 yet fail to release it because https://github.com/grpc/grpc-go/blob/master/transport/http2_client.go#L291 detects the canceled stream.  \r\n\r\n## Possible Workarounds\r\nPart of the problem here is that the comment on line https://github.com/grpc/grpc-go/blob/master/transport/http2_client.go#L292 isn’t true since not all callers of NewStream will result in a call to CloseStream (https://github.com/grpc/grpc-go/blob/master/transport/http2_client.go#L390) in all cases.\r\n\r\nA hacky solution that avoids any refactoring is to update https://github.com/grpc/grpc-go/blob/master/transport/http2_client.go#L292 to release the quota if it erroneously received any in https://github.com/grpc/grpc-go/blob/master/transport/http2_client.go#L282.  This appears to address our issue, but it seems that the root is an assumption around how wait works with respect to precedence.\r\n"},"comment":{"url":"https://api.github.com/repos/grpc/grpc-go/issues/comments/231412560","html_url":"https://github.com/grpc/grpc-go/issues/674#issuecomment-231412560","issue_url":"https://api.github.com/repos/grpc/grpc-go/issues/674","id":231412560,"user":{"login":"stxmendez","id":10625379,"avatar_url":"https://avatars.githubusercontent.com/u/10625379?v=3","gravatar_id":"","url":"https://api.github.com/users/stxmendez","html_url":"https://github.com/stxmendez","followers_url":"https://api.github.com/users/stxmendez/followers","following_url":"https://api.github.com/users/stxmendez/following{/other_user}","gists_url":"https://api.github.com/users/stxmendez/gists{/gist_id}","starred_url":"https://api.github.com/users/stxmendez/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/stxmendez/subscriptions","organizations_url":"https://api.github.com/users/stxmendez/orgs","repos_url":"https://api.github.com/users/stxmendez/repos","events_url":"https://api.github.com/users/stxmendez/events{/privacy}","received_events_url":"https://api.github.com/users/stxmendez/received_events","type":"User","site_admin":false},"created_at":"2016-07-08T16:53:17Z","updated_at":"2016-07-08T16:53:17Z","body":"@ianrose14 it addressed the issue that I had at the time.  We have not picked up an update in a couple of months.  I'm in the process of updating to bigtable v2 and latest grpc and can test this again."}}
{"actor":{"display_login":"stxmendez"}}