{"action":"created","issue":{"url":"https://api.github.com/repos/tgriesser/knex/issues/3447","repository_url":"https://api.github.com/repos/tgriesser/knex","labels_url":"https://api.github.com/repos/tgriesser/knex/issues/3447/labels{/name}","comments_url":"https://api.github.com/repos/tgriesser/knex/issues/3447/comments","events_url":"https://api.github.com/repos/tgriesser/knex/issues/3447/events","html_url":"https://github.com/tgriesser/knex/issues/3447","id":496183445,"node_id":"MDU6SXNzdWU0OTYxODM0NDU=","number":3447,"title":"Error: Connection reset after several hours of idle time","user":{"login":"bdahon","id":5084243,"node_id":"MDQ6VXNlcjUwODQyNDM=","avatar_url":"https://avatars0.githubusercontent.com/u/5084243?v=4","gravatar_id":"","url":"https://api.github.com/users/bdahon","html_url":"https://github.com/bdahon","followers_url":"https://api.github.com/users/bdahon/followers","following_url":"https://api.github.com/users/bdahon/following{/other_user}","gists_url":"https://api.github.com/users/bdahon/gists{/gist_id}","starred_url":"https://api.github.com/users/bdahon/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/bdahon/subscriptions","organizations_url":"https://api.github.com/users/bdahon/orgs","repos_url":"https://api.github.com/users/bdahon/repos","events_url":"https://api.github.com/users/bdahon/events{/privacy}","received_events_url":"https://api.github.com/users/bdahon/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":10,"created_at":"2019-09-20T07:01:26Z","updated_at":"2019-09-26T16:13:57Z","closed_at":null,"author_association":"NONE","body":"# Environment\r\n\r\nKnex version: 0.19.0\r\nDatabase + version: Mysql 5.1.73\r\nOS: Debian\r\n\r\n# Knex config\r\n\r\nThis is my knex configuration\r\n\r\n```\r\nknex({\r\n    client: 'mysql',\r\n    connection: {  ...  },\r\n  })\r\n```\r\n\r\n# Bug\r\n\r\nAfter a few hours of idle time in the application, we get a lot of `read ECONNRESET`. I wouldn't mind having this issue, if it was either handled by knex or tarn, but the error bubbles up to our application. This is the stack we get:\r\n\r\n```\r\n Error: read ECONNRESET\r\n  at TCP.onStreamRead (internal/stream_base_commons.js:111:27)\r\n  at Protocol._enqueue (/usr/app/node_modules/mysql/lib/protocol/Protocol.js:144:48)\r\n  at Connection.query (/usr/app/node_modules/mysql/lib/Connection.js:201:25)\r\n  at /usr/app/node_modules/knex/lib/dialects/mysql/index.js:130:18\r\n  at Promise._execute (/usr/app/node_modules/bluebird/js/release/debuggability.js:313:9)\r\n  at Promise._resolveFromExecutor (/usr/app/node_modules/bluebird/js/release/promise.js:488:18)\r\n  at new Promise (/usr/app/node_modules/bluebird/js/release/promise.js:79:10)\r\n  at Client_MySQL._query (/usr/app/node_modules/knex/lib/dialects/mysql/index.js:124:12)\r\n  at Client_MySQL.query (/usr/app/node_modules/knex/lib/client.js:158:17)\r\n  at Runner.query (/usr/app/node_modules/knex/lib/runner.js:135:36)\r\n  at /usr/app/node_modules/knex/lib/runner.js:39:23\r\n  at tryCatcher (/usr/app/node_modules/bluebird/js/release/util.js:16:23)\r\n  at /usr/app/node_modules/bluebird/js/release/using.js:185:26\r\n  at tryCatcher (/usr/app/node_modules/bluebird/js/release/util.js:16:23)\r\n  at Promise._settlePromiseFromHandler (/usr/app/node_modules/bluebird/js/release/promise.js:517:31)\r\n  at Promise._settlePromise (/usr/app/node_modules/bluebird/js/release/promise.js:574:18)\r\n  at Promise._settlePromise0 (/usr/app/node_modules/bluebird/js/release/promise.js:619:10)\r\n\r\n```\r\n\r\nI know there are already a few issues opened (https://github.com/tgriesser/knex/issues/3037 for instance), but as @elhigu sugggested, I opened a new issue. \r\n\r\nApart from this issue, is there any plan for knex to support validating a request in order to keep a healthy pool ?\r\n\r\nIf I understand the code correctly, there is already a `validateConnection` function that gets passed to Tarn.js. However, this seems to always validate the connection, no matter what\r\n\r\nIn client.js, line 302 (on master branch):\r\n```\r\n\r\n  validateConnection(connection) {\r\n    return true;\r\n  },\r\n```\r\n\r\n"},"comment":{"url":"https://api.github.com/repos/tgriesser/knex/issues/comments/535577333","html_url":"https://github.com/tgriesser/knex/issues/3447#issuecomment-535577333","issue_url":"https://api.github.com/repos/tgriesser/knex/issues/3447","id":535577333,"node_id":"MDEyOklzc3VlQ29tbWVudDUzNTU3NzMzMw==","user":{"login":"elhigu","id":696345,"node_id":"MDQ6VXNlcjY5NjM0NQ==","avatar_url":"https://avatars3.githubusercontent.com/u/696345?v=4","gravatar_id":"","url":"https://api.github.com/users/elhigu","html_url":"https://github.com/elhigu","followers_url":"https://api.github.com/users/elhigu/followers","following_url":"https://api.github.com/users/elhigu/following{/other_user}","gists_url":"https://api.github.com/users/elhigu/gists{/gist_id}","starred_url":"https://api.github.com/users/elhigu/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/elhigu/subscriptions","organizations_url":"https://api.github.com/users/elhigu/orgs","repos_url":"https://api.github.com/users/elhigu/repos","events_url":"https://api.github.com/users/elhigu/events{/privacy}","received_events_url":"https://api.github.com/users/elhigu/received_events","type":"User","site_admin":false},"created_at":"2019-09-26T16:13:57Z","updated_at":"2019-09-26T16:13:57Z","author_association":"COLLABORATOR","body":"`propagateCreateError:false` never do that with knex, it will not do anything good for anyone. Knex just start working wrong. Please also inform all sources which is suggesting that it is plainly wrong and does no good. I've been trying to shoot that tip down in many places, but it comes up again and again... 😭\r\n\r\nLooks like you are right about tarn's validate function. I'll change tarn to support async validation. Makes sense to support it.\r\n\r\n> I also agree, that validating the connection should not be done by executing another query before every connection acquisition. Instead I believe Knex should check for such errors after resolving the queryPromise (either https://github.com/tgriesser/knex/blob/master/lib/runner.js#L144 or https://github.com/tgriesser/knex/blob/master/lib/client.js#L158) and when connection reset by peer or similar happens, should try to destroy current connection, acquire new one and retry the query with the new connection.\r\n\r\nI don't thing knex should make that kind of decisions, it should just pass error to client, who will know if query should be retried or not. For example if that happened during transaction that naive approach would fail massively. Knex should have as little smartness in these matters as possible to allow user to decide what to do when unexpected error happens.\r\n\r\nAnyhow bug here is that is some cases connection validation doesn't work and would be good to find out why, which requires some way to reproduce the problem.\r\n\r\n> It also means connection end event is not called by pg in https://github.com/tgriesser/knex/blob/master/lib/dialects/postgres/index.js#L117 after reset from inactivity, otherwise connection would be marked as invalid and filtered out by original Knex validate fucntion. At the time of the query pg probably doesn't yet know the connection errored.\r\n\r\nI don't understand what you mean by this... currently when connection is closed when in pool the exception is recorded and next time when connection is to be fetched from pool it gets discarded and new connection is requested instead. Anyways this issue is not about any postgres problems, this happend with mysql. "}}
{"actor":{"display_login":"elhigu"}}