Google-cloud-node: logging: memory leak

[ofrobots]

Extracted from https://github.com/GoogleCloudPlatform/google-cloud-node/issues/2125#issuecomment-288700032

/cc @dennismartensson

[ofrobots]

@dennismartensson: My hope is that we can reproduce the memory leak on our end – let us start from that. It might be useful if you can give us (privately, if you prefer) some sample log entries that you sending. We'll try those in our load-test.

[dennismartensson]

I will share some here and then if you need more we can talk privately.

We use:

"restify": "^5.0.0-beta-1",
"restify-plugins": "^1.1.0",
"bunyan": "^1.8.8",
"@google-cloud/logging-bunyan": "^0.2.0",

We track requests with a child logger that is attached to the req with a id:
req.log = log.child({ req_id: req.getId() })

and we use the restifyplugins.requestLogger to create request Logs.
Out side of that most logs are very standard texted logs like req.log.info('created user')

Hope that helps and if you like more info please let me know.

[ofrobots]

I can reproduce this leak.

[dennismartensson]

Hi @ofrobots

Good let me know if we can help out with some thing. We are more then happy to send some traffic to it when you have a fix out.

Thanks for digging in to this.

[ofrobots]

I believe this should be fixed by d053b20be0e300cdff4c24da4ad159b31a9a67db. Need to confirm today.

[ofrobots]

Upon a deeper investigation, that commit doesn't seem to fix it 😞 .

I am still investigating.

[ofrobots]

With the above fix in place, I no longer see the JS heap or node external memory (i.e. memory bound to JS objects) growing. The RSS does grow fairly rapidly. This leaves native modules, and gRPC is the only native module present.

I have opened https://github.com/grpc/grpc/issues/10445 to see if this is possibly a leak in gRPC.

[murgatroid99]

I have published [email protected], which includes a fix for the leak.

[ofrobots]

With the gRPC issue fixed, I believe #2198 would address the remaining leak – the problem was that the log messages were being written faster than the writable stream (which only implemented _write) could drain them. Adding support for _writev fixes this issue.

[ofrobots]

@dennismartensson It would be great if you can try with @google-cloud/[email protected] and report back if you have any issues. Thanks!

[dennismartensson]

@ofrobots Thanks for all the work. We will try it out soon!

[gaofanmichael]

This issue was moved to googleapis/nodejs-logging-bunyan#3