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

Remove query callback when release the connection #1785

Closed
abou7mied opened this issue Jul 28, 2017 · 4 comments
Closed

Remove query callback when release the connection #1785

abou7mied opened this issue Jul 28, 2017 · 4 comments
Assignees
Labels

Comments

@abou7mied
Copy link

@abou7mied abou7mied commented Jul 28, 2017

I don't know if is it a good practice to define custom pools for specific operations in the application or not.
Here is an example

const mysql = require('mysql');
const pools = {};

function getPool(name) {

  if (!pools[name])
    pools[name] = mysql.createPool({
      host: 'localhost',
      user: 'root',
      connectionLimit: 5,
    });

  return pools[name];

}

class TestClss {

  constructor(i) {
    this.fetched = false;
    this.i = i;
  }

  start() {
    getPool("test-pool").query("SELECT SLEEP(1);", () => {
      console.log("this.i", this.i);
      this.fetched = true;
    })
  }

}

function start() {
  for (let i = 0; i < 10; i++) {
    let obj = new TestClss(i);
    obj.start();
  }
}

start();

After all queries has been finished I took a snapshot of heap and found that 5 TestClss objects weren't garbage collected because it is has binding with query callback -> poolConnection -> pool -> pools(global variable)

screenshot from 2017-07-28 08-07-11

I know that it will be kept until next query then replaced with new callback but the problem If I set the connection limit to large number, and has multiple pools, it will be alot of objects without beeing garbage collected.

Thanks.

@dougwilson
Copy link
Member

@dougwilson dougwilson commented Jul 28, 2017

Ah, I see. I believe this can be fixed, I'm taking a look.

@dougwilson dougwilson self-assigned this Jul 28, 2017
@dougwilson
Copy link
Member

@dougwilson dougwilson commented Jul 29, 2017

Yea, can be fixed. The easiest fix is a change to a private property, which shouldn't be an issue (though can revisit later if it is for a different fix). But yea, I was able to reproduce and will push up a fix in just a bit to the repo.

@dougwilson
Copy link
Member

@dougwilson dougwilson commented Jul 31, 2017

Ok, sorry for the delay. I just pushed up the fix. Please feel free to test it out and let me know if the issue wasn't fixed. I'll release it as a patch even if I don't hear back, so don't feel pressed to test :)

@dougwilson
Copy link
Member

@dougwilson dougwilson commented Aug 1, 2017

@abou7mied just pushed in 2.14.1

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

No branches or pull requests

2 participants