Wizcorp / node-graylog2

Graylog2 client library for Node.js
Other
58 stars 36 forks source link

CallCount++ will eventually break #18

Closed jasonmcaffee closed 7 years ago

jasonmcaffee commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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.

https://github.com/nodejs/node/issues/8871

ronkorving commented 7 years ago

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

jasonmcaffee commented 7 years ago

@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 commented 7 years ago

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.