Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Leaked exception if database crashes. (MySQL server has gone away) #2442

Closed
conglq opened this issue Jan 29, 2018 · 20 comments · Fixed by #2450
Closed

Leaked exception if database crashes. (MySQL server has gone away) #2442

conglq opened this issue Jan 29, 2018 · 20 comments · Fixed by #2450

Comments

@conglq
Copy link

conglq commented Jan 29, 2018

Environment

Knex version:0.14.2
Database + version: mariaDb v10.2
OS: Ubuntu
Node: v9.3.0

My connection:

const Knex = require('knex')({
  client: 'mariasql',
  connection: {
    host: config.host,
    user: config.user,
    password: config.password,
    db: config.database,
    charset: 'utf8',
    pool: { min: 2, max: 10 },
    acquireConnectionTimeout: 30000
  }
});

After period of time my node app crash with error:

events.js:136
      throw er; // Unhandled 'error' event
      ^

Error: MySQL server has gone away

Please help me resolve this issue. Thanks all!

@elhigu
Copy link
Member

elhigu commented Jan 29, 2018

@elhigu
Copy link
Member

elhigu commented Jan 29, 2018

if you have pool min size 0 does it still happen? Does it happen if you are not making any queries, but just initialise the kenx instance?

@conglq
Copy link
Author

conglq commented Jan 30, 2018

@elhigu It still happend when I set pool min size 0. It happen when I am not making any queries after period of time.

@elhigu
Copy link
Member

elhigu commented Jan 30, 2018

@conglq what period of time are we talking to? 1 minute, 1 hour, 1 day?

@elhigu
Copy link
Member

elhigu commented Jan 30, 2018

So are you able to reproduce the problem with following code (I need exact steps to reproduce the problem, I cannot spend hours just guessing the way to make it fail)?

const knex = require('knex')({
  client: 'mariasql',
  connection: {
    host: config.host,
    user: config.user,
    password: config.password,
    db: config.database,
    charset: 'utf8',
    pool: { min: 0, max: 10 },
    acquireConnectionTimeout: 30000
  }
});

async function main() {
    await knex.raw('select 1');
    await periodOfTime();
    await knex.raw('select 1');
}   

main();

@wubzz
Copy link
Member

wubzz commented Jan 30, 2018

Just a wild guess, but could this be due to not specifying evictionRunIntervalMillis causing connections to never be evicted? Like the pool keeps the connections eventhough the mysql server has closed them.

@elhigu
Copy link
Member

elhigu commented Jan 30, 2018

@wubzz I thought the default pool setting testOnBorrow: true should take care of that case that dead connection is not returned from pool (if knex get the event of closed connection from the driver). It would be interesting to see if setting the evictionRunIntervalMillis makes a difference 👍 One more thing that could happen is that connection is fetched from pool for example for transaction and never returned. In that case dead connection probably would throw the error mentioned in OP (I don't believe that this is the problem here though).

@conglq
Copy link
Author

conglq commented Jan 30, 2018

@elhigu I run your test script and my app, it crashes after ~15 mins later.

@elhigu
Copy link
Member

elhigu commented Jan 30, 2018

@conglq please share the code, app that I posted was incomplete

@conglq
Copy link
Author

conglq commented Jan 30, 2018

my code

const config = require('../config').mySQL;

const knex = require('knex')({
  client: 'mariasql',
  connection: {
    host: config.host,
    user: config.user,
    password: config.password,
    db: config.database,
    charset: 'utf8',
    pool: { min: 0, max: 10 },
    acquireConnectionTimeout: 30000
  }
});

console.log(new Date());

const Bluebird = require('bluebird');
function periodOfTime () {
  return Bluebird.delay(1000 * 60 * 60 * 24);
}

async function main () {
  await knex.raw('select 1');
  await periodOfTime();
  await knex.raw('select 1');
}

main();

@elhigu
Copy link
Member

elhigu commented Jan 30, 2018

Ok so app actually just crashes during waiting... Is the error message still the same that in OP?

@conglq
Copy link
Author

conglq commented Jan 31, 2018

@elhigu yes, the error message still 'MySQL server has gone away'

@elhigu
Copy link
Member

elhigu commented Jan 31, 2018

btw. you knex configuration is wrong... IIRC, pool and acquireConnectionTImeout should be in the same level with connection attribute, not nested. connection attributes are the ones passed to bd driver. I'm running the testcase now though.

@conglq
Copy link
Author

conglq commented Jan 31, 2018

@elhigu I'm sorry ! my misstake. But it still makes the same error :(

const knex = require('knex')({
  client: 'mariasql',
  connection: {
    host: config.host,
    user: config.user,
    password: config.password,
    db: config.database,
    charset: 'utf8'
  },
  pool: { min: 0, max: 10 },
  acquireConnectionTimeout: 30000
});

@elhigu
Copy link
Member

elhigu commented Jan 31, 2018

With myslq I still wasn't able to reproduce this.

Start DB server:

docker run -i \
    -p 127.0.0.1:3306:3306 \
    -e MYSQL_ROOT_PASSWORD=my-secret-pw \
    -t cytopia/mariadb-10.2

Create DB:

Mikaels-MacBook-Pro-2:~ mikaelle$ mysql -h 127.0.0.1 -u root -p -P 3306
Enter password: 
Welcome to the MariaDB monitor.  Commands end with ; or \g.
Your MariaDB connection id is 11
Server version: 10.2.8-MariaDB MariaDB Server

Copyright (c) 2000, 2015, Oracle, MariaDB Corporation Ab and others.

Type 'help;' or '\h' for help. Type '\c' to clear the current input statement.

MariaDB [(none)]> create database knex_test;
Query OK, 1 row affected (0.00 sec)

MariaDB [(none)]> Ctrl-C -- exit!
Aborted
Mikaels-MacBook-Pro-2:~ mikaelle$ 

Running test code:

Mikaels-MacBook-Pro-2:objection.js mikaelle$ cat test-mysql.js 
const knex = require('knex')({
  client: 'mariasql',
  connection: 'mysql://root:my-secret-pw@localhost/knex_test',
  pool: { min: 0, max: 10 },
  acquireConnectionTimeout: 30000
});

console.log(new Date());

const Bluebird = require('bluebird');
function periodOfTime () {
  return Bluebird.delay(1000 * 60 * 60 * 2);
}

async function main () {
  const a = await knex.raw('select 1');
  console.log('a', a);
  await periodOfTime();
  console.log('b');
  await knex.raw('select 1');
  console.log('c');
  await knex.destroy();
}

main();
Mikaels-MacBook-Pro-2:objection.js mikaelle$ time node test-mysql.js 
2018-01-31T09:29:40.073Z
a [ [ { '1': '1' },
    info: { numRows: '1',
      affectedRows: '1',
      insertId: '0',
      metadata: [Object] } ],
  { numRows: '1',
    affectedRows: '1',
    insertId: '0',
    metadata: { '1': [Object] } } ]

// Closed server 

events.js:183
      throw er; // Unhandled 'error' event
      ^

Error: MySQL server has gone away

real	0m30.365s
user	0m0.276s
sys	0m0.067s

However I was able to get the error / crash when I closed DB server during waiting.

So basically the problem is that knex leaks exception and crashes in that case. I believe that better would be to catch the error and pass to knex in some knex.on('error', ...) event / through query rejection if DB is not available.

@elhigu elhigu changed the title MySQL server has gone away Leaked exception if database crashes. (MySQL server has gone away) Jan 31, 2018
@paulogdm
Copy link

paulogdm commented Feb 6, 2018

Hello! I have the same problem and I can help with testing/debugging.
Using now.sh: https://imgur.com/a/PXXT7
So the crash happens each 8 hours???

@koskimas
Copy link
Collaborator

koskimas commented Feb 6, 2018

@elhigu Does this still happen with the tarn branch? I fixed a mariadb related error handling problem there c963c68

@koskimas
Copy link
Collaborator

koskimas commented Feb 6, 2018

I just noticed that it still has this line:

connection.removeAllListeners('error');

in the ready event, that can screw things up. I think that could be the cause.

@koskimas
Copy link
Collaborator

koskimas commented Feb 6, 2018

I fixed that too in the PR

@elhigu
Copy link
Member

elhigu commented Feb 7, 2018

@koskimas thanks! your fix did work 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants