This repository has been archived by the owner on Jun 29, 2022. It is now read-only.
Proposed changes for initial go implementation: Partial response code & @stebalian changes #97
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
1. Remove the indirection (`RequestList`). 2. Rename `full` to `completeRequestList` and document it. I'm still not happy with the name but it's hopefully a bit less confusing. 3. Avoid abbreviated names.
Support a partial response status code so that blocks and metadata can be sent to the client without keeping them in memory
ghost
assigned 
Stebalien
approved these changes
Feb 21, 2019
There was a problem hiding this comment.
LGTM but probably needs a quick signoff from @jbenet and/or @whyrusleeping (although I don't expect any objections).
prataprc
pushed a commit
to iprs-dev/ipld-specs
that referenced
this pull request
Oct 13, 2020
Proposed changes for initial go implementation: Partial response code & @stebalian changes
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This proposal covers all proposed changes required for the initial go implementation.
It includes:
@Stebalien 's changes, because they make sense, outlined here: graphsync: rework protobufs a bit #89
One additional response code, 14, called a partial response. Here is the rationale:
The original proposal includes an acknowledgement of the request, and two terminal response success codes - one for a completed request with all content, one for a completed request with only partial content, because the server was unable to satisfy all of the potential paths covered by the requested selector.
However, in order to send all of the found blocks in a terminal response code, the server would buffer all of them in memory as it traverses the graph before sending them. Similarly, in order to verify these blocks actually match the selector requested, the client would need to keep all of them in memory as the response is processed.
The proposal for the initial go implementation, in order to optimize memory usage, is to instead have the server send blocks as soon as it finds them, as well as information about what blocks are not available as soon as it knows. Then the client will verify and return blocks to the graphsync caller as soon as it receives them.
To support this, I'm proposing adding a "partial response" code. When this response code is transmitted, the "extra" field of the response becomes metadata about what has been transmitted -- either CIDs of blocks that are included in the response, or CIDs the server knows it doesn't have, or other control messages. For the moment, the content and structure of this extra data is left intentionally ambiguous, because more potential use cases may emerge during the actual implementation of go-graphsync and the IPLD selector library. It's structure will likely be specified more concretely once an implementation has been completed.
This proposal does not address the larger potential issues with the protocol outlined in #96