Closed hannahhoward closed 4 years ago
I'm behind on graphsync unfortunately. This sounds like a reasonable idea but the mechanics are mostly lost on me so I can't form a strong opinion either way.
Is this message only valid in an existing graphsync connection? The way I'm picturing it working from what you've written is that you're in the middle of a receive and decide to alter the request so you send this update, which also contains the updated request data and then the sender can adjust mid-sync what it's sending? Then in addition, the pause feature is added so the sender can send a "hold up, this isn't working, you need to send me an update to make it work again", with the obvious cause being lack of $ to complete the sync. Correct?
@rvagg yes, you understand the flow correctly. Similar to a cancel, this message is only valid in an existing graphsync connection.
@Stebalien @warpfork is there any chance you can give this a quick glance? I am probably going to go ahead an implement it experimentally in go-graphsync. But it would be nice to stay aligned with spec.
Goals
The update to GraphSync is intended to support future mechanisms for modifying an in progress request. Rather than define a complex update protocol, we simply add an update boolean to the request, indicating a request is an update to an already in progress request, and delegate what the update actually is to whatever extensions are shared between requestor and responder. We can promote actual types of updates to the core request structure as we determine them to be so useful they need to be standard across all GraphSync clients. However, at the moment, I do not any such update mechanisms are defined well enough to merit that. There will probably be a period of experimentation as we determine how to best spread out graphsync requests among multiple responders to quickly replicate IPLD graphs, and I think it's best to delegate to extensions until we determine effective ways to do this, so as not to break the core protocol. (thing we may want to add: way to query for supported extensions)
In addition, we define a paused response code -- your request is in progress, it hasn't error-ed, but it's halted pending additional information from you that you can send in an update request. This is useful for layering any kind of system for metering requests based on payment on top of graphsync.
For discussion
It is possible we could get away with not adding this boolean? if a request ID is received from a given peer by a responder, that already has a request with that ID in progress for that peer, arguably that implies an update? Seems like hiding that information as an implied result of the request already existing is not helpful
Alternatively, rather than adding another boolean, we could add a request code, similar to an HTTP request code, and remove the cancel boolean and for now define request codes like: 10 - New request 20 - Update Request 30 - Cancel Request (I'm just spitballing numbers here -- we could use other ones)