Leaflet / Leaflet.VectorGrid

Display gridded vector data (sliced GeoJSON or protobuf vector tiles) in Leaflet 1.0.0
598 stars 194 forks source link

Cancelling requests #136

Open furstenheim opened 6 years ago

furstenheim commented 6 years ago

I've noticed that when using protobuf and changing the zoom, old requests aren't cancelled. This is sometimes clogging the server if the tiles take too long and the user changes zoom quick.

If it is of any help, in leaflet tile layers this is handled by changing the src of the img and that makes the browser cancel the old request https://github.com/Leaflet/Leaflet/blob/master/src/layer/tile/TileLayer.js#L230

Of course that won't work out here, since you are doing fetch directly.

Thanks

furstenheim commented 6 years ago

I suppose it'll have to wait at least for the abort api for whatwg-fetch is merged https://github.com/github/fetch/pull/592

mo-ian-watkins commented 6 years ago

Would also find this useful

dbauszus-glx commented 6 years ago

@furstenheim Is there anything we can do to make this happen? This would increase performance significantly when generating the vector tiles dynamically.

furstenheim commented 6 years ago

@dbauszus-glx for now I'm waiting for the people from the fetch polyfill to implement the feature. I'll give it a try once they have it ready.

mo-ian-watkins commented 6 years ago

Agreed. All our tiles are built on the fly from live data that is changing frequently so sometimes the backend can be a little slow in sending the tiles if it is busy.

So you are viewing at zoom level 6, you zoom in to zoom level 7 (requests are fired) and then zoom back out again but zoom level 7 tiles then start to dribble in/block level 6 requests. If you are doing anything with the tile data once they are received it can sometimes mess up that process as well, so downstream effects like features from zoom level 7 get plotted at zoom level 6 etc. (but suggest that's more of an edge case issue)

So super up vote from me.

JamesLMilner commented 6 years ago

https://github.com/Leaflet/Leaflet.VectorGrid/pull/151 should resolve this partly. There's a way to take this a step further by actually cancelling the requests using abort signals for fetch, but browser support is a little patchy to make it worth it.

JamesLMilner commented 6 years ago

On my last note; we could potentially bundle an AbortController (i.e. abortable fetch) polyfill with Leaflet.VectorGrid? It would make sense (at least for the next year or two) as support is currently pretty low.

This one could potentially work, and I think it might dramatically improve user experience. Thoughts @IvanSanchez @tomchadwin ?

tomchadwin commented 6 years ago

This "polyfill" doesn't actually close the connection when the request is aborted, but it will call .catch() with err.name == 'AbortError' instead of .then().

Will this catch anything beyond what you did in #151? I agree that we should implement abort in time, but is my understanding that the issue isn't just browser support, but also support at the API end? Without the latter, I'm not sure how much this improves things, and I have no idea how common API support for abort is. If it's there, it's definitely worth doing.

JamesLMilner commented 6 years ago

@tomchadwin ah, yes that is a pain. Panning/zooming in turn makes a lot requests which is actually the bottleneck as browsers like Chrome are limited to 6 (fact check that!) concurrent requests. Closing the actual HTTP request prioritises the most recent/relevant tile requests. We could implement it behind feature detection, as shortly it will be supported by Chrome (next release). This in turn would mean Firefox, Edge and Chrome support which should cover about 60%+ of users with minimal impact on everyone else.

tomchadwin commented 6 years ago

Looks like a plan. My guess is that API maintainers will want to implement it ASAP anyway, given the saving on traffic.