Skip to content
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

Add idleConnectionTimeout to pool options #2218

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

dicearr
Copy link

@dicearr dicearr commented May 10, 2019

This option is specially useful when connecting to serverless databases, in which the instances are turned off when there is no connection alive.

Coming from #2195.

This aims to close #1276 and resolve #962.

@gyj1278
Copy link

gyj1278 commented May 20, 2019

This option is great,and when will it be released to the official edition?@@dougwilson

@dougwilson
Copy link
Member

I am working through this module prs and issues this week.

@@ -32,6 +32,13 @@ PoolConnection.prototype.release = function release() {
return undefined;
}

if (this._pool.config.idleConnectionTimeout) {
this._idleTimeout = setTimeout(
this.destroy.bind(this),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably end the connection gracefully, running the QUIT packet to the server instead of just dropping the TCP connection. Various database setups will block a client that keeps dropping TCP connections without properly closing them.

Use conn.end instead of conn.destroy for closing idle connections
@akleiber
Copy link

@dicearr this would be ❤️ by many cloud users. Any plan to continue this PR?

@dicearr
Copy link
Author

dicearr commented Mar 21, 2020

Hi @akleiber. After the review comment I tried to gracefully close the connection by using conn.end instead of conn.destroy in 6633202, which is a fixup commit. So, I don't think there is too much I can do here. If 6633202 is not what @dougwilson meant, some feedback would be appreciated. If it is, then I just need to squash the commits.

@rolandmarg
Copy link

rolandmarg commented Jan 8, 2021

We use this library with aws aurora and idleConnectionTimeout is really needed. I don't really have time to wait for this ticket to be merged. What do you guys think about attaching a timer on connection 'release' event, which will close connection if it's not acquired after 'x' time, can be done on top of library.
update: Seems like you can't call conection.end() on released connection in 'release' event

@Mwni
Copy link

Mwni commented Jan 13, 2021

This is a temporary solution until this functionality has been merged. Place this right after importing the mysql library.

(() => {
	let createPool = mysql.createPool

	mysql.createPool = function(config){
		let pool = createPool(config)

		if(config.idleConnectionTimeout){
			let getConnection = pool.getConnection

			pool.getConnection = function(callback){
				getConnection.call(pool, (err, connection) => {
					if(err){
						callback(err, connection)
						return
					}

					if(connection.__lastUsed){
						if(Date.now() - connection.__lastUsed > config.idleConnectionTimeout){
							connection.destroy()
							pool.getConnection(callback)
							return
						}
					}

					connection.__lastUsed = Date.now()
					callback(err, connection)
				})
			}
		}

		return pool
	}
})()

This is forward compatible. So what it does is patch the createPool function to check for idleConnectionTimeout, if this setting is present, it patches the pool's getConnection function to destroy any connection, that it was about to serve, that has been been idle for longer than specified.

@qingyang-id
Copy link

https://github.com/mysqljs/mysql/pull/2507/files

Hello, everyone, I commit a pr and to solve this issue, any feedback would be appreciated.

@mysqljs mysqljs deleted a comment from jpike88 Oct 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

Connections in pool have an infinite lifetime It would be good to add some idleTimeout to pool options
7 participants