Skip to content
This repository has been archived by the owner on Feb 1, 2023. It is now read-only.

fix(decision): cleanup request queues #116

Merged
merged 2 commits into from May 3, 2019
Merged

Conversation

hannahhoward
Copy link
Contributor

Goals

Remove memory leak by making sure when request queues are idle that they are removed
fix #112

Implementation

Unfortunately, cause the peer request queue uses a PriorityQueue which only has a Pop method (and no Remove method) the only point you can remove a partner is the moment it's already popped off the priority queue before you put it back on. This adds code to check and remove the partner. Also since partners do not have a peer ID you have to do an O(N) search through the list of active partners to find the peer ID -- I don't forsee this happening so much that it's necessary to improve but it could be added.

Also adds a test to make sure it happens

For discussion

  • Is this the memory leak you're thinking of in Queue Memory Leak #112?
  • Does this approach make sense?
  • Does the remove need to be optimized by adding the peer id to the activepartner struct?

Make sure when request queues are idle that they are removed

fix #112
@ghost ghost assigned hannahhoward May 2, 2019
@ghost ghost added the status/in-progress In progress label May 2, 2019
@hannahhoward hannahhoward moved this from In progress to Needs Review in Bitswap Prioritization Backlog May 2, 2019
@Stebalien
Copy link
Member

We should totally add the peer ID to the activePartner struct. Disregarding performance, it'll probably just make things simpler.

@Stebalien
Copy link
Member

Otherwise, LGTM.

Add a peer id to an active partner queue
@hannahhoward
Copy link
Contributor Author

@Stebalien added the peer ID -- going to merge later today unless I hear otherwise.

@Stebalien Stebalien merged commit 61f1223 into master May 3, 2019
Bitswap Prioritization Backlog automation moved this from Needs Review to Done May 3, 2019
@ghost ghost removed the status/in-progress In progress label May 3, 2019
@Stebalien
Copy link
Member

Thanks!

@Stebalien Stebalien deleted the bugs/queue-memory-leak branch May 3, 2019 16:45
@momack2 momack2 added this to Done in ipfs/go-ipfs May 9, 2019
Jorropo pushed a commit to Jorropo/go-libipfs that referenced this pull request Jan 26, 2023
fix(decision): cleanup request queues

This commit was moved from ipfs/go-bitswap@61f1223
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

Queue Memory Leak
2 participants