Use shared peer task queue with Graphsync #119
Conversation
ghost
assigned 
hannahhoward
force-pushed
the
feat/improve-docs
branch
from
dc03ebf to
7af3e0a
Compare
Uses shared external package peer task queue in place of peer request queue. Shared by graphsync.
hannahhoward
force-pushed
the
feat/use-peer-task-queue-package
branch
from
71e05e1 to
d2a4d88
Compare
| case <-e.ticker.C: | ||
| e.peerRequestQueue.thawRound() | ||
| nextTask = e.peerRequestQueue.Pop() | ||
| e.peerRequestQueue.ThawRound() |
There was a problem hiding this comment.
How does graphsync use freeze/thaw?
There was a problem hiding this comment.
so graphsync does support cancellation, and long term it makes sense that if a request gets cancelled, we probably out to wait and see if that peer is going to send other cancel requests... especially if we move to a context where requests are being broadcast.
however, I think it makes sense to have an option to turn freezing off since at least in graphsync's current implementation it does not make a lot of sense. I think I will probably go ahead and implement that.
| @@ -289,7 +292,10 @@ func (e *Engine) addBlock(block blocks.Block) { | |||
| for _, l := range e.ledgerMap { | |||
| l.lk.Lock() | |||
| if entry, ok := l.WantListContains(block.Cid()); ok { | |||
| e.peerRequestQueue.Push(l.Partner, entry) | |||
| e.peerRequestQueue.PushBlock(l.Partner, peertask.Task{ | |||
| Identifier: entry.Cid, | |||
There was a problem hiding this comment.
I'm a bit worried about allocations here. This used to be a bit of a hot-spot until we made entries and CIDs by-value.
This is a great abstraction so I'm not going to block on that but we just need to keep it in mind.
There was a problem hiding this comment.
Yea I mean, it's still technically by-value -- just a new value that makes a new allocation :) I will keep an eye on it as this goes into production.
…ueue-package Use shared peer task queue with Graphsync This commit was moved from ipfs/go-bitswap@a32fa8a
Goals
During the course of writing go-graphsync, I ended up extracting the peerRequestQueue to be an abstract PeerTaskQueue -- essentially an arbiter between different queues of tasks distributed between peers, no caring what the actual task is (tasks just have an identifier and a priority).
The basic code for PeerTaskQueue is essentially identical to peerRequestQueue with a few changes. There is one frustrating type cast (thanks go no generics) but otherwise, this will allow any changes or improvements to this queue to propagate to both go-graphsync and go-bitswap simultaneously.