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

Preferred way to handle reconnect #2113

Closed
chris-rock opened this issue Aug 4, 2014 · 79 comments
Closed

Preferred way to handle reconnect #2113

chris-rock opened this issue Aug 4, 2014 · 79 comments
Labels
type: feature For issues and PRs. For new features. Never breaking changes.

Comments

@chris-rock
Copy link

In case the db connection is lost, we need to reconnect to the database. I couldn't find a preferred way to handle this issue. Do you have a best practice? The issue can be solved on various ways:

  • handle in sequelize
  • handle in db module like pg
  • handle this issue outside of nodejs with connection handler

Thanks in advance

@janmeier
Copy link
Member

I believe this would be best handled in sequelize - at least we could retry a number of times and otherwise give up. I believe we could handle this rather nicely actually. Each dialect returns an instance of Sequelize.ConnectionLost when the connector lib gives and error, and the abstract connector manager handles trying to reconect. It should be as simple as just trying the query again, since that should try to fetch a new connection (provided that the previous query flagged its connection as broken properly).

Flagging this as a feature request

@chris-rock
Copy link
Author

@janmeier Thanks that would be amazing.

@bushev
Copy link

bushev commented Jan 17, 2015

@kungfoolabs
Copy link

+1 This is definitely something desired. Thanks!

@gustavohenke
Copy link

👍 for this feature.

@herlon214
Copy link

👍 Waiting too

@gtomitsuka
Copy link

+1 I'm gonna add a bounty for this one

@trshafer
Copy link

+1

@Pomax
Copy link

Pomax commented Jun 9, 2015

This feature is so important in actual production setting, where databases run on completely different hosts, and having those hosts hiccup completely wreck everything because sequelize won't reconnect even though it should be able to.

I can't even emphasize how much this should be the most important feature to add as soon as possible, even just an autoreconnect: true to make Sequelize try to reconnect without any kind of checking what actually went wrong would make DB interfacing with it vastly more useful =)

@mickhansen
Copy link
Contributor

@Pomax so effectively retry queries anytime we get a connection type error? (Retrying a query should try to initiate a new connection assuming the old one was marked invalid which it should have).

That would necessarily be so hard to implement, could happen at query level and look for a global or local option.

@mickhansen
Copy link
Contributor

(if you are willing to be a guinea pig i'd be happy to write something like that, we've just been hesitant since it would be hard to have unit or regression tests for, so we need someone to battle test it, and for some reason i've never really encountered it).

@mickhansen
Copy link
Contributor

Small issue of transactions of course, they would likely need to be retried at the transaction level rather than the query level.

@Pomax
Copy link

Pomax commented Jun 10, 2015

Yeah, as just a simplest option "retry ad infinitum, or the user explicitly set a retry count" would be amazingly useful already. My api.nihongoresources.com has a database hosted not just on a different machine, but by a different organization, and it drops every so often because the machine it's on power cycles fairly often (few weeks, if not days sometimes), which absolutely destroys API functionality.

If you want a guinea pig: it can't get worse than what it's doing right now, hook me up ;) This would be amazing to have:

var reconnectOptions = {
  max_retries: 999,
  onRetry: function(count) {
    console.log("connection lost, trying to reconnect ("+count+")");
  }
};

var sequelize = new Sequelize(database,username,password, {
                  dialect: "mysql",
                  port: 3306,
                  logging: false,
                  reconnect: reconnectOptions || true
                });

("or true" because I'll even just take a plain old "just try to reconnect forever" over "all things now broken" =D)

Transaction failure could be left up to the user really:

var reconnectOptions = {
  retry_on_reconnect: {
    transactions: true,
    ...
  },
  max_retries: 999,
  onRetry: function(count) {
    console.log("connection lost, trying to reconnect ("+count+")");
  }
};

@mickhansen mickhansen self-assigned this Jun 10, 2015
@mickhansen
Copy link
Contributor

Great :) I'll see if i can take a look at this soon.
The problem with transactions is that for PG a failed query will take down the entire transaction, so you can't retry a transaction alone, i think i'd like to build functionality to retry the entire transaction.

@Pomax
Copy link

Pomax commented Jun 10, 2015

that's how transactions should work though, if it's a true transaction? if any part of a transaction fails, up to and including the commit, it should be discarded.

@mickhansen
Copy link
Contributor

@Pomax Right but query retrying would have to be disabled inside a transaction then :) And then it might make sense to retry the entire transaction if one query had a connection error.

Although now that i'm thinking about it queries inside a transaction run on the same connection so might not be an issue.

@Pomax
Copy link

Pomax commented Jun 10, 2015

Ahh, yes, very true. If the DBMS is smart enough to roll back on a broken connection or timeout, that should be fine (not sure off-hand what pg and mysql do there)

@sebay00
Copy link

sebay00 commented Jun 11, 2015

+1

@fatmatto
Copy link

+1 this would be great

@Joshua-F
Copy link

Joshua-F commented Jul 3, 2015

Since this is still open any suggestions on how we should handle a database dropping out until it's implemented into Sequelize?

@Pomax
Copy link

Pomax commented Jul 3, 2015

@Joshua-F my solution was to use a fairly dumb try/catch probe (like a select 1+1), and rebuilding the sequelize instance when it throws. That still fails when the database server is entirely unavailable (i.e. it plain old just drops instead of the connection being flaky) but when that happens you have a completely different set of problems anyway.

@YourDeveloperFriend
Copy link

@Pomax that's a great idea! Do you have an example of how you rebuild the sequelize instance? Do you essentially wrap the construction in a factory function:

var Sequelize = require('sequelize');

module.exports = function buildSequelize() {
  var sequelize = new Sequelize('username', 'password');
  sequelize.define('User', {
    // ...
  });
  return sequelize;
};

Or are you using an easier method that simply closes and reopens sequelize? (I would prefer that method if it exists)

@Pomax
Copy link

Pomax commented Jul 15, 2015

nope, that's what I do =)

@dxdc
Copy link

dxdc commented Aug 27, 2015

@mickhansen is there an updated solution or fix for this pending? I'm having this issue as well...great to see that it's been well captured in this thread.

The 'wait_timeout' variable may be relevant as well, see:
mysqljs/mysql#1070

@AndrewFarley
Copy link

@dxdc I originally thought I had this problem and commented on this thread like two days ago, but deleted my comment when I found that the latest sequelize does tend to do this automatically every time that a query is requested. At least I found this true with using sequelize with mysql.

@dxdc
Copy link

dxdc commented Aug 27, 2015

thanks @AndrewFarley ; I'm still having this issue. As far as I can tell, it's connected to the wait_timeout variable in the MySQL settings. The default is 28800 (8 h), but this is too high for common web applications where a more realistic recommendation is 30-300 sec.

@ashish277d
Copy link

@rimiti @rameshm1
thanks for the update.Do we need to pass any parameter as a part of config to make it working?

@rameshm1
Copy link

@ashish277d No. If you have the above-mentioned version (4.11.1), the reconnection behavior is default.

@anonrig
Copy link

anonrig commented Oct 14, 2017

I couldn't see this feature/fix on the changelog, can anybody reference the appropriate commit for this? Thanks.

@rimiti
Copy link

rimiti commented Oct 16, 2017

@ashish277d

@ashish277d No. If you have the above-mentioned version (4.11.1), the reconnection behavior is default.
👍

@RBYD3vYaox
Copy link

I did the following testing:

  • Setup a connection pool with at least 5 connections, at most 100 connections, and maximum idle time is 10 minutes.
  • Start the application and the connections are being established.
  • Shutdown the database (PostgreSQL 9.6 in my case). All connections in the connection pool are disconnected.
  • Start the database again.
  • Keep trying to send queries to the database.

According to my testing, it takes around 1 minute before a query can be sent to the database successfully.

Is it possible to configure how quickly are those connections being re-established?

@ashish-dehariya
Copy link

it is still not working for me. I have latest version 4.13.8.
events.js:160
throw er; // Unhandled 'error' event
^
Error: Connection lost: The server closed the connection.
at Protocol.end (/home/default/node_modules/mysql/lib/protocol/Protocol.js:113:13)
at Socket. (/home/default/node_modules/mysql/lib/Connection.js:109:28)
at emitNone (events.js:91:20)
at Socket.emit (events.js:185:7)
at endReadableNT (_stream_readable.js:974:12)
at _combinedTickCallback (internal/process/next_tick.js:80:11)
at process._tickDomainCallback (internal/process/next_tick.js:128:9)
npm info lifecycle @qes/nbme-portal@0.1.0~start: Failed to exec start script

@RBYD3vYaox
Copy link

RBYD3vYaox commented Oct 24, 2017

@ashish-dehariya it is still not working for me. I have latest version 4.13.8.

I think Sequelize reconnects after a certain period of time (perhaps controlled by a timeout value?) instead of reconnect immediately, but I'm not sure about this.

@kostyay
Copy link

kostyay commented Jan 23, 2018

did anyone manage to get this working? it doesnt work for me either.

@RBYD3vYaox
Copy link

RBYD3vYaox commented Jan 24, 2018

@kostyay did anyone manage to get this working? it doesnt work for me either.

See my comment above.

@yocontra
Copy link
Contributor

yocontra commented Jan 29, 2018

Here's my thinking:

  • sequelize already has retry logic for queries
  • sequelize already manages the connection pool, and removes closed/errored connections

From what I understand the only missing piece (and the root of this issue) is that the retry around the queries will reuse the same connection, even if it died. I'll try to dig more into this and send a PR, but I think this might end up being a really simple fix - when retrying a query, yield the old connection and ask for a new one.

This configuration will already retry the queries on connection errors, but it will fail forever because it reuses the old connection:

// this goes to retry-as-promised for queries
retry: {
  match: [
    /SequelizeConnectionError/,
    /SequelizeConnectionRefusedError/,
    /SequelizeHostNotFoundError/,
    /SequelizeHostNotReachableError/,
    /SequelizeInvalidConnectionError/,
    /SequelizeConnectionTimedOutError/
  ],
  name: 'query',
  backoffBase: 100,
  backoffExponent: 1.1,
  timeout: 60000,
  max: Infinity
}

When a query fails due to a connection error, retry with exponential backoff with a max time of 60 seconds. This seems like a really robust way to manage reconnects/retries, just needs that fix for the connections and this should close a bunch of tickets around this.

Edit: PR submitted - #8961

@stringbeans
Copy link

stringbeans commented Jun 16, 2018

im still unable to get this working...

according to the documentation (http://docs.sequelizejs.com/class/lib/sequelize.js~Sequelize.html) it states that there are defaults set for all the pool options..

my repro steps using mariadb 10.2:

  1. start my api server that is using sequelize v4.37.10
  2. issue a query (works fine)
  3. stop the database
  4. issue a query (fails due to db being down)
  5. start the database
  6. issue a query (continues to fail)

even after a few minutes it seems as though the connections arent reconnecting and im unable to query the db.

my config looks like so...

{
    dialect: 'mysql',
    dialectOptions: {
      multipleStatements: true,
      //this is to force mysql2 lib to coerce decimal numbers into actual numbers instead of strings https://github.com/sequelize/sequelize/issues/8019
      decimalNumbers: true
    },
    replication: {
      write: {
        host: process.env.MYSQL_MASTER_HOST,
        port: process.env.MYSQL_MASTER_PORT,
        username: process.env.MYSQL_MASTER_USERNAME,
        password: process.env.MYSQL_MASTER_PASSWORD
      },
      read: [{
        host: process.env.MYSQL_SLAVE_HOST,
        port: process.env.MYSQL_SLAVE_PORT,
        username: process.env.MYSQL_SLAVE_USERNAME,
        password: process.env.MYSQL_SLAVE_PASSWORD
      }]
    },
    pool: {
      max: process.env.MYSQL_MASTER_CONNECTION_LIMIT,
      min: 1,
      idle: 10000
    },
    define: {
      timestamps: false
    }
}

this recently affected us because there was an unexpected DB crash with our AWS RDS instance.

@kingjerod
Copy link

@stringbeans
Did you ever figure this out? Seeing this on production right now, Aurora Postgres failover and Sequelize doesn't reconnect (for over 40 minutes). Before when we didn't have replication I think it worked OK. But after adding replication config it won't reconnect on failover.

@kingjerod
Copy link

I ended up listening for a SequelizeConnectionRefusedError or SequelizeConnectionError in my middleware and if it detects that it restarts the process. A bit hacky but it works.

@zhming0
Copy link

zhming0 commented Nov 14, 2018

After a lot testing, I can confirm that this issue still exists under replication mode.
A quick & dirty mitigation is increasing pool.acquire to a huge value, and configure retry like @contra mentioned above.

I am trying to locate the bug and maybe make a patch for it. (not anymore as we moved to TypeORM 😢 )

@bertolo1988
Copy link

bertolo1988 commented Feb 1, 2019

My connection to MySQL is lost and never recovers.

Sequelize: 4.41.1
Node: 8.12.0

@davelandry
Copy link

@kingjerod would you mind posting a code snippet for your middleware hack?

@kingjerod
Copy link

@davelandry

My processes are managed with PM2 so exiting will cause PM2 to restart the process. There is another middleware before this that catches errors and sets the ctx.body.error message. There is some typescript in here that can be easily stripped out.

let exiting: boolean = false;
async function failoverCheck(ctx: Koa.Context, next) {
  await next();
  if (exiting) {
    return;
  }

  if (ctx.body && ctx.body.error && typeof ctx.body.error === "string") {
    const error: string = ctx.body.error.toLowerCase();
    const sequelizeErrors = [
      "SequelizeConnectionRefusedError",
      "SequelizeConnectionError"
    ];
    for (const sequelizeError of sequelizeErrors) {
      if (error.includes(sequelizeError.toLowerCase())) {
        exiting = true;
        // We wait 10 seconds so that when it reconnects hopefully the failover is finished
        setTimeout(() => {
          process.emit("SIGTERM");
        }, 10000);
      }
    }
  }
}

@gwilakers
Copy link

Here's my thinking:

* sequelize already has retry logic for queries

* sequelize already manages the connection pool, and removes closed/errored connections

From what I understand the only missing piece (and the root of this issue) is that the retry around the queries will reuse the same connection, even if it died. I'll try to dig more into this and send a PR, but I think this might end up being a really simple fix - when retrying a query, yield the old connection and ask for a new one.

This configuration will already retry the queries on connection errors, but it will fail forever because it reuses the old connection:

// this goes to retry-as-promised for queries
retry: {
  match: [
    /SequelizeConnectionError/,
    /SequelizeConnectionRefusedError/,
    /SequelizeHostNotFoundError/,
    /SequelizeHostNotReachableError/,
    /SequelizeInvalidConnectionError/,
    /SequelizeConnectionTimedOutError/
  ],
  name: 'query',
  backoffBase: 100,
  backoffExponent: 1.1,
  timeout: 60000,
  max: Infinity
}

When a query fails due to a connection error, retry with exponential backoff with a max time of 60 seconds. This seems like a really robust way to manage reconnects/retries, just needs that fix for the connections and this should close a bunch of tickets around this.

Edit: PR submitted - #8961

@contra Quick question: I see that the documentation mentions the retry.match and retry.max properties, but none of the other ones. Are those properties undocumented? Thanks.

@yocontra
Copy link
Contributor

@gwilakers Yeah, they are undocumented.

BTW for retrying queries - this bug is currently breaking some cases: #10453

@gwilakers
Copy link

@contra Okay, thanks for confirming.

Ironically, I just subscribed to that issue this morning haha. Thanks for the update.

@adams-family
Copy link

This doesn't seem to work in one specific case:

  1. first start sequelize with sequelize.sync()
  2. second start mysql db

The connection to the database is established fine and sequelize is operational, however model sync has been skipped and is never retried.

Any ideas if I'm doing something wrong?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feature For issues and PRs. For new features. Never breaking changes.
Projects
None yet
Development

No branches or pull requests