Optimize IPFS retries #5698
Optimize IPFS retries #5698
Conversation
| #[tokio::test] | ||
| async fn new_unchecked_creates_the_client_without_checking_the_gateway() { | ||
| let server = mock_server().await; | ||
|
|
||
| IpfsGatewayClient::new_unchecked(server.uri(), &discard()).unwrap(); | ||
| } | ||
|
|
||
| #[tokio::test] |
There was a problem hiding this comment.
did you replace these with something else?
There was a problem hiding this comment.
These tests were for the .can_provide() method used by the IPFS client pool to select the appropriate client for requests, but this is no longer needed, so the method and its tests have been removed.
I also removed tests for non-retriable errors, because from the perspective of IPFS clients, all errors are now retriable, except for deterministic errors that are not implemented yet.
| &path, | ||
| self.max_file_size, | ||
| Some(self.timeout), | ||
| RetryPolicy::None, |
There was a problem hiding this comment.
Any way we can make sure this is tested to prevent regression?
There was a problem hiding this comment.
The IPFS client's internal retries caused the high number of IPFS requests here, because the polling monitor has it's own infinite retries with priorities. By disabling the internal retries of the IPFS client, we allow the polling monitor to behave as expected and rely on it's retry functionality.
There was a problem hiding this comment.
Understood. It's kind of hard to differentiate the different layers of retries. I'm happy to leave it as is since now we know it won't retry on the client level.
b2aee12 to
d56ce13
Compare
* ipfs: optimize retries * ipfs: add more tests
This PR reduces the number of unnecessary IPFS retries by requiring an explicit retry policy, so that different components of the graph-node can decide to use different types of retries, or not use retries at all.
Closes #5696
Other improvements:
IPFS client pool now reuses the initial response instead of doubling the number of requests.
Future improvements:
This PR includes the first steps in defining deterministic IPFS errors so that some retries can be eliminated altogether. This functionality is not part of this PR as it requires more thought and testing and will be added in a future PR.