pool.query fails after calling pool.end

[HQidea]

Sometimes, I need to close the old pool each 5 mins and create a new one. So I can't make sure pool.end is called after all queries end. Since it is said

The end method takes an optional callback that you can use to know once all the connections have ended. The connections end gracefully, so all pending queries will still complete and the time to end the pool will vary.

in the doc, I think I can just call pool.end any time I want, but I will occasionally get an error:

Error: Pool is closed.

So,
What does pending queries mean in the doc?
Is a query which have been called pool.query a pending query?

For example,

var mysql = require('mysql');
var pool  = mysql.createPool({
  connectionLimit : 10,
  host            : 'example.org',
  user            : 'bob',
  password        : 'secret',
  database        : 'my_db'
});

pool.query('SELECT 1 + 1 AS solution', function (error, results, fields) {
  if (error) throw error;
  console.log('The solution is: ', results[0].solution);
});
pool.end();  // will be called somewhere else, but will raise the same error here

I've found this commit 76de66e fixed pool.getConnection race conditions, but I don't quite understand what are the race conditions here. In my opinion, after I have called pool.query, this query is a pending query no matter what phase the operation is under the hood such as ping.

[elemount]

Hi @HQidea , the pending query is the query which has been dispatched on a free connection. Not all the queries which called pool.query.

That is why your callback functions throw the error because the new queries can not find a connection for the pool is closed.

[elemount]

@HQidea , could you close this issue?

[HQidea]

Hi @elemount, does this count as an official response?

[elemount]

Sorry, I'm not sure what's your problem? Could you clarify it?

[HQidea]

To a user, there isn't any document contains the definition of pending queries. And since the pool will end gracefully, raising an error from previous query doesn't act as ending gracefully.

[sidorares]

raising an error from previous query doesn't act as ending gracefully.

"Error: Pool is closed." is not an error from one of the queries, it's "you are trying to close pool that is already closed" type of error. When you call .end() you should somehow remove your pool from 'active' state. For example:

function refreshPool() {
   const poolToClose = pool;
   pool = null;
   if (poolToClose) {
     poolToClose.end();
   }
   pool = mysql.createPool(config);
}

[HQidea]

Hi @sidorares, I don't think the error is from close pool that is already closed.

I have similar mechanism as yours, but the error is still there because pool.query has already been called.

After pool.query is called, the pool will get a free connection or create a new one, either way is asynchronous.

If pool.end is called at the time after calling pool.query but before running pool. getConnection's callback will cause this error.

mysql/lib/Pool.js

Lines 190 to 218 in e8fea70

	Pool.prototype.query = function (sql, values, cb) {
	var query = Connection.createQuery(sql, values, cb);
	if (!(typeof sql === 'object' && 'typeCast' in sql)) {
	query.typeCast = this.config.connectionConfig.typeCast;
	}
	if (this.config.connectionConfig.trace) {
	// Long stack trace support
	query._callSite = new Error();
	}
	this.getConnection(function (err, conn) {
	if (err) {
	query.on('error', function () {});
	query.end(err);
	return;
	}
	// Release connection based off event
	query.once('end', function() {
	conn.release();
	});
	conn.query(query);
	});
	return query;
	};

[sidorares]

I can see there could potentially be a race (not sure if it's actually happening)

@HQidea would you be able to make simplest possible self contained example to show this problem?

[HQidea]

example:

const mysql = require('mysql');

class Driver {
  constructor() {
    this.pool = null;
  }

  getPool() {
    if (!this.pool) {
      const pool = this.pool = mysql.createPool({
        host: '',
        port: 3306,
        user: '',
        password: '',
        database: ''
      });
      setTimeout(() => {
        this.pool = null;
        pool.end();
      }, 1000);
    }
    return this.pool;
  }

  query(cb) {
    this.getPool().query('show tables', cb);
  }
}

const driver = new Driver();

function call() {
  driver.query((err, data) => {
    if (err) {
      console.log(err);
      return;
    }
    setTimeout(() => {
      call();
    }, Math.random() * 1000);
  });
}
call();

error:

{ Error: Pool is closed.
    at /path/to/code/node_modules/_mysql@2.14.1@mysql/lib/Pool.js:149:17
    at Array.forEach (native)
    at Pool.releaseConnection (/path/to/code/node_modules/_mysql@2.14.1@mysql/lib/Pool.js:148:37)
    at Pool._removeConnection (/path/to/code/node_modules/_mysql@2.14.1@mysql/lib/Pool.js:279:8)
    at Pool._purgeConnection (/path/to/code/node_modules/_mysql@2.14.1@mysql/lib/Pool.js:260:8)
    at Ping.onOperationComplete [as _callback] (/path/to/code/node_modules/_mysql@2.14.1@mysql/lib/Pool.js:101:12)
    at Ping.Sequence.end (/path/to/code/node_modules/_mysql@2.14.1@mysql/lib/protocol/sequences/Sequence.js:88:24)
    at Ping.Sequence.OkPacket (/path/to/code/node_modules/_mysql@2.14.1@mysql/lib/protocol/sequences/Sequence.js:97:8)
    at Protocol._parsePacket (/path/to/code/node_modules/_mysql@2.14.1@mysql/lib/protocol/Protocol.js:279:23)
    at Parser.write (/path/to/code/node_modules/_mysql@2.14.1@mysql/lib/protocol/Parser.js:76:12)
    --------------------
    at Pool.query (/path/to/code/node_modules/_mysql@2.14.1@mysql/lib/Pool.js:199:23)
    at Driver.query (/path/to/code/test.js:26:20)
    at call (/path/to/code/test.js:33:10)
    at Timeout.setTimeout (/path/to/code/test.js:39:7)
    at ontimeout (timers.js:365:14)
    at tryOnTimeout (timers.js:237:5)
    at Timer.listOnTimeout (timers.js:207:5) code: 'POOL_CLOSED' }

It may not raise an error the first time you run, but it will eventually if you run several times.

[sidorares]

Thanks @HQidea , I can reproduce it.

So what's happening is

1. you do pool.query() first.
2. pool is in the state when number of active connections already maxed so inside pool.query() it has to wait for idle connection
3. pool.end() marks pool to a 'closing' state
4. one of connections finishes previous query and becomes idle, pool.query() from 1) gets connection tries to do connectcion.query() and fails because pool is in closed state

[dougwilson]

So, sorry to come late, but looks like there is two issues here: (1) the docs do not describe well the current behavior and (2) you would like to change the current behavior.

I'll write up the docs for (1) and @HQidea you're welcome to make a pull request for (2) to do what you're looking for. The behavior you're seeing isn't a bug, rather it is designed like that, but you're welcome to implement a pull request that adds a new option to implement the behavior you're looking for :) !

[dougwilson]

Ok, I tried to improve the docs. If you have any additional wording suggestions, please feel free to pull request those changes as well :) !