ipfs: Remove concurrency limit, rely on rate limit (#4570)

* ipfs: Remove concurrency limit, rely on rate limit

The concurrency limit, because it uses a semaphore, had unfortunate
interactions with the use of `call_all` in the polling monitor.
`CallAll` can internally have requests which are holding the semaphore,
so it must be polled regularly to check on those requests.
If the task holding the `CallAll` hangs on some other future, that
can lead to a deadlock.

In our case, it can hang in the `response_sender.send((id, response)).await`,
if the channel is full and the subgraph runner takes a long time to check it.
This could be fixed, but the footgun would remain so it seemed best to
rely only on the rate limit which can't deadlock.

* polling monitor: Log when not found

Author: leoyvens

core/src/polling_monitor/ipfs_service.rs

@@ -17,7 +17,7 @@ pub fn ipfs_service(
     client: IpfsClient,
     max_file_size: u64,
     timeout: Duration,
-    concurrency_and_rate_limit: u16,
+    rate_limit: u16,
 ) -> IpfsService {
     let ipfs = IpfsServiceInner {
         client,
@@ -26,8 +26,7 @@ pub fn ipfs_service(
     };
 
     let svc = ServiceBuilder::new()
-        .rate_limit(concurrency_and_rate_limit.into(), Duration::from_secs(1))
-        .concurrency_limit(concurrency_and_rate_limit as usize)
+        .rate_limit(rate_limit.into(), Duration::from_secs(1))
         .service_fn(move |req| ipfs.cheap_clone().call_inner(req))
         .boxed();
 

core/src/polling_monitor/mod.rs

@@ -161,6 +161,8 @@ where
 
                     // Object not found, push the id to the back of the queue.
                     Ok((id, None)) => {
+                        debug!(logger, "not found on polling"; "object_id" => id.to_string());
+
                         metrics.not_found.inc();
                         queue.push_back(id);
                     }

docs/environment-variables.md

@@ -78,8 +78,8 @@ those.
   may use (in bytes, defaults to 256MB).
 - `GRAPH_MAX_IPFS_CACHE_SIZE`: maximum number of files cached (defaults to 50).
 - `GRAPH_MAX_IPFS_CACHE_FILE_SIZE`: maximum size of each cached file (in bytes, defaults to 1MiB).
-- `GRAPH_IPFS_REQUEST_LIMIT`: Limits both concurrent and per second requests to IPFS for file data
-   sources. Defaults to 100.
+- `GRAPH_IPFS_REQUEST_LIMIT`: Limits the number of requests per second to IPFS for file data sources.
+   Defaults to 100.
 
 ## GraphQL
 

graph/src/env/mappings.rs

@@ -49,7 +49,7 @@ pub struct EnvVarsMapping {
     /// bytes). Defaults to 256 MiB.
     pub max_ipfs_file_bytes: usize,
 
-    /// Limits both concurrent and per second requests to IPFS for file data sources.
+    /// Limits per second requests to IPFS for file data sources.
     ///
     /// Set by the environment variable `GRAPH_IPFS_REQUEST_LIMIT`. Defaults to 100.
     pub ipfs_request_limit: u16,