Feat/have new msg types #211
Conversation
dirkmc
force-pushed
the
feat/have-new-msg-types
branch
from
15bb89b to
a100ecf
Compare
|
ping @alanshaw @jacobheun @achingbrain Would love to get some eyes on this 🙏 |
| // the new block size | ||
| if (!existingData.haveBlock && taskData.haveBlock) { | ||
| existingData.haveBlock = taskData.haveBlock | ||
| existingData.blockSize = taskData.blockSize |
There was a problem hiding this comment.
What about updating existingData.size?
There was a problem hiding this comment.
We do that at the bottom, so as to do it one place (we also need to do it when replacing want-have with want-block)
|
|
||
| // If the task is a want-block, make sure the entry size is equal | ||
| // to the block size (because we will send the whole block) | ||
| if (existingData.isWantBlock && existingData.haveBlock) { |
There was a problem hiding this comment.
Should this be taskData instead of existingData?:
| if (existingData.isWantBlock && existingData.haveBlock) { | |
| if (existingData.isWantBlock && taskData.haveBlock) { |
If not then this feels like something that shouldn't be done here!
There was a problem hiding this comment.
This is to cover any changes that have happened in either case above.
Either
- a DONT_HAVE was replaced with a want-have / want-block
- a want-have was replaced with a want-block
In either case we need to make sure that if it's now a want-block that the entry size (the size of the response on the wire) is equal to the block size
|
|
||
| // If we now have block size information, update the task with | ||
| // the new block size | ||
| if (!existingData.haveBlock && taskData.haveBlock) { |
There was a problem hiding this comment.
haveBlock => isHave? and then isWantBlock => isHaveAndBlock
...because want-block is basically a "I have this and here it is" where as want-have is just "I have this", right? Neither of these are actually wanting things so the naming want-have and want-block is super confusing.
There was a problem hiding this comment.
It's a little confusing: This is the request queue, so these requests are want-have or want-block. When we pop stuff off the request queue we turn them into HAVE / block
src/index.js
Outdated
| // Notify listeners that we have received the new blocks | ||
| for (const block of newBlocks) { | ||
| self.notifications.hasBlock(block) | ||
| self.engine.receivedBlocks([block]) |
There was a problem hiding this comment.
No need to self anymore, but why did this get pulled out here? If it stays, could you pass newBlocks to self.engine.receivedBlocks(newBlocks) and not have to call it multiple times?
There was a problem hiding this comment.
It's a little deceptive: the async iterator passed to putMany() yields blocks that should be put (it doesn't yield blocks that have already been put). So we actually need to perform post-put actions after the whole putMany() call has completed.
You're right about the receivedBlocks() call, it's more efficient to pass all blocks at once, I've fixed 👍
|
ping @alanshaw @jacobheun |
|
Thanks @jacobheun 🎂 As far as I know we just need to do interop testing |
* feat: SortedMap * refactor: make SortedMap more efficient * feat: add request queue * feat: support want-have & want-block in engine * fix: block presences deserialization * fix: ensure blocks are in blockstore before adding them to engine request queue * fix: reduce engine queue contention * feat: listen on bitswap/1.2.0 (#212) * fix: several engine fixes * refactor: rename some things * docs: add some docs to task merger * refactor: more efficient block receive * docs: add message comments * refactor: better var names in merger
* feat: update message protobuf to support new message types * fix: case of protobuf fields * feat: include pending bytes in response message (#205) * Feat/have new msg types (#211) * feat: SortedMap * refactor: make SortedMap more efficient * feat: add request queue * feat: support want-have & want-block in engine * fix: block presences deserialization * fix: ensure blocks are in blockstore before adding them to engine request queue * fix: reduce engine queue contention * feat: listen on bitswap/1.2.0 (#212) * fix: several engine fixes * refactor: rename some things * docs: add some docs to task merger * refactor: more efficient block receive * docs: add message comments * refactor: better var names in merger * fix: merge with master
Support responses to want-block and want-have message types in decision engine
See #204 for more detail