ipfs / go-graphsync

Initial Implementation Of GraphSync Wire Protocol
Other
100 stars 38 forks source link

Remove request allocation backpressure #241

Closed hannahhoward closed 2 years ago

hannahhoward commented 3 years ago

What

Request allocation backpressure was thrown in quite ad-hoc to respond to miner concerns about memory usage. (as opposed to response allocation backpressure which has seen pretty careful development over a while).

I have been aware of potential issues with it for some time, but have failed to properly address them as they require a fair amount of plumbing to get right.

Given the presences of outgoing request limits, it seems prudent to remove allocation backpressure on the requestor side as a first step.

How

hannahhoward commented 3 years ago

Follow-up: at this point, we have simultaneous outgoing requests limits, where we didn't before. The memory backpressure on the requestor side has been a source of issues and was put in somewhat haphazardly at a time when we didn't have these -- is it neccesary? should it be removed entirely?

rvagg commented 2 years ago

I believe this was introduced primarily in https://github.com/ipfs/go-graphsync/pull/205/commits/9171ce6bccd62bf4303c12edd07e278713e6da06