Skip to content
This repository has been archived by the owner. It is now read-only.

Not reconnecting after mysql server is restarted #10

Closed
fcarreiro opened this issue Aug 25, 2016 · 6 comments
Closed

Not reconnecting after mysql server is restarted #10

fcarreiro opened this issue Aug 25, 2016 · 6 comments

Comments

@fcarreiro
Copy link
Contributor

@fcarreiro fcarreiro commented Aug 25, 2016

  • Linux machine1 3.19.0-25-generic #26~14.04.1-Ubuntu SMP Fri Jul 24 21:16:20 UTC 2015 x86_64 x86_64 x86_64 GNU/Linux
  • Node v4.5.0
  • "mysql-connection-manager": "0.0.13"
  • mysqld Ver 10.1.16-MariaDB-1~trusty for debian-linux-gnu on x86_64 (mariadb.org binary distribution)
  mysql-connection-manager Sending keep-alive signal +10s
  mysql-connection-manager Connection OK +1ms

mysql server stopped

  mysql-connection-manager Connection error: "PROTOCOL_PACKETS_OUT_OF_ORDER" +5s
  mysql-connection-manager Sending keep-alive signal +25s
  mysql-connection-manager Connection error: "PROTOCOL_ENQUEUE_AFTER_FATAL_ERROR" +1ms

mysql server restarted

  mysql-connection-manager Sending keep-alive signal +18s
  mysql-connection-manager Connection error: "PROTOCOL_ENQUEUE_AFTER_FATAL_ERROR" +1ms
  mysql-connection-manager Sending keep-alive signal +30s
  mysql-connection-manager Connection error: "PROTOCOL_ENQUEUE_AFTER_FATAL_ERROR" +0ms

After stopping and rerunning the node app the connection is established.

@fcarreiro
Copy link
Contributor Author

@fcarreiro fcarreiro commented Aug 25, 2016

Maybe the lib is not realizing that there was fatal error and therefore not reconnecting?

@fcarreiro
Copy link
Contributor Author

@fcarreiro fcarreiro commented Aug 25, 2016

Indeed, with some more debug

  mysql-connection-manager Connection error: "PROTOCOL_PACKETS_OUT_OF_ORDER" +6s
  mysql-connection-manager {"code":"PROTOCOL_PACKETS_OUT_OF_ORDER","fatal":true} +0ms

Now if I change these lines

        if (error.code == 'PROTOCOL_CONNECTION_LOST') {
            this.disconnected();
        }

to

        if (error.fatal) {
            this.disconnected();
        }

then it works correctly

  mysql-connection-manager Connection error: "PROTOCOL_PACKETS_OUT_OF_ORDER" +6s
  mysql-connection-manager {"code":"PROTOCOL_PACKETS_OUT_OF_ORDER","fatal":true} +0ms
  mysql-connection-manager Connection to database has been lost +0ms
  mysql-connection-manager Attempting to establish connection +503ms
  mysql-connection-manager Creating new database connection +1ms
  mysql-connection-manager Failed to establish connection: "ECONNREFUSED" +4ms
...
server restarted
...
  mysql-connection-manager Attempting to establish connection +1s
  mysql-connection-manager Creating new database connection +0ms
  mysql-connection-manager Connection has been established +4ms
  mysql-connection-manager Listening for disconnects +0ms

Will you accept a pull request?

@chill117
Copy link
Owner

@chill117 chill117 commented Aug 29, 2016

Yes, I accept PRs. Nice work. I'll have a closer look, test, and merge later today.

@fcarreiro
Copy link
Contributor Author

@fcarreiro fcarreiro commented Aug 29, 2016

Thanks. One a few more remarks about all this:

(1) It did happen to me a few times that the manager "missed" the fatal error (and the PROTOCOL_... error before this change). Therefore, even if the queries were failing the whole time (even the keepalive) the manager was not trying to reconnect because the errors were not fatal (I was getting PROTOCOL_ENQUEUE_AFTER_FATAL_ERROR which is not fatal). If possible, a more robust approach would be to check if the underlying connection is connected, and reconnect if not. Does the mysql driver have such a function?

(2) I did a bit more research and, what is the advantage of this package w.r.t. using connection pooling in the mysql driver? In that mode, even with a pool of 1 connection, the driver seems to manage reconnection. Probably does not send keep-alive signals and I think it does not have a reconnectDelay.

@chill117
Copy link
Owner

@chill117 chill117 commented Aug 29, 2016

(1) Yes, probably a better scheme could be designed to ensure that the connection is still good.

(2) Connection pooling via the node-mysql module might be better than using this module. I will have a closer look at this as well, to see if this module should be deprecated in favor of just using connection pooling.

@chill117
Copy link
Owner

@chill117 chill117 commented Sep 15, 2016

Deprecating this module.

@chill117 chill117 closed this Sep 15, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants