1.0.0-rc.1 connection pool get exhausted using pool's query method

[dolphin278]

I know this is bad bug report, since I am currently not able to provide reproducible test, but anyway:

• On 1.0.0-rc.1, using Pool.query method, you eat all connections and they are not released to pool (callback is never invoked and connection is not released)
• On 0.15.8, Pool.query works just fine, pool is not getting exhausted
• On 1.0.0-rc.1, Pool.execute works properly, connections are successfully returning to pool

Node v5.6.0. I'll try to prepare working example for debugging, but I am in a bit of a rush here, so I don't know exactly when.

[sidorares]

thanks for report @dolphin278 !

There was a number of changes in Pool.query implementation to allow to access query command, might be a bug there. example would be very helpful!

[sidorares]

hi @dolphin278 - any update on this?

[dolphin278]

@sidorares I've got narrowed it down – when Pool.query used in two argument form – sql and cb only, callback freeing pool connection (that is passed to conn.query) is never called. Moreover, even if it will be called, it will work incorrectly – cb argument here is always a third argument regardless, whether there was two- or three-argument invocation (i.e. actual cb went to values).

I've put some clumsy ad-hoc solution for my own use, which is not 100% compatible with your original code (it does not return cmdQuery object):

function (sql, values, cb) {
    if (typeof values === 'function') {
      cb = values;
      values = [];
    }
    this.getConnection(function (err, conn) {
      if (err) return cb(err);
      var cmdQuery = Connection.createQuery(sql, values, function () {
        conn.release();
        if (typeof cb === 'function') return cb.apply(this, arguments);
      });
      conn._resolveNamedPlaceholders(cmdQuery);
      var rawSql = conn.format(cmdQuery.sql, cmdQuery.values || []);
      cmdQuery.sql = rawSql;
      conn.query(cmdQuery);
    });
  };

[sidorares]

oh. Yes, createQuery parameters are incorrect

Corresponding code from node-mysql https://github.com/felixge/node-mysql/blob/1720920f7afc660d37430c35c7128b20f77735e3/lib/Pool.js#L184-L212

with few modifications should be possible to do in a similar way

Pool.prototype.query = function (sql, values, cb) {
  var cmdQuery = Connection.createQuery(sql, values);
  this.getConnection(function (err, conn) {
    if (err) {
       cmd.on('error', function() {});
       cmd.onResult(err);
       return;
    }
    conn._resolveNamedPlaceholders(cmdQuery);
    var rawSql = conn.format(cmdQuery.sql, cmdQuery.values || []);
    cmdQuery.sql = rawSql;
    conn.query(cmdQuery).once('end', function() {
       conn.release();
    })
  });
  return cmdQuery;
};

would you be able to test this and submit pr?

[dolphin278]

@sidorares I am so sorry for the delay, I will get back to this ASAP (hopefully, this weekend)

[sidorares]

Thanks @dolphin278 ! If you happen to work on this please have a look at linked issues as well. I'm afraid I wont have time for next week or two

[bitcloud]

@dolphin278 could you recheck with current master?

[felixfbecker]

I have the same issue, always using the query(options, cb) syntax (because of https://www.npmjs.com/package/sql-template-strings). I can make one request to the API (which will do a lot of queries), then on the second it will just hang. It totally seems like it uses the maximum connections on the first request without freeing and on the second there are no connections left, so it waits forever to get a connection.

[sidorares]

@felixfbecker can you check it's "full pool" problem by logging pool._freeConnections.length, pool._allConnections.length and pool._connectionQueue.length ? you'll see _connectionQueue is growing and _allConnections at constant maximum level if connections never relaesed

[dolphin278]

Finally, I got back and tried to reproduce original issue on 1.1.1. Can not reproduce it anymore. So issue looks like a resolved one.

Got some other weird stuff, like not releasing connection back to pool after executing pool.executeAsync (we use bluebird's promisification on pool, while current Promise-based api hides useful internal state vars like queue length you mentioned in this thread), but this problem is for another issue.

[dolphin278]

Got some other weird stuff, like not releasing connection back to pool after executing pool.executeAsync (we use bluebird's promisification on pool, while current Promise-based api hides useful internal state vars like queue length you mentioned in this thread), but this problem is for another issue.

Partly – it my own error – I called promisified .executeAsync without array of args, just sql, and among execution values argument that finally arrived at Connection.prototype.execute had value of:

function (err, value) {
        if (promise === null) return;
        if (err) {
            var wrapped = wrapAsOperationalError(maybeWrapAsError(err));
            promise._attachExtraTrace(wrapped);
            promise._reject(wrapped);
        } else if (!multiArgs) {
            promise._fulfill(value);
        } else {
            var $_len = arguments.length;var args = new Array(Math.max($_len - 1, 0)); for(var $_i = 1; $_i < $_len; ++$_i) {args[$_i - 1] = arguments[$_i];};
            promise._fulfill(args);
        }
        promise = null;
    } cb: function () {
      console.log('pool:execute:cb');
      conn.release();
      cb.apply(this, arguments);
    }

But what interesting, calling .queryAsync using same args instead of .executeAsync has not give me any problems, so it's a bug, after all.

[sidorares]

so both queryAsync and executeAsync` come from bluebird promisification? Could you post a very simple self-contained test to reproduce issue?

[DJ-DJL]

I am getting this same issue with the execute method on a connection pool (when called with no parameters the connections are not released).
I traced it to this function in pool.js

    execute: function (sql, args) {
      return new Promise(function (resolve, reject) {
        var done = makeDoneCb(resolve, reject);
        if (args) {
          corePool.execute(sql, args, done);
        } else {
            corePool.execute(sql, {}, done);
        }
      });
    },

mysql2 version: 1.1.1
changing line 105 to this fixes the issue for me:
corePool.execute(sql, {}, done);
I think a similar fix may also be needed on line 57?

((This is my first comment on github so please accept my apology if I did something wrong or missed something out))

[felixfbecker]

I am also still getting this issue. I don't have to time to investigate much, but it only happens when using the methods on the pool directly. I worked around it by first getting a connection with getConnection().