Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cancelling requests #136

Open
furstenheim opened this issue Dec 26, 2017 · 10 comments
Open

Cancelling requests #136

furstenheim opened this issue Dec 26, 2017 · 10 comments

Comments

@furstenheim
Copy link

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
Copy link
Author

I suppose it'll have to wait at least for the abort api for whatwg-fetch is merged JakeChampion/fetch#592

@mo-ian-watkins
Copy link

Would also find this useful

@dbauszus-glx
Copy link

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

@furstenheim
Copy link
Author

@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
Copy link

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
Copy link
Contributor

#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
Copy link
Contributor

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
Copy link
Collaborator

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
Copy link
Contributor

JamesLMilner commented Mar 13, 2018

@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
Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants