sdk: Allow to limit the number of concurrent network requests #3625
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3625 +/- ##
==========================================
- Coverage 84.22% 84.21% -0.02%
==========================================
Files 256 256
Lines 26551 26565 +14
==========================================
+ Hits 22363 22372 +9
- Misses 4188 4193 +5 ☔ View full report in Codecov by Sentry. |
|
(Also would be nice to test, but likely it's a lot of bother for little value, so not mandatory. For this one time :)) |
| async fn acquire(&self) -> MaybeSemaphorePermit<'_> { | ||
| match self.0.as_ref() { | ||
| Some(inner) => { | ||
| // ignoring errors as we never close this |
There was a problem hiding this comment.
What kind of errors do you mean here?
There was a problem hiding this comment.
If the semaphore has been closed, this returns an AcquireError.
This only ever happens when there the Semphore was explicitly closed, which this MaybeSemaphore doesn't ever do. So the error can never occur and can be ignored.
There was a problem hiding this comment.
Can you please update the comment to make this clear?
There was a problem hiding this comment.
New test failure when the test is running in the super-slow code coverage run: https://github.com/matrix-org/matrix-rust-sdk/actions/runs/9760610496/job/26939845059?pr=3625#step:9:1663
| async fn acquire(&self) -> MaybeSemaphorePermit<'_> { | ||
| match self.0.as_ref() { | ||
| Some(inner) => { | ||
| // ignoring errors as we never close this |
There was a problem hiding this comment.
Can you please update the comment to make this clear?
|
@bnjbvr yeah, looks like that 300ms wasn't long enough. I suppose the only way to fix it is to increase it to 1s again for that second test :( . |
bnjbvr
changed the title
Add a new
max_concurrent_requestsparameter in theRequestConfiglimits the number of http(s) requests the internal sdk client issues concurrently (if > 0). The default behavior is the same as before: there is no limit on concurrent requests issued.This is especially useful for resource constrained platforms (e.g. mobile platforms), and if your pattern might lead to issuing many requests at the same time (like downloading and caching all avatars at startup).
Signed-off-by: Benjamin Kampmann [email protected]