Store other formats to allow better cache usage #14
Conversation
| if self.store and coord.zoom <= 20: | ||
| self.io_pool.apply_async( | ||
| async_store, (self.store, tile_data, coord, format)) | ||
|
|
||
| # since we're a request for all, enqueue the coordinate to | ||
| # ensure other formats get processed | ||
| if self.sqs_queue and coord.zoom <= 20: |
There was a problem hiding this comment.
Do we only want to do this when layer_spec == 'all', or is it worth doing this always? I guess what I'm trying to figure out is: If we add something to the TOI, should we always enqueue work to render the tile as if the tile were originally seeded?
The way the code is at the moment, we always add the tile to the TOI (e.g: perhaps it's a buildings layer request), and we'd store the JSON and (if any) other format that was requested, but not all the formats.
There was a problem hiding this comment.
Yes that's right. But if we always enqueued, the scenario that wouldn't work out so well would be consistently receiving requests for a custom layer, eg buildings. We would enqueue tiles for every request, even though nothing would be changing.
I think it makes sense to enqueue when we receive a request for something that would have gotten served by the cache. Currently it's just the all layer, but we talked about starting to store tiles for the popular layers like buildings. If we did, then we would want to enqueue and store when we receive requests for these as well.
I'm inclined to leave it the way it is currently, and when code gets added to start storing other layers on s3, update this bit as well to reflect that, maybe through some explicit configuration in case we want to add more combinations in the future.
Thoughts?
There was a problem hiding this comment.
If the (JSON) tile data was in the store, then we'd have returned on line 257, wouldn't we? In which case, once we store the JSON and enqueue a re-render, we shouldn't end up in this branch more than once per tile (modulo it concurrently happening on several tileservers, and us potentially returning None from tile fetching because there was an error rather than no tile).
So, while a request for, say, buildings would end up at the tileserver again, it should be able to generate it from the JSON we stored last time rather than come through this branch again? Which my very circuitous way of trying to say I think it's safe to enqueue anything that gets added to the TOI.
There was a problem hiding this comment.
I think I follow what you mean now. Does it look better with d95a5fd?
|
👍, looks good to me! |
Store other formats to allow better cache usage
Depends on tilezen/tilequeue#66
@zerebubuth could you review please?