ipfs-cluster / ipfs-cluster

Pinset orchestration for IPFS
https://ipfscluster.io
Other
1.49k stars 289 forks source link

Avoid recontacting peers that never authorized us #849

Open hsanjuan opened 5 years ago

hsanjuan commented 5 years ago

Describe the feature you are proposing

If making an RPC request to a peer results in Authorization denied, we should avoid recontacting that peer on subsequent requests. whyrusleeing/timecache may serve as example: if we get an authorization error, we put the peer in it for a while. Before making a request we check if a peer is in the timecache and, if so, skip it.

Possibly worth to allow a hidden configuration here, with 0 to disable. But the default time would be quite high.

Additional context

Right now authorization errors are ignored when doing things like Status and other broadcast calls and we simply ignore those answers in the final result. Repeteadly contacting those peers on subsequent requests only makes things slower (peers are very unlikely to have "allowed" this peer).

Along with this we might choose to show authorization errors when they first happen, explaining that we will not try to recontact that peer for X hours.

Initially, this would apply only for broadcast calls in cluster.go.

kishansagathiya commented 5 years ago

I am thinking of creating a wrapper for rpcClient which would check the cache before requests are made.

kishansagathiya commented 5 years ago

Wonder if it should be added to the rpc library instead of cluster? I am more inclined to this

hsanjuan commented 5 years ago

This is not meant to affect all rpc requests, only broadcast-rpc ones (sync etc). A wrapper for the Multicall function is probably the cleaner way but it wastes some allocations and memory (the calling method would allocate responses for calls that are not going to be made). This would be An object with the timecache, the original rpcclient and the multicall method. In cluster, not in the rpc library.

Let's try that way first.

kishansagathiya commented 5 years ago

This would be An object with the timecache, the original rpcclient and the multicall method.

That sound like a wrapper for rpc client, but in cluster.

hsanjuan commented 5 years ago

That sounds like a wrapper for the multicall method.