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 leak:- ark-forger process. #2326

Closed
samharperpittam opened this issue Mar 27, 2019 · 6 comments
Closed

Memory leak:- ark-forger process. #2326

samharperpittam opened this issue Mar 27, 2019 · 6 comments

Comments

@samharperpittam
Copy link

samharperpittam commented Mar 27, 2019

Describe the bug
Multiple users have reported that the ark-forger process shows signs of a memory leak. A delegate's ark-forger was using 1.4GB of memory upon checking as shown below...

ark@gnia:~$ pm2 status
┌────────────┬────┬──────┬────────┬───┬───────┬────────────┐
│ Name │ id │ mode │ status │ ↺ │ cpu │ memory │
├────────────┼────┼──────┼────────┼───┼───────┼────────────┤
│ ark-forger │ 2 │ fork │ online │ 0 │ 3% │ 1.4 GB │
│ ark-relay │ 1 │ fork │ online │ 0 │ 14.9% │ 374.1 MB │
└────────────┴────┴──────┴────────┴───┴───────┴────────────

To Reproduce
Steps to reproduce the behavior:

  1. Start ark-forger process
  2. Let the process run for weeks/months without restart

Expected behavior
For the ark-forger process to be using significantly less than 1.4GB of memory.

Screenshots

Server (please complete the following information):

  • OS: Ubuntu
  • Version: 18:04
    However, this issue has been reported by multiple users with various versions of operating system& system specs.

Additional context

Delegate alessio // fun has created a patch file for this issue which he will send to me. Once received, I shall forward it to the dev team for review.

@ghost
Copy link

ghost commented Mar 27, 2019

Thanks for opening this issue! A maintainer will review this in the next few days and explicitly select labels so you know what's going on.

If no reviewer appears after a week, a reminder will be sent out.

@kristjank
Copy link
Contributor

Thanks for the report @alessiodf and providing the patch core. We are testing this already, as we want to narrow it down to exact cause/version/setup. So we will get back on this soon.

@alessiodf
Copy link
Contributor

alessiodf commented Mar 28, 2019

The exact cause was described in the patch comment:

The default Promise implementation in JavaScript leaks memory when used recursively. This was confirmed by heap snapshot profiling which shows an increased number of unresolved Promises from the forger process that accumulate over time and are never GC'd. Refactoring this to remove the recursion, in favour of a traditional while loop, resolves the memory leak.

As a further demonstration, I've attached an extremely crude script that will cause Brian to have an aneurysm if he reads the source, but it demonstrates the memory leak in the existing approach versus the refactored design as it prints the heap memory usage after manually running the garbage collector.

Run it with --expose-gc --stack_size=10000 parameters for node, and then either true or false (true executes the leaky branch, false executes the fixed branch) and then the number of repetitions. Don't try to run more than 50000 repetitions with the leaky branch or you may encounter a segfault (the non-leaky branch is fine to go beyond this).

With 5000 repetitions:

node --expose-gc --stack_size=10000 heap.js true 5000
Finished 5000 repetitions - heap memory used: 3979136

node --expose-gc --stack_size=10000 heap.js false 5000
Finished 5000 repetitions - heap memory used: 3459096

With 10000 repetitions:

node --expose-gc --stack_size=10000 heap.js true 10000
Finished 10800 repetitions - heap memory used: 4517656

node --expose-gc --stack_size=10000 heap.js false 10000
Finished 10800 repetitions - heap memory used: 3459096

With 50000 repetitions:

node --expose-gc --stack_size=10000 heap.js true 50000
Finished 50000 repetitions - heap memory used: 8684944

node --expose-gc --stack_size=10000 heap.js false 50000
Finished 50000 repetitions - heap memory used: 3708392

A couple of extreme examples using the non-leaky version:

node --expose-gc --stack_size=10000 heap.js false 10000000
Finished 10000000 repetitions - heap memory used: 3708392
node --expose-gc --stack_size=10000 heap.js false 1000000000
Finished 1000000000 repetitions - heap memory used: 3708408

As you will see, the memory usage in the non-leaky version barely rises as the number of repetitions increases and eventually caps out at 3.7MB regardless of the number of further repetitions, whereas the leaky version continually uses more memory as the number of repetitions grows, which demonstrates that the forger process will leak each time the loop executes (i.e. once every ~8 seconds). Over many days, weeks and months, this leak will become very substantial as indicated by kolap's mem usage.

@kristjank
Copy link
Contributor

💯 yes. Nice detailed explanation. Also looks like related to node v10.x.x. As node v11 behaves much better on this.

@alessiodf
Copy link
Contributor

The tests above were node v10. Running v11 now:

Using promise recursion:

node --expose-gc --stack_size=10000 heap.js true 20000
Finished 20000 repetitions - heap memory used: 3156488

Using normal loop:

node --expose-gc --stack_size=10000 heap.js false 20000
Finished 20000 repetitions - heap memory used: 2445768

Using promise recursion:

node --expose-gc --stack_size=10000 heap.js true 50000
Finished 50000 repetitions - heap memory used: 4597400

Using normal loop:

node --expose-gc --stack_size=10000 heap.js false 50000
Finished 50000 repetitions - heap memory used: 2445768

So it still leaks in v11 (even if not as much, it still does leak which will accumulate over days/weeks/months), whereas the normal loop does not leak at all.

@ghost
Copy link

ghost commented Mar 31, 2019

This issue has been closed. If you wish to re-open it please provide additional information.

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

4 participants