-
Notifications
You must be signed in to change notification settings - Fork 108
Conversation
This PRs on top of #74 |
BTW: on another look, this doc is crap right now. (sorry!) I needs a lot of work to be useful as a proposal. But i think it is meaningful to people who have implemented Bitswap, and they may be able to write an implementation from it. |
See the video presentation here: https://drive.google.com/file/d/1NbbVxZQFKXwW6mdodxgTaftsI8eID-c1/view |
|
||
- Give me the nodes for the path `<hash>/a/b/c/d/e/f/g` | ||
|
||
### Loading a large video optimizing for playback and seek |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem with these approaches that say "give me [indefinite number] of blocks" is that they don't parallelize well.
The client making a request of indefinite size, by definition, doesn't know the size and shape of the request. At each layer of depth we lose the ability to grab that content from other peers. Depending on the shape of the graph and the network conditions of the different peers this approach is rarely the fastest and for many edge cases ends up being incredibly punitive. If we instead just focus on requesting more blocks concurrently once we have the knowledge to do so we actually gain concurrency vectors at each depth of the tree and can optimize our requests across many peers.
The inability to request a large number blocks serially from a single peer should not be a source of performance issues right now. Currently, we think the largest bottlenecks are:
- inability to grab a deep node without a roundtrip for every branch.
- lack of parallelization when grabbing blocks from a peer (which, on the JS side at least, is apparently just a bug).
- lack of storage knowledge for each CID (basically, we need to note when we have the full graph for a CID so that when the client is syncing an updated graph it knows if it needs to continue traversing down a particular path).
The rough outline of a fix for this was:
- Create a new RPC call that asks for the merkle proof of a given PATH.
- Request large numbers of blocks in parallel from known peers (I think someone said this isn't actually a problem on the Go side).
- Create a layer on top of BlockStore that has a boolean index for whether or not the local repo has the entire graph referenced by that CID.
That's enough to drastically improve over what we have today and avoids a lot of very complex logic when trying to spread out these indefinite requests over multiple peers.
|
||
message RequestList { | ||
repeated Request reqs = 1; // a list of requests | ||
bool full = 2; // whether this is the full RequestList. default to false, for incremental requests. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should probably call this something like replaceRequests
.
bool cancel = 5; // whether this cancels a request | ||
} | ||
|
||
message RequestList { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can probably remove this indirection and embed the requests direly in the message.
- (3) The `responder` fulfills the request by sending content to the `requester` (the `response`) . | ||
- (4) The `responder` and `requester` can terminate the request process at any time. | ||
- Notes: | ||
- We are explicitly avoiding the `client-server` terminology to make it clear that `requester` and `responder` are "roles" that any peer might play, and to avoid failling in the two-sided client-server model of the web.. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❤️ 💟 ❤️ Words matter. ❤️ 💟 ❤️
Merging as this is the current "working draft". |
Proposal: GraphSync (B)
This is an alternative proposal for graphsync.
Status of this proposal: