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 leaking #20

Closed
frct1 opened this issue Sep 24, 2018 · 17 comments
Closed

Memory leaking #20

frct1 opened this issue Sep 24, 2018 · 17 comments
Labels

Comments

@frct1
Copy link

frct1 commented Sep 24, 2018

Hi @schroffl i've detected memory leak when we recreating connection and for example sending hostinfo command

let Helpers=require('./helper');
let helper=new Helpers('memorytest');
let async=require('async');
let runloop=function () {
    async.forEachOf(credentials,(instance,index,callback)=>{
        helper.connectAndAuth(instance)
            .then(connection=>{
                return {query:connection,info:query.send('hostinfo')};
            })
            .then((object)=>{
                object.query.send('logout');//logout
                object.query.sock.destroy();//destroy connection because limit in 200 connections per instance
                callback();
            });
    },function () {
        console.log(`OK`);
        runloop();
    })
}
runloop();

Method connectAndAuth from helper class

connectAndAuth(credentials){
        return new Promise(((resolve, reject) => {
            let connect = new TeamspeakQuery.Raw({host:credentials.SQHost || credentials.ip, port:credentials.SQPort||credentials.query,localAddress: 'address'}, );
            connect.sock.on('error', function (err) {
                reject(err.code);
            });
            connect.sock.on('connect',()=>{
                connect.send('login', credentials.SQLogin||credentials.username, credentials.SQPassword||credentials.password)
                    .then(function () {
                        connect.ip = credentials.ip;
                        connect.name = credentials.name;
                        connect.throttle.set('enable', false);
                        resolve(connect);
                    }).catch(function (error) {
                        reject(error);
                        console.log(error);
                });
            });
        }))
    }

Already tried query.sock.removeAllListeners(), but it doesn't matter for memory using
Currently same thing is working on my API for job and memory growing so fast due to big count of request per minute.
P.S* Memory leak doesn't depend from command which we send to server query, that can be only simple connection with login command

@schroffl
Copy link
Owner

Can you try to replace the logout command in the first snippet with quit? The latter should actually cause the server to close the connection, which then leaves the TeamspeakQuery instance open to garbage collection (as long as you don't store any other references).

I'm not entirely sure though, so if you could run the tests and tell me if it helped I'd be really thankful :)

@frct1
Copy link
Author

frct1 commented Sep 25, 2018

2018-09-25_14-02-47
Replaced to quit command, but no effect

@frct1
Copy link
Author

frct1 commented Sep 25, 2018

image
Memory using after 30k+ connections and login command

@schroffl
Copy link
Owner

I will check it out next week, since I won't be able to work on it until then.
I think I will finally implement #19 while analyzing/fixing this problem, because any resources can be freed in a proper disconnect method.

@frct1
Copy link
Author

frct1 commented Sep 26, 2018

Yeah that's very needed.
Api app (in cluster mode 4 instances) after multiple days take around 3-4 GB of RAM lol

@frct1
Copy link
Author

frct1 commented Oct 1, 2018

Hi, any changes or forecasts by troubleshooting?

@schroffl
Copy link
Owner

schroffl commented Oct 1, 2018

Hey, I just published Version 3.1.0 which adds a disconnect method as you requested in #19.
Can you try to call it like this instead of manually destroying the socket:

helper.connectAndAuth(instance)
  .then(connection => {
    return {
      query: connection,
      info: query.send('hostinfo')
    };
  })
  .then(object => object.query.disconnect())
  .then(() => callback());

Let's hope this fixes it :)

@frct1
Copy link
Author

frct1 commented Oct 2, 2018

Hi @schroffl.
I've tested update: now it works better.
After 10+k connections and login command memory usage doesn't growing up.
Now deployed new api version, and provide more info after 3 days

@schroffl
Copy link
Owner

schroffl commented Oct 2, 2018

Alright, thanks for your feedback :)

@schroffl schroffl added the bug label Oct 2, 2018
@frct1
Copy link
Author

frct1 commented Oct 4, 2018

Hi, there is still some leaking. But growing not so fast, by steps (i dont know what the reason).
Before update it is growing by every second. Now memory growing 1 time by N minutes
Tried with 100k+ connections

@schroffl
Copy link
Owner

schroffl commented Oct 4, 2018

The Garbage Collector doesn‘t have to kick in if there is no memory pressure. Are you sure that it is actually a leak? How much memory does it use in the end?

@frct1
Copy link
Author

frct1 commented Oct 4, 2018

At this moment 866k connections.
Stats in htop
image
Stats in pm2
App name │ id │ mode │ pid │ status │ restart │ uptime │ cpu │ mem
memorytest │ 35 │ fork │ 4344 │ online │ 6 │ 74m │ 0% │ 85.3 MB

@frct1
Copy link
Author

frct1 commented Oct 4, 2018

1400k connections 85MB of memory

@schroffl
Copy link
Owner

Hey, sorry for taking so long to finally answer. How does the memory consumption look now?

@frct1
Copy link
Author

frct1 commented Oct 11, 2018

Now all is great. All services works perfect without 5+GB memory using even in peak cases :D
p.s: Thank you for update 😘

@schroffl
Copy link
Owner

I will aim to run the memory profiler at some point in the next few weeks and if no suspicious characteristics come up I'd consider this resolved.

@schroffl
Copy link
Owner

I'm going to close this issue now. I have to admit that I didn't run the profiler, but since no other complaints came up I feel confident to consider this resolved.

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

No branches or pull requests

2 participants