bitswap/client: memory leak on BlockPresenceManager #574
Assignees
Labels
kind/bug
A bug in existing code (including security flaws)
need/analysis
Needs further analysis before proceeding
P1
High: Likely tackled by core team if no one steps up
Comments
|
Thank you for submitting your first issue to this repository! A maintainer will be here shortly to triage and review.
Finally, remember to use https://discuss.ipfs.io if you just need general support. |
aschmahmann
added
need/analysis
|
@Dreamacro can you attach or link to a pprof dump for analysis? Also, which versions of boxo and go-libp2p are you using? |
|
I first provide a few fragments of pprof/heap. I can confirm a memory leak here because the leakage disappears when I replace heap fragments: (Although boxo 0.15 is shown below, it has been upgraded to boxo 0.17) |
gammazero
added a commit
that referenced
this issue
Jul 8, 2024
Limit BlockPresenceManager map growth be doing the following: - Use nil map when BlockPresenceManager CID map is empty. - Delete peer map when from CID map when peer map is empty. - Remove peers from BlockPresenceManager when peers are pruned from session. This allows GC to free memory when maps in BlockPresenceManager become empty. Additional improvement: Do not look for lists of keys or peers in empty maps; return early instead. Fix for issue #574
gammazero
added a commit
that referenced
this issue
Jul 8, 2024
Limit BlockPresenceManager map growth be doing the following: - Use nil map when BlockPresenceManager CID map is empty. - Delete peer map when from CID map when peer map is empty. - Remove peers from BlockPresenceManager when peers are pruned from session. This allows GC to free memory when maps in BlockPresenceManager become empty. Additional improvement: Do not look for lists of keys or peers in empty maps; return early instead. Fix for issue #574
|
@Dreamacro PR #636 is a simple code modification that should help with unbounded map growth observed in this issue. The change allows GC to free memory when the |
|
@gammazero I can't quite confirm if this PR fixes the memory leak or not. The root cause of the problem is that after I'm not sure if the changes here can fix the root cause of the memory leak, if HAVE/DONT_HAVE is no longer processed after the prune node, the problem should be fixed |
lidel
pushed a commit
that referenced
this issue
Jul 26, 2024
* Fix memory leak on BlockPresenceManager Limit BlockPresenceManager map growth be doing the following: - Use nil map when BlockPresenceManager CID map is empty. - Delete peer map when from CID map when peer map is empty. - Remove peers from BlockPresenceManager when peers are pruned from session. This allows GC to free memory when maps in BlockPresenceManager become empty. Additional improvement: Do not look for lists of keys or peers in empty maps; return early instead. Fix for issue #574
wenyue
pushed a commit
to wenyue/boxo
that referenced
this issue
Oct 17, 2024
* Fix memory leak on BlockPresenceManager Limit BlockPresenceManager map growth be doing the following: - Use nil map when BlockPresenceManager CID map is empty. - Delete peer map when from CID map when peer map is empty. - Remove peers from BlockPresenceManager when peers are pruned from session. This allows GC to free memory when maps in BlockPresenceManager become empty. Additional improvement: Do not look for lists of keys or peers in empty maps; return early instead. Fix for issue ipfs#574
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Recently, I observed that when using bitswap for fetching data, there is a large amount of memory consumption and no release after downloading.
I use pprof and find most heap is occupied here.
boxo/bitswap/client/internal/blockpresencemanager/blockpresencemanager.go
Line 40 in 89bceff
I try to print stat after
bpm.presence[c] = make(map[peer.ID]bool)and I foundbpm.presencewould delete onBlockPresenceManager.RemoveKeys, so I added some logs.I found that there is a race condition problem here.
BlockPresenceManager.RemoveKeyshas been called and remove the CidABlockPresenceManager.ReceiveFromhas been called and createdbpm.presence[CidA]bpm.presence[CidA]is no longer used or removed, and the map is getting bigger and bigger.The text was updated successfully, but these errors were encountered: