Pool lost connections for erroneous queries

[lukfol]

I've experienced such error behaviour:

1. Connection pool with 10 connections limit
2. Queries from pool object
3. Problem: we have much more connections above the limit open to MySql server, to the moment MySql limit is reached and we can't do more queries

I've traced the problem, I had error in mysql query but didn't have callback (I don't know if it matters), just sqlConnPool.query("insert …."). On error, _removeFromPool is called in PoolConnection.js - which removes the connection from pool, but doesn't close it…
I think that it should not close the connection, just return it to the pool. Or, at least, the connection should be closed when removed from pool…

[dougwilson]

Hm, I'm not exactly sure what is going on here. Would it be possible to paste in some fully-functional code that we can use to reproduce the issue or even offer up a pull request with a fix for the problem you are experiencing?

[lukfol]

First create local mysql test database names "testdb", than run the code:

var mysql = require('mysql');
var sqldb = mysql.createPool({
    connectionLimit: 5,
    host     : 'localhost',
    database: "testdb",
    user: "root",
    password: ""
});


for(var i=0; i<50; i++)
    sqldb.query("insert into non_existing_table(a_column) values('')");

setInterval(function(){

}, 100);

Than- the code above must be running- enter mysql console, than run:
show processlist

50 new connections are present (despite of the limit of 5)…

What is important- if code above is modified to contain callback like:

    sqldb.query("insert into non_existing_table(a_column) values('')", function(){});

than everything is correct, there are no "leaked" connections… I know there always should be callbacks and error handling :) So the easiest and most compatible with older library way to solve the problem is to provide default function(){} callback in mysql pool query function…

[dougwilson]

Ah, interesting! Yes, connections should not be leaking even if the callback is missing. That you so much for the instructions :)

[dougwilson]

Ok, so I was able to determine the issue. Right now on master half of the issue is fixed, as the error from the invalid SQL should not have removed the connection from the pool in the first place, which is the reason you are going above your connectionLimit.

There is still one more fix, where we need to properly clean up connections removed from the pool.

[dougwilson]

Alright, this should be fixed now. You can wait until the next minor version or you can confirm now by doing npm install felixge/node-mysql and seeing if your issue has been resolved :)

[lukfol]

Now it's ok, I'll update node-mysql in all my projects, since I'm using queries without callbacks sometimes :)
Thanks for your support and all your contribution into node.js!

[dougwilson]

Awesome, thanks for the confirmation! This was actually a pretty important issue and I just published these changes to npm as 2.7.0.