move findproviders out of critical path #1290
Conversation
|
It looks like the reprovider worker needs to move into the wantmanager... i dont want to have to lock around the wantlist to protect that. |
whyrusleeping
changed the title
|
thinking about this some more, bitswap needs to have a way to decide when it needs to connect to providers for a given key on its wantlist. This isnt simple to do... |
|
interesting batch of failures here... indicating i've done something wrong |
| bs.counterLk.Lock() | ||
| bs.blocksRecvd++ | ||
| if has, err := bs.blockstore.Has(block.Key()); err == nil && has { | ||
| has, err := bs.blockstore.Has(block.Key()) | ||
| if err == nil && has { |
There was a problem hiding this comment.
i think the error is dropped here.
There was a problem hiding this comment.
right you are. not sure what should be done with one should it arrive though...
There was a problem hiding this comment.
probably depends on the error?
i think we have crappy reproviding atm. in general we don't do a good job of advertising out what we mean to advertise.
|
This PR moves the addition of new blocks to our wantlist (and their subsequent broadcast to the network) outside of the clientWorker loop. This allows blocks to more quickly propogate to peers we are already connected to, where before we had to wait for the previous findProviders call in clientworker to complete before we could notify our partners of the next blocks that we want. I then changed the naming of the clientWorker and related variables to be a bit more appropriate to the model. Although the clientWorker (now named providerConnector) feels a bit awkward and should probably be changed. |
|
@whyrusleeping thanks for the summary, that SGTM. |
|
in the future, ill do writeups like that for all my PRs, its helpful for everyone (myself included) |
|
@whyrusleeping we should put them in the commits so available with the history. |
whyrusleeping
force-pushed
the
feat/bitswap-speed
branch
4 times, most recently
from
9bf678c to
db8af56
Compare
|
@whyrusleeping going to need to squash db8af56 somewhere. then try running gpe maybe can do: |
This PR moves the addition of new blocks to our wantlist (and their subsequent broadcast to the network) outside of the clientWorker loop. This allows blocks to more quickly propogate to peers we are already connected to, where before we had to wait for the previous findProviders call in clientworker to complete before we could notify our partners of the next blocks that we want. I then changed the naming of the clientWorker and related variables to be a bit more appropriate to the model. Although the clientWorker (now named providerConnector) feels a bit awkward and should probably be changed. fix test assumption
whyrusleeping
force-pushed
the
feat/bitswap-speed
branch
from
db8af56 to
2ec4c9a
Compare
move findproviders out of critical path
This PR makes bitswap really fast. but the organization is a bit weird, some feedback would be nice