Client: support 'OR" operator between 'pingInactive' and 'pingWaitRes' #182
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mscdex
reviewed
Oct 30, 2017
lib/Client.js
Outdated
| && this._config.pingInactive > 0 | ||
| && this._config.pingWaitRes > 0) { | ||
| && this._config.pingWaitRes > 0)) { | ||
| if (this._config.pingInactive === undefined){ |
There was a problem hiding this comment.
Missing space before { here and below.
This commit is to fix issue mscdex#181. According to https://github.com/mscdex/node-mariasql/blob/master/lib/Client.js#L633 , The protocol-level pings can be enabled by setting both 'pingInactive' and 'pingWaitRes' variablei as following: a. 'pingInactive' to how many milliseconds to wait before sending a ping when no queries are pending, b. AND 'pingWaitRes' to how many milliseconds to wait for a ping response before assuming a lost/dead connection. For example, If you want to check a connection state per 60 seconds, we have to define both 'pingInactive' and pingWaitRes. It means that the "MySQL server has gone away" issue will be always generated in case that we declare the timeout with one between 'pingInactive' and 'pingWaitRes'. Let's support an optional conditional operator instead of "AND" operation conditions. If user does not declared a default timeout, it will be 60 seconds by default. * Example source code: [foo.js] pingInactive: 60000, pingWaitRes: 60000, * Before applying this patch: events.js:160 throw er; // Unhandled 'error' event ^ Error: MySQL server has gone away at Error (native) * How to monitor a sleep time of database on MariaDB server: $ sudo apt-get install mytop [enter] $ mytop -u root -p**** ourjs [enter] MySQL on localhost (10.0.29) load 0.32 0.30 0.26 2/580 9391 up 0+14:33:20 [07:47:10] Queries: 1.7k qps: 0 Slow: 0.0 Se/In/Up/De(%): 02/00/00/00 Sorts: 0 qps now: 1 Slow qps: 0.0 Threads: 3 ( 1/ 2) 00/00/00/00 Key Efficiency: 100.0% Bps in/out: 0.9/127.1 Now in/out: 21.3/ 2.9k Id User Host/IP DB Time Cmd State Query -- ---- ------- -- ---- --- ----- ---------- 754 ourjs localhost:34766 test 38 Sleep $ cat /proc/754/stat $ cat /proc/754/status * How to change a default timeout of Mariadb server: $ sudo vi /etc/mysql/mariadb.conf.d/50-server.cnf [mysqld] wait_timeout = 120 (default value is 28800) interactive_timeout = 120 (default value is 28800) Signed-off-by: Geunsik Lim <[email protected]>
leemgs
force-pushed
the
upstream-wait-timeout
branch
from
b48dbb7
to
5f5fdc0
Compare
|
@mscdex I have re-submitted the PR with |
|
@mscdex, PTAL, Could you review this PR? |
|
@mscdex, RESEND, Could you review and merge this PR? |
|
@mscdex, Knock Knock. I re-submitted updated PR after applying your comment. Could your give me comment or merge this PR? |
|
RESEND |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit is to fix issue #181.
According to https://github.com/mscdex/node-mariasql/blob/master/lib/Client.js#L633 ,
The protocol-level pings can be enabled by setting both 'pingInactive' and
'pingWaitRes' variablei as following:
a. 'pingInactive' to how many milliseconds to wait before sending a ping
when no queries are pending,
b. AND 'pingWaitRes' to how many milliseconds to wait for a ping response
before assuming a lost/dead connection.
For example, If you want to check a connection state per 60 seconds, we have
to define both 'pingInactive' and pingWaitRes. It means that the "MySQL server
has gone away" issue will be always generated in case that we declare the
timeout with one between 'pingInactive' and 'pingWaitRes'.
Let's support an optional conditional operator instead of "AND" operation conditions.
If user does not declared a default timeout, it will be 60 seconds by default.
Example source code:
[foo.js]
pingInactive: 60000,
Before applying this patch:
events.js:160
throw er; // Unhandled 'error' event
^
Error: MySQL server has gone away
at Error (native)
How to monitor a sleep time of database on MariaDB server:
$ sudo apt-get install mytop [enter]
$ mytop -u root -p**** ourjs [enter]
MySQL on localhost (10.0.29) load 0.32 0.30 0.26 2/580 9391 up 0+14:33:20 [07:47:10]
Queries: 1.7k qps: 0 Slow: 0.0 Se/In/Up/De(%): 02/00/00/00
Sorts: 0 qps now: 1 Slow qps: 0.0 Threads: 3 ( 1/ 2) 00/00/00/00
Key Efficiency: 100.0% Bps in/out: 0.9/127.1 Now in/out: 21.3/ 2.9k
$ cat /proc/754/stat
$ cat /proc/754/status
$ sudo vi /etc/mysql/mariadb.conf.d/50-server.cnf
[mysqld]
wait_timeout = 120 (default value is 28800secs)
interactive_timeout = 120 (default value is 28800secs)
Signed-off-by: Geunsik Lim [email protected]
\CC: @myungjoo