ER_ACCESS_DENIED_ERROR differ behavior to mysql/mysql2

[jimmyolo]

in mysql2, both promise and callback createPool are synchronize.
and throw error earlier when ACCESS_DENIED,
in mariadb need wait util first use and then timeout throw error.

"use strict";
require("make-promises-safe");
const mariadb = require("mariadb/promise");
//const mariadb = require("mysql2/promise");
const pool = mariadb.createPool({
     user:"Jimmy", 
     connectionLimit: 5,
});
async function test() {
  try {
	const rows = await pool.query("SELECT now()");
	console.log(rows);
  } catch (err) {
	throw err;
  } finally {
	if (pool) return pool.end();
  }
}
test();

/* in mysql2

{ Error: Access denied for user 'Jimmy'@'127.0.0.1' (using password: YES)
    at Packet.asError (/home/Projects/test/node_modules/mysql2/lib/packets/packet.js:684:17)
    at ClientHandshake.execute (/home/Projects/test/node_modules/mysql2/lib/commands/command.js:28:26)
    at PoolConnection.handlePacket (/home/Projects/test/node_modules/mysql2/lib/connection.js:455:32)
    at PacketParser.onPacket (/home/Projects/test/node_modules/mysql2/lib/connection.js:73:18)
    at PacketParser.executeStart (/home/Projects/test/node_modules/mysql2/lib/packet_parser.js:75:16)
    at Socket.<anonymous> (/home/Projects/test/node_modules/mysql2/lib/connection.js:80:31)
    at Socket.emit (events.js:189:13)
    at addChunk (_stream_readable.js:288:12)
    at readableAddChunk (_stream_readable.js:269:11)
    at Socket.Readable.push (_stream_readable.js:224:10)
  code: 'ER_ACCESS_DENIED_ERROR',
  errno: 1045,
  sqlState: '28000',
  sqlMessage:
   "Access denied for user 'Jimmy'@'127.0.0.1' (using password: YES)" }
*/

/* in mariadb

{ Error: retrieve connection from pool timeout
    at Object.module.exports.createError (/home/Projects/test/node_modules/mariadb/lib/misc/errors.js:55:10)
    at rejectTimeout (/home/Projects/test/node_modules/mariadb/lib/pool.js:268:16)
    at Timeout.rejectAndResetTimeout [as _onTimeout] (/home/Projects/test/node_modules/mariadb/lib/pool.js:288:5)
    at listOnTimeout (timers.js:326:17)
    at processTimers (timers.js:268:5)
  fatal: false,
  errno: 45028,
  sqlState: 'HY000',
  code: 'ER_GET_CONNECTION_TIMEOUT' }
*/

[rusher]

That's because implementation basis is different :

MariaDB pool implementation completely differs to avoid creating some huge number of connection when having query pike :

To explain :
Example with a pool that is configured to have a maximum of 50 connections. actual connection number is 5.
With a basis of a connection creation taking 1.5ms, and query taking 0.05ms (example on a local server).
Everything is quiet, and then ... Boom! ... 100 queries on the pool at once, wanting a connection.

on mysql2, that would mean the first query will use the 5 idle connections, then waiting to create 45 connections.
So in timeline :
0.05ms = 5 query finished, 5 connections in pool, 45 connections in creation
0.10ms = 10 query finished, 5 connections in pool, 45 connections in creation
0.15ms = 15 query finished, 5 connections in pool, 45 connections in creation
0.20ms = 20 query finished, 5 connections in pool, 45 connections in creation
0.25ms = 25 query finished, 5 connections in pool, 45 connections in creation
0.30ms = 30 query finished, 5 connections in pool, 45 connections in creation
0.35ms = 35 query finished, 5 connections in pool, 45 connections in creation
0.40ms = 40 query finished, 5 connections in pool, 45 connections in creation
0.45ms = 45 query finished, 5 connections in pool, 45 connections in creation
0.50ms = 50 query finished, 5 connections in pool, 45 connections in creation
0.55ms = 55 query finished, 5 connections in pool, 45 connections in creation
0.60ms = 55 query finished, 5 connections in pool, 45 connections in creation
...
1.45ms = 55 query finished, 5 connections in pool, 45 connections in creation
1.50ms = 100 queries finished, 50 connections in pool.

45 queries have waited for connection creation to fulfill, taking longer time, and resulting in more connection in the pool.

In mariadb implementation :
0.05ms = 5 query finished, 5 connections in pool, 1 connection in creation
0.10ms = 10 query finished, 5 connections in pool, 1 connection in creation
0.15ms = 15 query finished, 5 connections in pool, 1 connection in creation
0.20ms = 20 query finished, 5 connections in pool, 1 connection in creation
0.25ms = 25 query finished, 5 connections in pool, 1 connection in creation
0.30ms = 30 query finished, 5 connections in pool, 1 connection in creation
0.35ms = 35 query finished, 5 connections in pool, 1 connection in creation
0.40ms = 40 query finished, 5 connections in pool, 1 connection in creation
0.45ms = 45 query finished, 5 connections in pool, 1 connection in creation
0.50ms = 50 query finished, 5 connections in pool, 1 connection in creation
0.55ms = 55 query finished, 5 connections in pool, 1 connection in creation
0.60ms = 60 query finished, 5 connections in pool, 1 connection in creation
0.65ms = 65 query finished, 5 connections in pool, 1 connection in creation
0.70ms = 70 query finished, 5 connections in pool, 1 connection in creation
0.75ms = 75 query finished, 5 connections in pool, 1 connection in creation
0.80ms = 80 query finished, 5 connections in pool, 1 connection in creation
0.85ms = 85 query finished, 5 connections in pool, 1 connection in creation
0.90ms = 90 query finished, 5 connections in pool, 1 connection in creation
0.95ms = 95 query finished, 5 connections in pool, 1 connection in creation
1.00ms = 100 query finished, 5 connections in pool, 1 connection in creation
...
1.50ms = 100 queries finished, 6 connections in pool.

Queries have taken less time to fulfill (1ms), and the pool doesn't force the creation of connections if not really needed. Of course, if the pike takes longer time, the number of connection will increase.
acquireTimeout really something too.

(And that has been thought to have another option minConnection - not existing for now - for an application that has a period of slow activity, connections number in the pool can decrease if not used with a minimum of minConnection )

Still, if a connection fails because of authentication, that must be logged. I'll think on how to change that

[rusher]

sorry, the answer wasn't finished when send. updated now.
So you're right, there is some behavior to change. failed authentication must clearly be sent back..

[knoxcard]

Does this commit solve the issue presented in this ticket?

900e515

[rusher]

no, this issue is still to be solved. Next week !
But I need some thinking here to avoid changing API from
createPool(options) → Pool to createPool(options) → Promise

[jimmyolo]

and, I see the code, since connection.connect() return Promise, if connect occur error and not await createConnection, node will show Unhandle-promise-warning

[rusher]

corrected with 59fafd1