Add support for IPLD prime's budgets feature in selectors #235
Conversation
add options to configure the maximum number of allowed links to traverse on the requestor and the responder
hannahhoward
force-pushed
the
feat/request-budgets
branch
from
91a6ceb
to
9776ae5
Compare
| @@ -27,7 +27,7 @@ require ( | |||
| github.com/ipfs/go-peertaskqueue v0.2.0 | |||
| github.com/ipfs/go-unixfs v0.2.4 | |||
| github.com/ipld/go-codec-dagpb v1.3.0 | |||
| github.com/ipld/go-ipld-prime v0.12.0 | |||
| github.com/ipld/go-ipld-prime v0.12.3-0.20210929125341-05d5528bd84e | |||
There was a problem hiding this comment.
can we ask for a tag of this?
There was a problem hiding this comment.
will do on monday
There was a problem hiding this comment.
before I commit go-graphsync v0.10.0 final
| requestExecutor := executor.NewExecutor(requestManager, incomingBlockHooks, asyncLoader.AsyncLoad) | ||
| responseAssembler := responseassembler.New(ctx, peerManager) | ||
| peerTaskQueue := peertaskqueue.New() | ||
| responseManager := responsemanager.New(ctx, linkSystem, responseAssembler, peerTaskQueue, requestQueuedHooks, incomingRequestHooks, outgoingBlockHooks, requestUpdatedHooks, completedResponseListeners, requestorCancelledListeners, blockSentListeners, networkErrorListeners, gsConfig.maxInProgressIncomingRequests, network.ConnectionManager()) | ||
| responseManager := responsemanager.New(ctx, linkSystem, responseAssembler, peerTaskQueue, requestQueuedHooks, incomingRequestHooks, outgoingBlockHooks, requestUpdatedHooks, completedResponseListeners, requestorCancelledListeners, blockSentListeners, networkErrorListeners, gsConfig.maxInProgressIncomingRequests, network.ConnectionManager(), gsConfig.maxLinksPerIncomingRequest) |
There was a problem hiding this comment.
these have gotten pretty unwieldy. we probably want options or some other pattern for these constructors at some point
There was a problem hiding this comment.
they're initialized by the code itself, so I'd probably lean towards a struct, but I also want to move this whole package to an internal directory :) but yes for later
requestmanager/server.go
Outdated
| fmt.Println(responseMetadata) | ||
| if len(filteredResponses) > 0 { | ||
| fmt.Println(filteredResponses[0].Status()) | ||
| } |
| requestHooks RequestHooks | ||
| responseAssembler ResponseAssembler | ||
| linkSystem ipld.LinkSystem | ||
| maxLinksPerRequest uint64 |
There was a problem hiding this comment.
can we be clear/consistent about whether 0 means fail immediately or no limit?
In this file it seems to mean 'no limit', but in the test in traverser_test "errors correctly, no budget" link budget is set to 0, and the traversal then fails due to being out of budget.
There was a problem hiding this comment.
in means no limit. The traverser test gets a nil value for the budget, meaning no budget, when this value is 0
There was a problem hiding this comment.
i think what i'm asking for is a comment to that effect :)
termination from remote peers was not properly handling blocks included up to termination, and could include unpredictable amounts in the response channel
hannahhoward
force-pushed
the
feat/request-budgets
branch
from
fc4fc63
to
a1036c8
Compare
Goals
Provide absolute limits on links traversed for incoming and outgoing requests
Implementation