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

CallCount++ will eventually break #18

Closed
jasonmcaffee opened this issue Sep 29, 2016 · 6 comments
Closed

CallCount++ will eventually break #18

jasonmcaffee opened this issue Sep 29, 2016 · 6 comments

Comments

@jasonmcaffee
Copy link

jasonmcaffee commented Sep 29, 2016

Probably not a huge priority, but while investigating a memory leak somewhere in winston, I noticed the getServer code using a clever way of load balancing between configured servers.
CallCount is never reset though, so once the count reaches 9007199254740992, callCount++ will stop incrementing, thus breaking load balancing.
https://github.com/Wizcorp/node-graylog2/blob/master/graylog.js#L49

@ronkorving
Copy link
Collaborator

Yeah, that's a crazy high number, but you're not incorrect :)
Basically it boils down to being able to send 1 million logs per second per process for 285 years. I hope that anyone logging that much with this library, is willing to put in a PR to fix this ;)

@jasonmcaffee
Copy link
Author

I'm looking into a significant memory leak in your module when running inside of a docker 1.11 container, and there is no graylog server running to accept messages.

I've been able to rule out dgram, going to take a look at zlib next...

@jasonmcaffee
Copy link
Author

jasonmcaffee commented Sep 30, 2016

The memory leak issue is with zlib.deflate.

Just doing this inside of docker (latest node) eats up 1GB of ram on my macbook pro, and it never releases

let zlib = require('zlib');

let message = {
  some:"data"
};
let payload = new Buffer(JSON.stringify(message));

for(var i =0; i < 10000; ++i){
  zlib.deflate(payload, function (err, buffer) {
  });
}

It's also a memory leak without docker, just nowhere near as bad. After that runs, local node process has ~250MB of ram.

nodejs/node#8871

@ronkorving
Copy link
Collaborator

As @bnoordhuis explained, this is not that strange. You're deflating 10000 times in parallel. Anyway, unrelated to node-graylog2.

@jasonmcaffee
Copy link
Author

jasonmcaffee commented Oct 5, 2016

@ronkorving I'm not sure what you mean. Using your package causes our services to crash after about a week of logging (our services aren't heavily utilized at the moment, so its not even a large amount of logging). Your package uses uses zlib deflate. Anyone who uses your package on a Linux system will be affected by this issue.

@ronkorving
Copy link
Collaborator

ronkorving commented Oct 7, 2016

Let's discuss that in issue #19, where it belongs (and where you can see that I'm all for allowing compression to be configurable). I'm gonna close this one.

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

2 participants