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

Memory Leak associated to Cycling Connections #726

Closed
aikar opened this issue Feb 7, 2018 · 5 comments
Closed

Memory Leak associated to Cycling Connections #726

aikar opened this issue Feb 7, 2018 · 5 comments

Comments

@aikar
Copy link
Contributor

aikar commented Feb 7, 2018

RE: #723 issue came to light because of this ability to cycle connections automatically with #724 , but this could in theory be triggered without my PR if the end user doesn't use a persistent pool, the connections timeout often, or they manually close and create new pools (maybe changing databases being worked on at a time)

I can verify that #723's issue has been fixed, but it appears there is yet another memory leak related to cycling connections.

Do you have any thoughts on where the issue might lie?

We're going to need to hold off on pulling #724 until this memory leak can be resolved.

It's leaking in native memory, as a 700M process only gave me a 20M heap dump, which is the same size heap as a 200M process.

Doing a heap comparison over 24 hours showed no signs of leaking on the heap....

So what other areas could be touching native memory? Doesn't look like the PreparedStatements have anything to do with it.

To reproduce, pull in 723 to local environment, configure it to have min 0 max 5 or so, timeout 30, write some code that every 60s it does 4 queries. this should cause connections to constantly cycle.

Then let the process run for a few days.

@aikar
Copy link
Contributor Author

aikar commented Feb 26, 2018

To add information to the issue, when I did a GDB core dump, and ran the memory through strings, I found my DSN information inside of the memory to be the source of the leak, in the various chunks (hostname, password, etc) but also various 9 character pieces of it (I see partials of the password)

These instances are 1.3 million of each of the hostnames (I connect to multiple servers), 6k each of various DSN's with the password + host + db, missing username.

This leads me to believe the leak is relating to the url parsing.

I found this issue nodejs/node#17448
I wonder if something was missed in that work. I will file an issue with node.

@sidorares
Copy link
Owner

interesting, thanks for update @aikar

mysqljs/mysql uses URL as well - https://github.com/mysqljs/mysql/blob/013f8fbd8a1866b3ab61787c1ae7657dd638cf7f/lib/ConnectionConfig.js#L172

if you happen to find root cause would be good to fix there too

@aikar
Copy link
Contributor Author

aikar commented Feb 27, 2018

@sidorares It appears it is the WHATWG one that leaks, which mysql2/mysql does not use.

I am calling const url = new URL(dsn); in my own code and passing the DSN information in object form to mysql2. so we may be able to say 'not an issue' for this bug, but let me spend a bit more time on it before we make that call.

@aikar
Copy link
Contributor Author

aikar commented Feb 27, 2018

Based on my findings, the leak is relating to my own codes use of WHATWG parser, and node devs has confirmed that the memory leak fix had not landed in 8.x yet.

closing this issue as it did not apply to this library, and we should be good to pull my cycling connections PR.

@aikar aikar closed this as completed Feb 27, 2018
@IronsDu
Copy link

IronsDu commented Dec 30, 2019

@aikar ,I have same question.
Do you know some library support set idle connections? Can you tell me ? Thank you

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

3 participants