Elevation tile on demand fetching

[Neyromancer]

standalone on-demand elevation tile loader which allows to download elevation tiles from remote storage.

• added service add_elevation_on_demand - allows to fetch elevation from remote server based on provided tile and config
• added tests to test add_elevation_on_demand functionality

[kevinkreiser]

Excellent I've been wanting to do this forever! I'll take a look in the next few days

[nilsnolde]

can you elaborate a little on what "on demand" means? didn't look too much into the changes, but it seems like it's mostly a new executable, from what I can see duplicating what we just added to valhalla_build_elevation python script (#3318 ), i.e. fetching elevation only for graph tile coverage? can you please say a few words how valhalla_add_elevation is adding functionality compared with valhalla_build_elevation @Neyromancer ? for now it's pretty confusing to me, but I might miss smth..

[nilsnolde]

ah think I got it.. was just adjusting a planet build to only cover the graph with elevation and in the current situation I have to do the build in stages, stopping after the graph initially built, then run the py script to download the elevation, then start again at enhance. with this PR I guess you don't have to do multi-stage builds right? argh, then I wish I would've waited 2 more weeks before implementing exactly that in the py script, would've saved me a few hours 😅 much simpler now, thanks for that!

[merkispavel]

ah think I got it.. was just adjusting a planet build to only cover the graph with elevation and in the current situation I have to do the build in stages, stopping after the graph initially built, then run the py script to download the elevation, then start again at enhance. with this PR I guess you don't have to do multi-stage builds right? argh, then I wish I would've waited 2 more weeks before implementing exactly that in the py script, would've saved me a few hours 😅 much simpler now, thanks for that!

The general idea is not to fetch unnecessary elevation tiles(e.g around the oceans) and this is what both valhalla_build_elevation.py script and this PR changes are good at.

1. For small input pbfs c++ on demand fetching might be a better choice(as you noticed)
2. Though I'm not sure about the whole-planet-build. This is where python script can win in the sense of performance compared to lazy on demand fetching: the script fetches all at once(rather than many individual fetches) so I guess it might be "cheaper"

Speaking of new valhalla_add_elevation binary. We deal with spot instances. So I'm thinking about caching the progress of the elevation stage not to lose progress when an instance dies. This is where I would need valhalla_add_elevation binary

for tile in valhalla_tiles:
   if tile not processed:
    ./valhalla_add_elevation --graph-tile tile
    cache tile

We can probably move valhalla_add_elevation to the private fork not to burden public contributors to support it as it's usage is pretty specific. As you wish guys

[nilsnolde]

right, thanks for the explanation @merkispavel ! maybe for planet py might be slightly cheaper. eventually I hope I don't have to download the full planet for a while though;)

IMHO valhalla_add_elevation is mildly specific but not a lot of code, could be useful for others and burden is minimal, so from me 👍

[kevinkreiser]

then I wish I would've waited 2 more weeks before implementing exactly that in the py script, would've saved me a few hours  much simpler now, thanks for that!

@nilsnolde yeah this is what i was trying to suggest in that recent pr from @molind , its great to see someone just knock this out

[kevinkreiser]

Though I'm not sure about the whole-planet-build. This is where python script can win in the sense of performance compared to lazy on demand fetching: the script fetches all at once(rather than many individual fetches) so I guess it might be "cheaper"

@merkispavel im not sure about that, i still think this implementation can win because we coalesce requests for the same tile to the same future and we write those tiles to the elevation cache dir so that subsequent requests will h it the directory first. the only way we could be even better is if we spawn a background thread that pre-warms the elevation directory cache while other parts of the tile building process are running

[merkispavel]

1. I'm not sure we need concurrent fetching on the sample::get_all level
2. I would think about merging sample::get_from_remote method with cache_t class so that we could just call cache->tile(index) and all fetching/saving/caching things are hidden in the cache class.
   Maybe we can wrap existing cache_t class and enhance it with fetching logic

[kevinkreiser]

In my opinion this is so incredibly similar to the graph tile fetching we should move that implementation into the curler header and just use it in both places (graph tile and sample). Otherwise we are duplicating the logic but with slight differences. The only real difference they need is to fetch different urls

[merkispavel]

thank you!

[merkispavel]

I'm sorry that it took so long. And thank you for the work @Neyromancer. I tested it locally: works as expected

@kevinkreiser Let us know if you'd like to take a look or it's ok to merge it

[merkispavel]

re-approve