{"action":"created","issue":{"url":"https://api.github.com/repos/vitessio/vitess/issues/3719","repository_url":"https://api.github.com/repos/vitessio/vitess","labels_url":"https://api.github.com/repos/vitessio/vitess/issues/3719/labels{/name}","comments_url":"https://api.github.com/repos/vitessio/vitess/issues/3719/comments","events_url":"https://api.github.com/repos/vitessio/vitess/issues/3719/events","html_url":"https://github.com/vitessio/vitess/pull/3719","id":302107852,"number":3719,"title":"Queryserver conn pool timeout take 2","user":{"login":"demmer","id":3308504,"avatar_url":"https://avatars3.githubusercontent.com/u/3308504?v=4","gravatar_id":"","url":"https://api.github.com/users/demmer","html_url":"https://github.com/demmer","followers_url":"https://api.github.com/users/demmer/followers","following_url":"https://api.github.com/users/demmer/following{/other_user}","gists_url":"https://api.github.com/users/demmer/gists{/gist_id}","starred_url":"https://api.github.com/users/demmer/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/demmer/subscriptions","organizations_url":"https://api.github.com/users/demmer/orgs","repos_url":"https://api.github.com/users/demmer/repos","events_url":"https://api.github.com/users/demmer/events{/privacy}","received_events_url":"https://api.github.com/users/demmer/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":4,"created_at":"2018-03-04T18:16:48Z","updated_at":"2018-03-05T19:16:03Z","closed_at":null,"author_association":"COLLABORATOR","pull_request":{"url":"https://api.github.com/repos/vitessio/vitess/pulls/3719","html_url":"https://github.com/vitessio/vitess/pull/3719","diff_url":"https://github.com/vitessio/vitess/pull/3719.diff","patch_url":"https://github.com/vitessio/vitess/pull/3719.patch"},"body":"As it turns out, PR #3717 wasn't sufficient to actually implement the desired functionality for a couple reasons:\r\n\r\n1. Due to some refactoring I had done during that change, I accidentally omitted the one line needed to actually set QueryEngine.connPoolTimeout from the tablet server Config. This wasn't caught from the unit tests because the tests use a hook to override the configured timeout.\r\n\r\n2. I missed some calls to conns.Get due to the fact that some (but not all) QueryExecutor functions used a `getConn` wrapper that took the pool as a parameter so it could be reused for stream connections as well. To fix this I refactored the call sites so that all of them should now use the timeout if configured.\r\n\r\nThis PR fixes both of  those and _should_ implement the desired timeout functionality for all queries.\r\n\r\nNote that to test this I also refactored the mysql query code so that the sending of a query and blocking to get the result are exposed as separate public functions.\r\n"},"comment":{"url":"https://api.github.com/repos/vitessio/vitess/issues/comments/370530742","html_url":"https://github.com/vitessio/vitess/pull/3719#issuecomment-370530742","issue_url":"https://api.github.com/repos/vitessio/vitess/issues/3719","id":370530742,"user":{"login":"alainjobart","id":4625714,"avatar_url":"https://avatars3.githubusercontent.com/u/4625714?v=4","gravatar_id":"","url":"https://api.github.com/users/alainjobart","html_url":"https://github.com/alainjobart","followers_url":"https://api.github.com/users/alainjobart/followers","following_url":"https://api.github.com/users/alainjobart/following{/other_user}","gists_url":"https://api.github.com/users/alainjobart/gists{/gist_id}","starred_url":"https://api.github.com/users/alainjobart/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/alainjobart/subscriptions","organizations_url":"https://api.github.com/users/alainjobart/orgs","repos_url":"https://api.github.com/users/alainjobart/repos","events_url":"https://api.github.com/users/alainjobart/events{/privacy}","received_events_url":"https://api.github.com/users/alainjobart/received_events","type":"User","site_admin":false},"created_at":"2018-03-05T19:16:03Z","updated_at":"2018-03-05T19:16:03Z","author_association":"CONTRIBUTOR","body":"Yes I agree the last part should be split up. Let me know when you do the split up, and I'll review both this change and the split change."}}
{"actor":{"display_login":"alainjobart"}}