Simultaneous HTTP downloads #121
Conversation
podusowski
changed the title
podusowski
changed the title
walkers/src/download.rs
Outdated
| let request = request_rx.next().await.ok_or(())?; | ||
| let url = source.tile_url(request); | ||
| let request = request_rx.next(); | ||
| let below_concurrent_downloads_limit = ongoing_downloads.len() < 2; |
There was a problem hiding this comment.
Is this really limit the connections?
In my world, one Client == one connection.
For http2 (tile.openstreetmaps.org supports multiplexing) this looks like obstacle.
Maybe better to have two client workers (to restrict number of connections) and limit download queue by any reasonable value (like a half of number of tiles on display)?
There was a problem hiding this comment.
In my world, one Client == one connection.
Well, this does make sense, and initially I started with two clients, but then I noticed that with one it works too. And if test test is correct, this indeed creates two connections.
I did not consider HTTP/2 (test uses HTTP/1). OSM says:
Maximum of 2 download connections.
and I do not know if this translates to 2 HTTP/2 streams or not. I have a gut feeling that it does though.
There was a problem hiding this comment.
I have a gut feeling that it does though.
I think different because http/2 streams not require system resources (like port), and they not leave many zombie-connections (in state CLOSE_WAIT).
I think we can forget about http/1 because osm.org uses http2. This can be checked using this site: https://tools.keycdn.com/http2-test or curl:
curl -v -v -v https://tile.openstreetmap.org/18/187598/121559.png -o /dev/null 2>&1 | grep -i http2
* Using HTTP2, server supports multiplexing
About testing many connections in current way:
TCP sockets take to change status: so far first client will idle in state CLOSING/FIN_WAIT/CLOSE_WAIT, next client will download the tile. Next, the roles will change and server always assume that more are connected to him if I understand correctly how TCP works.
There was a problem hiding this comment.
watch -n 1 "bash -c 'netstat -aptun| grep target'"
says what only one stable connection is used in multi-download branch
There was a problem hiding this comment.
OK, I guess I need to do more thinking on this. In the meanwhile, I have checked how Leaflet does this. I've disabled both HTTP/2 and HTTP/3.
There was a problem hiding this comment.
this looks like 6 connections in parallel
There was a problem hiding this comment.
Seems consistent with the statement over here: seanmonstar/reqwest#1517 (comment)
Well then, the approach I've taken is completely wrong. This limit needs to be enforced by reqwest/hyper, not here.
There was a problem hiding this comment.
similar result: 6 permanent connections to tile.osm.org (151.101.*.*) and 6 parallel downloads
There was a problem hiding this comment.
Setting the limit to 6 seems to give pretty pleasing experience. If reqwest can do http/2 (which I believe it can), it should use a single connection anyway, so it should meet the OSM rules.
| @@ -0,0 +1,17 @@ | |||
| [package] | |||
There was a problem hiding this comment.
I want to get rid of Mockito, and use this crate instead. Once it can handle all Walkers' use cases, I will create a new repo for it.
It's a quite big PR, because I really wanted to test simultaneous downloads, especially that we don't start more than two. Unfortunately, neither
mockito, nor any other HTTP mocking library seems to have an option to deffer responding to incoming HTTP request. So I've written a new one. :)