mariadb-corporation / mariadb-connector-nodejs

MariaDB Connector/Node.js is used to connect applications developed on Node.js to MariaDB and MySQL databases. MariaDB Connector/Node.js is LGPL licensed.
GNU Lesser General Public License v2.1
363 stars 93 forks source link

Fix prepared close and cache access #280

Closed mscdex closed 3 months ago

mscdex commented 3 months ago

Closing a prepared statement does not result in a server response, so prior to these changes prepared.close() would end up blocking any further enqueued commands on the connection because it was adding to the receive queue.

Additionally there was a bug when resetting a connection and the prepared statement cache was not being used.

mscdex commented 3 months ago

/cc @rusher Thoughts?

rusher commented 3 months ago

Good point about the issue : if (conn.prepareCache) conn.prepareCache.reset();

About the part "blocking any further enqueued commands", there is normally no problem: in https://github.com/mariadb-corporation/mariadb-connector-nodejs/blob/master/lib/cmd/close-prepare.js#L33, event 'send_end' and 'end' are emitted, so further commands will be sent without problem, and on the reading part, command without onPacketReceive methods will be removed from queue : https://github.com/mariadb-corporation/mariadb-connector-nodejs/blob/master/lib/io/packet-input-stream.js#L97. Have you been facing some issue with current implementation ?

Still, this make more sense to directly avoid queuing command that doesn't have need response like you indicate. I would tend to think having the information on command object would make more sense, in place of having parameter

mscdex commented 3 months ago

Here's an example tested against 3.3.0 that reproduces the prepared close blocking further commands:

'use strict';

const mariadb = require('mariadb');
const dbPool = mariadb.createPool({
  // ...
  connectionLimit: 1,
  permitLocalInfile: true,
  prepareCacheLength: 0,
});

(async () => {
  for (let i = 0; i < 2; ++i) {
    let db = await dbPool.getConnection();
    console.log('preparing');
    {
      const prepared = await db.prepare('SELECT ?');
      await prepared.execute(['hello']);
      prepared.close();
    }
    console.log('before select');
    await db.query('SELECT 1');
    console.log('after select');
    await db.release();
  }
  console.log('done');
})();

/*
  Output:
    preparing
    before select
*/
rusher commented 3 months ago

yep, this correspond to the issue you correct with

if (conn.prepareCache)
  conn.prepareCache.reset();