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 in ipfs add #1924

Closed
ghost opened this issue Nov 1, 2015 · 14 comments
Closed

Memory leak in ipfs add #1924

ghost opened this issue Nov 1, 2015 · 14 comments
Labels
kind/bug A bug in existing code (including security flaws) topic/perf Performance

Comments

@ghost
Copy link

ghost commented Nov 1, 2015

I'm add'ing https://downloads.openwrt.org/chaos_calmer/15.05/

lars@castor:~$ find downloads.openwrt.org/chaos_calmer/15.05/ | wc -l
278636
lars@castor:~$ du -sh downloads.openwrt.org/chaos_calmer/15.05/
37G downloads.openwrt.org/chaos_calmer/15.05/

It ran for about 1h20m, in the meantime Go resident memory went from 121M to 13261M, and then it probably OOMed.

These are a lot of files that it needs to keep hashes for in memory, but it looks more like we're leaking the file contents.

I just started another run and so far it's consuming 3761M after adding 1870M worth of files. Here's the pprof goroutine, goroutine-full, and heap dumps: https://ipfs.io/ipfs/QmYCkvATcJcpZbs4mwvMZXzNp2CPKZKBkawYEBpZnv4Xbq

Aborting the second run now.

cc @whyrusleeping

@jbenet
Copy link
Member

jbenet commented Nov 1, 2015

@whyrusleeping think this is the dag editor accumulating stuff in resident memory for a long time?

@whyrusleeping
Copy link
Member

@lgierth could I also get your binary?

@ghost
Copy link
Author

ghost commented Nov 2, 2015

@rht
Copy link
Contributor

rht commented Nov 2, 2015

This was reported in #1584 (i.e. this issue's reference count is 2).

@rht
Copy link
Contributor

rht commented Nov 24, 2015

@lgierth I think this has been fixed since #1965 (in dev0.4.0). There will be even less memory growth once #1964 is merged.

@ghost
Copy link
Author

ghost commented May 29, 2016

Still leaks, @davidar's add is eating 2.5G/h right now:

graph

@ghost
Copy link
Author

ghost commented May 29, 2016

Interestingly, the other metric being affected by this leak is the number of peers:

peers graph

@ghost ghost added kind/bug A bug in existing code (including security flaws) topic/perf Performance labels May 29, 2016
@ghost ghost changed the title Possible memory leak with ipfs add Pretty bad memory leak with ipfs add May 29, 2016
@ghost ghost changed the title Pretty bad memory leak with ipfs add Memory leak in ipfs add May 29, 2016
@ghost
Copy link
Author

ghost commented May 29, 2016

@rht
Copy link
Contributor

rht commented May 30, 2016

This is because the mfs abstraction still creates intermediate root dag for each mfs.PutNode.
The memory profile as of 6c33503:
memory

One fix is to repurpose #1964 into a new function in mfs that doesn't construct any intermediate root dag.
Simplifying the function to roughly just contain r.dserv.Add results in a flatter graph:
memory_efficient

(and a write throughput rate that is slightly faster than git / tar|gz at files with each size of >10kb range)

@whyrusleeping
Copy link
Member

This is because the mfs abstraction still creates intermediate root dag for each mfs.PutNode.

This is a regression, introduced in 06e7103. There is a fix for it in #2690 but it appears interest there has... languished.

@rht
Copy link
Contributor

rht commented May 31, 2016

This is a regression, introduced in 06e7103.

This assumes

  1. there is no regression before the commit / pr
  2. mfs.PutNode is 'fixed' (i.e. made faster) by caching childNode
    Neither is the case. I reverted the pr associated with 06e7103 and ran the memory profiling benchmark, still observed the leak.

@ghost
Copy link
Author

ghost commented May 31, 2016

@davidar restarted his add command and biham is back to 29G, i dumped the debugging info into biham:/root/memleak.

@whyrusleeping
Copy link
Member

This should be resolved by #2795. Its ready to go, just watiing for a small bit of CR first

@matthewrobertbell
Copy link

Hi,

I brought this up before in #2615 - This time I took the 64 bit Linux v0.4.3 official binary, put it on a 2GB digital ocean VPS as before. This time, it got to 67GB of (mostly small) files added, but was killed because it was out of memory.

Things are a lot better in this version, but it seems like there is still a memory leak somewhere.

Happy to publish the script which I've been using if anyone wants to reproduce this, it's archiving open source data (python packages).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug A bug in existing code (including security flaws) topic/perf Performance
Projects
None yet
Development

No branches or pull requests

4 participants