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

Store a complete wantlist #22

Closed
Stebalien opened this issue Nov 7, 2018 · 2 comments · Fixed by #24
Closed

Store a complete wantlist #22

Stebalien opened this issue Nov 7, 2018 · 2 comments · Fixed by #24
Assignees

Comments

@Stebalien
Copy link
Member

As far as I can tell, sessions mean that we don't actually store a complete wantlist anywhere. This is extremely frustrating from a UX standpoint and makes solving #21 difficult.

@hannahhoward
Copy link
Contributor

@Stebalien

As far as I can tell, the in WantManager struct, the wl wantlist.Threadsafe field contains a complete list of wanted block CIDs, even with sessions.

Am I missing something? Here's how I get there:

  1. sessions.go and bitswap.go both call WantBlocks on the WantManager
  2. want manager calls internal method addEntries which sends a message to an internal incoming channel that gets picked up by Run function. The run function adds all incoming blocks to the wl wantlist.Threadsafe queue (I was initially confused by the distinction between bcwl & wl but I now see while sessions will prevent blocks from ending up on bcwl, all blocks wanted end up on wl based on the Run code.
  3. The internal implementation of wantlist.ThreadSafe's AddEntry method should prevent duplicate blocks from appearing on the wantlist with sessions, but still track all sessions a block is needed for.

All of this is somewhat supported by the fact that Bitswap's GetWantList method calls b.wm.wl.Entries()

Anyway, I could be super wrong -- I just want to understand what I'm missing so I know what to fix.

hannahhoward added a commit that referenced this issue Nov 13, 2018
If Bitswap receives a block that isn't in it's wantlist, is should ignore it

fix #21 fix #22
hannahhoward added a commit that referenced this issue Nov 13, 2018
If Bitswap receives a block that isn't in it's wantlist, is should ignore it

fix #21 fix #22
@ghost ghost assigned hannahhoward Nov 13, 2018
@ghost ghost added the status/in-progress In progress label Nov 13, 2018
@ghost ghost removed the status/in-progress In progress label Nov 15, 2018
@Stebalien
Copy link
Member Author

No, that sounds right. That makes this much simpler.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants