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

keepalive not behaving as documented #1586

Open
mrx8 opened this issue Jun 14, 2022 · 4 comments
Open

keepalive not behaving as documented #1586

mrx8 opened this issue Jun 14, 2022 · 4 comments

Comments

@mrx8
Copy link

mrx8 commented Jun 14, 2022

according to the commit: 6b59a36
keepalive is now always on. It cannot be disabled. The inline-documentation erroneously says it is off by default.
I have a problem with this in combination with the libkeepalive-library and need to disable it.
Is it possible to integrate the possibility to disable keepalive in mysql2 once more ?

@sidorares
Copy link
Owner

hm, the change looks quite out of sync with the comments, I might need to re-read communication leading to in #1081

@benbotto can you comment?

@benbotto
Copy link
Contributor

I think this is a bug--true shouldn't be hard coded in the call to setKeepAlive. Seems like #1081 had the right logic, but in #1086 I erroneously removed the if (this.config.enableKeepAlive) check.

#1258 seems to correct the issue, mostly, but there's a comment on Pool.d.ts about a bad type. Also might want a !! on https://github.com/sidorares/node-mysql2/pull/1258/files#diff-1978d11697f58179bc32b66c6ac7f21cbdfd6649a8f013248a1f9feee25858feR99

    this.enableKeepAlive =
      options.enableKeepAlive === undefined || !!options.enableKeepAlive;

Not sure why #1258 was closed.

@sidorares
Copy link
Owner

@kn3ny are you keen to finish that work? The PR looks mostly ready, happy to merge it but if you still plan to make a fresh one let us know

@n0v1
Copy link
Contributor

n0v1 commented May 17, 2023

I've just created PR #2016 to fix this.

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

No branches or pull requests

4 participants