Extend Pool methods to enable create conns even if there are free ones available

[acorbi]

As indicated on #1229 this PR proposes the extension of the functions in the Pool.js module to add a createConnection method that, compared to getConnection, does not acquire a "free" available connection but creates it. createConnection would then be used within getConnection in case there is no "free" available connections.

NOTE: This PR includes only one new unity test related to the new code. Although my plan is to extend the test suite on this PR, I would like to open it now and ask folks over here to provide input on how it could be improved, better tested and in general whether it would be beneficial to integrate this code change in the library. Please provide feedback 👍

[dougwilson]

I'm not sure I understand exactly why this is necessary? If you wanted to "warm" up the pool, the following code works just fine, no?

function warmPool(pool, callback) {
  var connections = [];

  for (var i = 0; i < pool.config.connectionLimit; i++) {
    pool.getConnection(onConnection);
  }

  function onConnection(err, conn) {
    if (!connections && err) return;
    if (!connections) return conn.release();
    if (err) return releaseAll(err);
    connections.push(conn);

    if (connections.length === pool.config.connectionLimit) {
      releaseAll();
    }
  }

  function releaseAll(err) {
    connections.forEach(function (conn) {
      conn.release();
    });
    connections = null;
    callback(err);
  }
}

[acorbi]

Hi @dougwilson and thanks for your feedback.

Please correct me if I am wrong, but as I understand it, getConnection is going to first check if there are any "free" (created connections that were released and are available on the pool for some thread to acquire them) connections before creating a new one. See https://github.com/felixge/node-mysql/blob/master/lib/Pool.js#L33-L37

Considering this fact, the code you just proposed would try to get pool.config.connectionLimit connections. Some of them could be on the list of "free" connections and therefore, your logic does not guarantee the creation of new ones, just acquire and release the "free" ones available previously.

My code change proposes a way of creating those connections ignoring the "free" ones.

Does it make sense?

[dougwilson]

I understand the purpose, but can you provide the specific use-case it would serve? Perhaps provide the code you are trying to use this function for?

[acorbi]

@dougwilson The use-case is described on #1229 . The goal of the improvement is to reduce the CPU overhead caused by opening a big amount of connections in little time ( traffic peaks).

Here a code snippet showing how I plan to use it:

var connectionDiff = this.config.connectionLimit - this._allConnections.length;
for (var i = 0; i < connectionDiff; i++) {
    pool.createConnection(function(err,conn){
       conn.release();
    });
}

[dougwilson]

Hi @acorbi , unfortunately your use-case is already covered by the code in #1232 (comment) ; it only adds a larger API surface to this module for no gain over what you can already achieve by just using the existing getConnection.

To me, it sounds likely ultimately you want the initialSize feature from #505 , which wouold be much easier for people to use. Can you open a new PR porting at least that feature from PR #505 forward to master instead?

[acorbi]

Hi @dougwilson, I see: On #1232 (comment) you are retaining (not releasing) the connections until all get actually created. Got it!

OK, let me focus on your suggestion of implementing the feature proposed on #505 . It is exactly what I want!

One question though: Can you kindly shed some light on the status of that PR? It seems to me that it tries to add PoolCluster logic, which is now implemented in the library. So, all the code related to PoolCluster is out-dated now?. I would then extract the logic for the minIdle, maxIdle and initialSize Pool options into a new PR. Sounds good?

[dougwilson]

I would then extract the logic for the minIdle, maxIdle and initialSize Pool options into a new PR. Sounds good?

Sounds perfect.