Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: use bitswap fetch queue #216

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

dirkmc
Copy link
Contributor

@dirkmc dirkmc commented Mar 5, 2020

Allow multiple concurrent requests for the same block

achingbrain and others added 3 commits March 4, 2020 19:06
When we call `blockstore.putMany`, some implementations will batch
up all the `put`s and write them at once.  This means that
`blockstore.has` might not return `true` for a little while - if
another request for a given block comes in before `blockstore.has`
returns `true` it'll get added to the want list.  If the block then
finishes it's batch and finally a remote peer supplies the wanted
block, the notifications that complete the second block request
will never get sent and the process will hang idefinately.

The change made here is to separate the sending of notifications
out from putting things into the blockstore.  If the blockstore has
a block, but the block is still in the wantlist, send notifications
that we now have the block.
@dirkmc dirkmc requested a review from achingbrain March 10, 2020 18:11
@achingbrain achingbrain force-pushed the fix/race-condition-when-requesting-the-same-block branch from 2d8c892 to 57185ea Compare May 4, 2020 16:24
@achingbrain achingbrain force-pushed the fix/race-condition-when-requesting-the-same-block branch from 57185ea to 1fc09ed Compare May 19, 2020 09:18
@achingbrain
Copy link
Member

My feeling is that this module has too many places that hold state, so any solution should not add new places for state to hide.

Base automatically changed from fix/race-condition-when-requesting-the-same-block to master May 27, 2020 15:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants