Skip to content
This repository has been archived by the owner on Jun 29, 2022. It is now read-only.

RFC: Graphsync Response Metadata #111

Closed
wants to merge 1 commit into from
Closed

Conversation

hannahhoward
Copy link
Collaborator

@hannahhoward hannahhoward commented Apr 11, 2019

This PR serves as a follow up to #97

Specifically that PR states:


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.


Having progressed farther on the implementation of GraphSync, I want to surface the actual structure of the "extra data" sent back in a partial response -- what I call response metadata, and ask whether it should specifically become part of the specification. Moreover, I recently learned that the extra field is intended to transmit side channel protocol information, such as payments, and I wonder whether using it to transmit this bit of metadata is overloading the purpose of the field. I'm wondering if it makes better sense to add one more field to a response called metadata and put it there.

@hannahhoward
Copy link
Collaborator Author

@Stebalien @warpfork thoughts on whether I should just move this forward?

@warpfork
Copy link
Contributor

warpfork commented May 3, 2019

I don't think I have any opinion on this one way or the other.

@Stebalien
Copy link
Contributor

Moreover, I recently learned that the extra field is intended to transmit side channel protocol information, such as payments, and I wonder whether using it to transmit this bit of metadata is overloading the purpose of the field.

The response status tells the client how it should interpret the extra field so this sounds like something that would go in the extra field (unless I'm misunderstanding the issue).

For example, the server might send a response with status 12 and payment details in extra.

@Stebalien
Copy link
Contributor

However, I'm wondering if we should make the extra field work more like TLS extensions and make it a map of extension-type -> extension-value.

  • Status: Something you need to handle. Additional info may be in extra.
  • Extra: Additional information that you can often ignore.

Add a metadata field to a graphsync response and provide information about the IPLD schema it
contains
@vmx
Copy link
Member

vmx commented Oct 14, 2019

Rebased (and remove some trailing spaces and a trailing comma).

@Stebalien: Hannah told me it's good to be merged. Please let me know if you agree.

@hannahhoward
Copy link
Collaborator Author

@Stebalien also see: #204 which is a follow on to this.

I actually agree with your comment above about the structure of extra and I've made a PR to that effect.

I still feel metadata is different, as it communicates information about the response itself and neccessary to writing a performant client.

@hannahhoward
Copy link
Collaborator Author

Closing for now, final PR incoming.

@Stebalien Stebalien deleted the rfc/metadata-field branch October 24, 2019 21:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants