Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Discussion: Support planned pausing #344

Open
hannahhoward opened this issue Feb 4, 2022 · 3 comments
Open

Discussion: Support planned pausing #344

hannahhoward opened this issue Feb 4, 2022 · 3 comments
Assignees
Labels
need/triage Needs initial labeling and prioritization

Comments

@hannahhoward
Copy link
Collaborator

hannahhoward commented Feb 4, 2022

Goals

Right now, our pause mechanism in retrieval requires inspecting every block and telling graphsync whether you want to pause, and executing this synchronously. It is less than ideal to say the least. A better mechanism would be for the retrieval protocol to tell graphsync ahead of time when to pause. This becomes especially neccesary if we move to a mode where the payments might be happening in a different process -- and checking with them is an HTTP call (that we definitely would prefer not to make on each call)

Note that we could implement this entirely in go-data-transfer without these changes in Graphsync. see filecoin-project/go-data-transfer#297

That said, this seems like it'd make it easier.

For now, I see this working like as follows:

Add an action for the IncomingRequestHook:

type PlannedPause struct {
    PauseAfterBytesSent uint64
    PauseIfComplete bool
    OnPause(actualSent uint64) []graphsync.ExtensionData
}

type IncomingRequestHookActions interface {
    // maintain existing actions
    PlanPause(PlannedPause)
}

type UpdateRequestHookActions interface {
   // maintain existing actions
   PlanPause(PlannedPause)
}

Note that multiple hooks may create planned pauses so we may need to keep a sorted array of pauses.

Logic is after each block, check pause requests, if any are before the current pause point, call on OnPause for all that have passed, add the extensions to the response, and pause it.

@hannahhoward hannahhoward added the need/triage Needs initial labeling and prioritization label Feb 4, 2022
@rvagg
Copy link
Member

rvagg commented Feb 4, 2022

what does it mean to "pause if complete"? isn't that just a stop? or is that there just to provide an option to get that callback when complete?

@hannahhoward
Copy link
Collaborator Author

I mean it's considering the request "not done" -- this is one of the more bizarre aspects of the retrieval protocol, where the request is "stopped" pending the last payment, even though the other party has all their data and can easily walk away. Essentially it allows the retriever to send one last request for payment.

@hannahhoward hannahhoward changed the title Support planned pausing Discussion: Support planned pausing Feb 4, 2022
@willscott
Copy link
Collaborator

  • in that OnPause callback how would you signal an 'actually don't pause'?
  • is having one of these calculated at each future byte offset particularly efficient either?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need/triage Needs initial labeling and prioritization
Projects
None yet
Development

No branches or pull requests

3 participants