node-mysql2 icon indicating copy to clipboard operation
node-mysql2 copied to clipboard

Pool not releasing connections if maxIdle is not set

Open danj1980 opened this issue 1 year ago β€’ 18 comments

All of my queries include dbConnection.release(), however the connections do not get released and once connectionLimit is reached, the execution pauses waiting for an available connection.

I've re-worked my code so that every pool.getConnection() has a dbConnection.release() right next to it, to make absolutely certain that I'm not accidentally leaving any connections unreleased.

However, this did not help, and it seems that there is a bug in the releasing of pool connections.

I store my pool in a static variable with a class called Global.

class globals
{
	static dbPool = null;
}

Runs once on startup

		globals.dbPool = await mysql.createPool({
				host: databaseServer,
				port: databasePort,
				user: databaseUsername,
				password: databasePassword,
				database: databaseName,
				connectionLimit: 10
			});

Example usage

        const dbConnection = await globals.dbPool.getConnection();

        let mysqlResponse = null;

        await dbConnection.query(`SELECT * FROM guild;`).then(result => {
            mysqlResponse = result[0];

        }).catch(err => console.log(err));

        dbConnection.release();
Result after some queries have been executed 
![image](https://github.com/sidorares/node-mysql2/assets/12071228/17c7aca3-567e-4202-8ebe-b202fcba6c5d)

danj1980 avatar Mar 12 '24 16:03 danj1980

image

danj1980 avatar Mar 12 '24 16:03 danj1980

Hi @danj1980, πŸ™‹πŸ»β€β™‚οΈ

It's not directly related, but can you check if this PR affects this behavior in some way?

  • https://github.com/sidorares/node-mysql2/pull/2171

Also, this workaround:

  • https://github.com/sidorares/node-mysql2/issues/2317#issuecomment-1859841319

Possibly related

  • #2317

wellwelwel avatar Mar 12 '24 16:03 wellwelwel

It seems I've found the issue.

If maxIdle is not set on the pool, _removeIdleTimeoutConnections is never executed, and therefore there is never any cleanup of released connections. I think it's this line that's the problem. https://github.com/sidorares/node-mysql2/blob/b87ee4456556712cb8b0538f2f264d9a9fe5764a/lib/pool.js#L30

If maxIdle is set, the released connections get cleaned up.

danj1980 avatar Mar 12 '24 17:03 danj1980

Side note, if I manually kill a connection on MySQL server, execution continues in the pool until the connections fill back up again.

danj1980 avatar Mar 12 '24 17:03 danj1980

Hi @danj1980, πŸ™‹πŸ»β€β™‚οΈ

It's not directly related, but can you check if this PR affects this behavior in some way?

I dont think that's the same issue, as the CPU usage is 0% when the connections run out.

Also, this workaround:

This workaround looks similar to my test where I'm getting a connection, querying it, and then immediately releasing it.

danj1980 avatar Mar 12 '24 17:03 danj1980

This workaround looks similar to my test where I'm getting a connection, querying it, and then immediately releasing it.

@danj1980, can you provide a minimal repro for this issue?


According to the documentation, that should be the default behavior for maxIdle:

{
  connectionLimit: 10,
  maxIdle: 10, // max idle connections, the default value is the same as `connectionLimit`
}

wellwelwel avatar Mar 12 '24 17:03 wellwelwel

I'm sorry. Let me clarify my fix....

In order to get it to cleanup old released connections, I had to set maxIdle < connectionLimit. Without this, my connections maxed out for no reason, and I ended up with 10 idle connections, and no further queries would execute.

If maxIdle is default (eg. maxIdle => connectionLimit), no cleanup of old released connections occurs as shown below.

https://github.com/sidorares/node-mysql2/blob/b87ee4456556712cb8b0538f2f264d9a9fe5764a/lib/pool.js#L30

Unfortunately, I cannot create a demo repo. I've never posted code to github and wouldn't know where to start.

danj1980 avatar Mar 12 '24 17:03 danj1980

There is also a factor that I've not quite figured out. Sometimes connections are reused. Sometimes they aren't. It's when they aren't, and new connections are created that the problem starts occurring.

I would guess that the connection's idleTimeout is reached, that connection is marked as "old" and pending releasing, and therefore a new connection is created, but the old ones aren't cleaned up.

danj1980 avatar Mar 12 '24 17:03 danj1980

So, currently there are these tests for idle connections:

https://github.com/sidorares/node-mysql2/blob/0ce7081e5028c82d861d967ba730b28c7e7bada6/test/integration/test-pool-release-idle-connection.test.cjs#L6-L45

But unfortunately, I can't reproduce this behavior to implement a specific test case for it πŸ˜•

wellwelwel avatar Mar 12 '24 17:03 wellwelwel

In that test, Try removing maxIdle so it's default. Then run the test, pause in the middle for at least 5 seconds to allow idleTimeout to be reached, and then see if it releases the connections that have reached the idleTimeout. Also see if you can create a 6th connection after the idleTimeout has been reached on the first 5 connections. It should allow a 6th connection if the first 5 exceed their idleTimeout. I'd try it myself but I dont know how to run those tests.

danj1980 avatar Mar 12 '24 21:03 danj1980

Thanks, @danj1980 πŸš€

I'll show how I perform the tests locally using Docker, but you can also see the Contributing.md.


Choose a directory

For this example, I'll use the Desktop

mkdir -p Desktop/sidorares
cd Desktop/sidorares

Clone this repository

git clone https://github.com/sidorares/node-mysql2.git

Create a Docker Compose file

Desktop/sidorares/docker-compose.yml:

version: '3.9'
services:
  db:
    image: mysql:latest
    restart: always
    environment:
      MYSQL_DATABASE: 'test'
      MYSQL_ALLOW_EMPTY_PASSWORD: 'yes'
    ports:
      - '127.0.0.1:3306:3306'
    healthcheck:
      test: ['CMD', 'mysqladmin', 'ping', '-h', 'localhost', '-u', 'root']
      interval: 10s
      timeout: 5s
      retries: 5
      
  node-mysql2:
    image: node:lts-alpine
    working_dir: /usr/app
    volumes:
      - ./node-mysql2:/usr/app
    command: npm run test
    environment:
      MYSQL_HOST: db
      FILTER: test-pool-release-idle-connection
    depends_on:
      db:
        condition: service_healthy

Then, edit the Desktop/sidorares/node-mysql2/test/integration/test-pool-release-idle-connection.test.cjs to include your test case.

Finally, test it:

docker compose up

Then, press Ctrl + C to exit.

As it should be a case not covered, it's expected to exit with code 1 (error).

wellwelwel avatar Mar 12 '24 21:03 wellwelwel

Usually, this way (above) is when we want to submit a PR (which are always welcome).

But if you prefer to show just a minimal code to reproduce this error, we can try that too πŸ™‹πŸ»β€β™‚οΈ

wellwelwel avatar Mar 12 '24 21:03 wellwelwel

This happens to me too. I'm having exactly the same problem. Here is my codes;

db.js

import getConfig from "next/config";
import mysql from "mysql2/promise";

const { serverRuntimeConfig } = getConfig();
const { host, port, user, password, database } = serverRuntimeConfig.dbConfig;

const pool = mysql.createPool({
  host,
  port,
  user,
  password,
  database,
  timezone: "+00:00",
  waitForConnections: true,
  enableKeepAlive: true,
  connectionLimit: 10,
  keepAliveInitialDelay: 0,
  queueLimit: 0,
});

export { pool };

users-repo.js

export const usersRepo = {
  ...
  getAllUsers,
  SomeFunc,
};

async function getAllUsers(data) {
  const db = await pool.getConnection();
  try {
    const tableName = data.companyCodeShort + "_users";
    const [allusers] = await db.query("SELECT * FROM ?? WHERE rank != '-1'", [
      tableName,
    ]);

    allusers.forEach((a) => {
      delete a.hash;
      delete a.id;
    });

    return allusers;
  } finally {
    db.release();
  }
}

async function SomeFunc(data) {
  const db = await pool.getConnection();
  try {
    const [users] = await db.query("SHOW STATUS LIKE '%Threads_%'");
    return users;
  } finally {
    db.release();
  }
}

SomeFunc output before getAllUsers function

[
  { Variable_name: 'Threads_cached', Value: '0' },
  { Variable_name: 'Threads_connected', Value: '4' },
  { Variable_name: 'Threads_created', Value: '39' },
  { Variable_name: 'Threads_running', Value: '1' }
]

SomeFunc output after getAllUsers function

[
  { Variable_name: 'Threads_cached', Value: '0' },
  { Variable_name: 'Threads_connected', Value: '5' },
  { Variable_name: 'Threads_created', Value: '39' },
  { Variable_name: 'Threads_running', Value: '1' }
]

SomeFunc output after some time has passed

[
  { Variable_name: 'Threads_cached', Value: '0' },
  { Variable_name: 'Threads_connected', Value: '5' },
  { Variable_name: 'Threads_created', Value: '39' },
  { Variable_name: 'Threads_running', Value: '1' }
]

Kepron avatar Mar 16 '24 01:03 Kepron

Any solution?

Kepron avatar Mar 17 '24 17:03 Kepron