mysqljs / mysql

A pure node.js JavaScript Client implementing the MySQL protocol.
MIT License
18.28k stars 2.52k forks source link

Using node-mysql with promises #929

Closed chrisveness closed 7 years ago

chrisveness commented 9 years ago

Can node-mysql be used with promises, or only with callbacks? If it can't, can you suggest how best to use it within a promises structure? (I'm a bit lost with promisify, bluebird, Q, ...)

dougwilson commented 9 years ago

This library uses callbacks because they are more performant and also let you do anything. If you want to use promises with callbacks, bluebird I know offers this functionality. Here is an example:

var Promise = require('bluebird')
var mysql = require('mysql')

var connection = mysql.createConnection({ ... })

Promise.promisify(connection.query, connection)('SELECT 1').then(function (rows) {
  console.log('got rows!')
  console.dir(rows)
  connection.end()
})
gajus commented 9 years ago

This library uses callbacks because they are more performant and also let you do anything.

Would be nice to see some benchmarks. The time that promise resolution takes, in the context of roundtrips, is negligible.

dougwilson commented 9 years ago

The only way to truely benchmark it would be to convert at least the external APIs in this module to promises and then make a benchmark suite and use it to compare this module with and without them.

@sidorares may have some thoughts on this as well.

gajus commented 9 years ago

The only way to truely benchmark it would be to convert at least the external APIs in this module to promises and then make a benchmark suite and use it to compare this module with and without them.

The first part has been done, https://www.npmjs.org/package/promise-mysql.

sidorares commented 9 years ago

I've been doing some benchmarks recently, I'll add promise-mysql as well and post results here

dougwilson commented 9 years ago

@sidorares cool :) and @gajus ooo, I didn't know that wrapper module existed (though I don't use promises, so I guess that's understandable).

sidorares commented 9 years ago

I'll post more detailed results later, but rough numbers for simple "select 1" x 100000 times: 7300 qps - node-mysql 6400 qps - promise-mysql 8338 qps - node-mysql2

io.js: 8300 qps - node-mysql 7000 qps - promise-mysql 14200 qps - node-mysql2

So there is some overhead, probably less visible on bigger resultsets (or slower queries). There is a big difference between node ( v0.10.33 ) and io.js - likely due to newer V8 and turbofan.

gajus commented 9 years ago

What library for promises did you use?

On Dec 19, 2014, at 08:07, Andrey Sidorov notifications@github.com wrote:

I'll post more detailed results later, but rough numbers for simple "select 1" x 100000 times: 7300 qps - node-mysql 6400 qps - promise-mysql 8338 qps - node-mysql2

io.js: 8300 qps - node-mysql 7000 qps - promise-mysql 14200 qps - node-mysql2

So there is some overhead, probably less visible on bigger resultsets (or slower queries). There is a big difference between node ( v0.10.33 ) and io.js - likely due to newer V8 and turbofan.

— Reply to this email directly or view it on GitHub.

sidorares commented 9 years ago

https://www.npmjs.org/package/promise-mysql ( bluebird )

gajus commented 9 years ago

It would be interesting to test it using the native (0.11.13) promises, https://twitter.com/jongleberry/status/462260349173907456.

On Fri, Dec 19, 2014 at 11:59 AM, Andrey Sidorov notifications@github.com wrote:

https://www.npmjs.org/package/promise-mysql ( bluebird )

— Reply to this email directly or view it on GitHub https://github.com/felixge/node-mysql/issues/929#issuecomment-67618919.

dougwilson commented 9 years ago

BTW I'm going to reopen this issue while discussion continues.

chrisveness commented 9 years ago

It's been helpful following this discussion I prompted, but in fact I have the luxury of being in the position of planning future development rather than having to go live immediately, and am hoping that (providing Node.js v0.12 appears at some point!) I will be able to skip a generation, pass over promises, and go straight to generators using Koa with something like co-mysql or mysql-co.

Any views or suggestions would be welcome!

gajus commented 9 years ago

What advantage do generators have over promises in this scenario?

chrisveness commented 9 years ago

Using Koa at least, it seems that programs can be written almost as traditional synchronous code, just prefixing would-be blocking calls with 'yield', making for far more readable (hence supportable) code. I find promises avoid 'callback-hell', but still break up code flow in unnatural ways. I believe error handling is also simpler and more robust with Koa. There's a decent illustration here: www.strongloop.com/strongblog/node-js-express-introduction-koa-js-zone.

dougwilson commented 9 years ago

The whole Koa/yield thing is a hack because JavaScript doesn't support stuff. ES7 will give us async and await keywords, which will make promises look exactly like Koa and Koa plans to use promises as soon as this happens.

chrisveness commented 9 years ago

ES6 will give us generators/yield. ES6 is available in Node v0.11, and will be available for production use if Node v0.12 ever arrives. Koa uses generators ("through leveraging generators Koa allows you to ditch callbacks"). No need to wait for ES7.

dougwilson commented 9 years ago

I'm just explaining why Koa uses generators; besides, even the Koa generators (powered by co) actually use promises, so you can yield function_returning_a_promise() just fine.

gajus commented 9 years ago

Have a go at http://gajus.com/blog/2/the-definitive-guide-to-the-javascript-generators to understand better the generators. Generators are not a good candidate for MySQL abstraction.

dougwilson commented 9 years ago

Correct. generators have nothing to do with async programming: they just generate a list of values. The co library hacks generators such that it provides a trampoline. The section about Lisp in https://en.wikipedia.org/wiki/Trampoline_%28computing%29 literally describes what co does and when you put on the other side of yield is a thunk-returning function (co also accepts promises, not only thunks, though, but a promise is sort of line a fancy thunk).

If you look at the source for mysql-co, you'll see it returns thunks.

chrisveness commented 9 years ago

@gajus - that definitive guide is really clear and helpful, and the references at the end make good reading also. But why are generators (as used by Koa) not a good candidate for MySQL abstraction?

petkaantonov commented 9 years ago

https://www.npmjs.org/package/promise-mysql ( bluebird )

FWIW that wrapper uses much more inefficient wrapping than simply using bluebird's promisifyAll - using that properly will give same performance as equal callback code (given you test with real latencies, otherwise it is probably around 10-20% slower). I don't even understand why people make these modules - the bluebird docs provide an example how to promisify the mysql module with literally 2 lines using promisifyAll.

dougwilson commented 9 years ago

I don't even understand why people make these modules - the bluebird docs provide an example how to promisify the mysql module with literally 2 lines

Exactly :) This is one of the reasons I haven't pushed very hard to convert callback-style modules into promises at all, since it's so easy to just wrap them.

gajus commented 9 years ago

Promise-mysql is a wrapper for node-mysql that wraps function calls with Bluebird promises. Usually this would be done with Bluebird's .promisifyAll() method, but node-mysql's footprint is different to that of what Bluebird expects.

https://www.npmjs.com/package/promise-mysql

petkaantonov commented 9 years ago

@gajus the api docs show an example how to promisify it:

Promise.promisifyAll(require("mysql/lib/Connection").prototype);
Promise.promisifyAll(require("mysql/lib/Pool").prototype);

Typically modules can be promisified with Promise.promisifyAll(require("module")) because they expose classes as properties of the main export, but the above isn't too bad either.

gajus commented 9 years ago

Thats true, though promise-mysql also does other things, viz., handle disconnections (https://github.com/lukeb-uk/node-promise-mysql/blob/master/lib/connection.js#L8-L35). I am not advocating the greatness of a specific implementation. I would simply like to see mysql package having a better integration with Promise(s).

dougwilson commented 9 years ago

@gajus the files you referenced, where it tries to re-establish a dropped connection, is something you should never use anyway; session state can become reset any any moment with that kind of code and you'll never know and queries like SELECT LAST_INSERT_ID() will return bizarre results.

petkaantonov commented 9 years ago

@dougwilson btw is exposing the classes as properties of the main export something you would consider for even easier promisification? basically in index.js you would just say exports.Connection = Connection; exports.Pool = Pool; etc

gajus commented 9 years ago

Thats a good example. But it is even worse if a promise that gave an active connection suddenly changes its state, because by convention promise state must not change after it has been set. Therefore, as a user I would prefer whatever magic behind the scenes to ensure that mysql connection is not lost.

My suggestion to the said issue, while it is a workaround, it would prevent session state reset, https://github.com/lukeb-uk/node-promise-mysql/issues/2.

dougwilson commented 9 years ago

@dougwilson btw is exposing the classes as properties of the main export something you would consider for even easier promisification? basically in index.js you would just say exports.Connection = Connection; exports.Pool = Pool; etc

Sorry I took so long; I lost this issue in the large amount. I'm going to make this change for sure.

ghost commented 9 years ago

Just in case someone is wondering how to build an example without promisifyAll (as I couldn't really make it work exactly like I wanted and you know people have to work also...)

This is a ES7-like example of how things would go:


let mysql = require("mysql")

let database_query = function (options, query) {
 return new Promise(function(resolve, reject) {
  let {host, user, password, database} = options
  let conn = mysql.createConnection({
    host     : host,
    user     : user,
    password : password,
    database : database
  })
  conn.connect()
  conn.query(query, (err, data) => (err ? reject(err) : resolve(data)))
  conn.end()
 })
}

let options = {
  host     : process.ENV_HOST || "localhost",
  user     : process.ENV_USER || "root",
  // ...
}

async function () { 
  try {
    let rows = await database_query( options, queryString )
    rows.forEach((row) => row_view( row ))
  } catch (err) {
    handleError( err )
  }
}
AdriVanHoudt commented 9 years ago

@sidorares I'm interested in an update on the benchmarks, do they still hold valid? Also why is node-mysql2 so much faster?

sidorares commented 9 years ago

@AdriVanHoudt mysql2 core is completely different, it compiles row parsers dynamically at runtime + other perf optimisations. I'll try to make new benchmarks against latest versions, but expect similar results

AdriVanHoudt commented 9 years ago

@sidorares cool thanks, are there use cases as to when to use mysql2 and when not to?

sidorares commented 9 years ago

mysql2 pros:

cons:

mysql pros

So probably short answer is "use mysql2 if you really need to have a little bit of extra performance ( and have benchmarks to prove where is bottleneck) or hacking on mysql protocol and tools. In all other cases use mysql"

AdriVanHoudt commented 9 years ago

@sidorares thanks! I was looking for a clear statement like that for a while!

chrisveness commented 9 years ago

I've also found mysql2 works very well with Koa, using mysql-co (following on the original theme of this issue); there's an example in my Koa Sample App. Thanks @sidorares!

sidorares commented 9 years ago

@chrisveness this is not specific to mysql2, mysql-co should probably accept driver instance as a parameter instead of requiring it internally

chrisveness commented 9 years ago

Good point – though works fine for me as is :)

Redsandro commented 8 years ago

@gajus

It would be interesting to test it using the native (0.11.13) promises

@bucaran

Just in case someone is wondering how to build an example without promisifyAll

Using the new keyword is unnecessary overhead and a big no-no in node. Bluebird promises are much more optimized than the native Promise implementation.

For example, you can Promise.resolve('something') and chain from there, never having to use the new keyword.

The author spent a lot of time optimizing things. You can read his own explanation here:

StackExchange: Why are native ES6 promises slower and more memory-intensive than bluebird?

robertleeplummerjr commented 7 years ago

I'm trying to do:

import mysql from 'mysql';
import Promise from 'bluebird';
import { co } from 'bluebird-co';
import config from 'config';

co(function* () {
  const pool = mysql.createPool(config.mysql);
  const connection = yield Promise.promisify(pool.getConnection)();
  const query = Promise.promisify(connection.query);
  query('select * from table');
});

but keep getting an error thrown:

Unhandled rejection TypeError: this._implyConnect is not a function scripts/node

What am I doing wrong?

chrisveness commented 7 years ago

My suggestion would be to use mysql2, which now comes complete with a promise wrapper.

You can then do something like:

const koa = require('koa'); // koa v1
const mysql = require('mysql2/promise');
const router = require('koa-router')();

const app = koa();
const config - require('./config.json');
const pool = mysql.createPool(config.db); // create pool on app startup

app.use(function* mysqlConnection(next) { // get connection for this request
    this.state.db = yield pool.getConnection();
    yield next;
    this.state.db.release();
});

router.get('/', function*(next) {
    const [results] = yield this.state.db.query('select * from table');
    console.log(results);
});

app.use(router.routes());

app.listen(3000);

There is a fuller example on my Koa sample app.

Edit: my use-case is Koa, I realise you don't mention a framework. Nonetheless, if I understand well, you would create a pool on app startup, and get a connection for each request, so your example doesn't really make sense to me. Otherwise, I hope my example clarifies how you could use the mysql2 promise wrapper without needing co.

Redsandro commented 7 years ago

@chrisveness what's your opinion on mysql2 vs mysqljs?

chrisveness commented 7 years ago

@Redsandro I think both are very solid, but I've actually only ever used mysql2 myself.

For me, mysql2 coming with a promise wrapper makes it far more convenient (I love async/await) – that may or may not matter to you. If you are focused on performance, you can use bluebird, or stick to callbacks, though I would be surprised if performance of promises was remotely significant against performance of the database library itself – I'd love to see any benchmarks.

I think @sidorares' summary is pretty good.

Redsandro commented 7 years ago

@chrisveness thanks for the quick response. I've been using mysqljs with bluebird anyway, and I have been waiting for prepared statements, which mysql2 seems to implement. So it sounds good.

I see mysql2 is in the process of being merged with mysqljs. I might take the completion of that as proof of being production ready, for no clear reason.

Xstoudi commented 7 years ago

Using the new keyword is unnecessary overhead and a big no-no in node. Bluebird promises are much more optimized than the native Promise implementation. For example, you can Promise.resolve('something') and chain from there, never having to use the new keyword. The author spent a lot of time optimizing things. You can read his own explanation here: StackExchange: Why are native ES6 promises slower and more memory-intensive than bluebird?`

As you can see on the V8 project bug reporting, native promises are on the way.