gis-ops / docker-valhalla

This is our flexible Docker repository for the Valhalla routing engine
MIT License
240 stars 70 forks source link

Cannot customize Valhalla configuration #1

Closed JackCaster closed 3 years ago

JackCaster commented 4 years ago

I would like to increase the number of allowed coordinates to pass to the server. As suggested (here)[https://github.com/gis-ops/docker-valhalla#customize-valhalla-configuration] I edited the custom_files/valhalla.json ("max_shape": 16000 to "max_shape": 99999) and restarted the container. The configuration file, however, is recreated with the default values any time the container is run. Also the tiles are recreated when the container is restarted:

TEST: /custom_files
cat: .files_hashes.txt: No such file or directory
Found valid hash for /custom_files/valhalla_tiles.tar!
Valid valhalla_tiles.tar found with use_tile_only: ! Jumping directly to the tile loading!
cat: .files_hashes.txt: No such file or directory
Found valid hash for /custom_files/massachusetts-latest.osm.pbf!
Scanning old hash d224c24138207cd581e82cdccf66b9b2922d37cb054e2e8fbae70b4d829a557a e8f1f76aac8a713c7c610b7f0e4a390148113d3aa184329c004e1aa63340f8e0
Either valhalla_tiles.tar couldn't be found or new files or file constellations were detected. Rebuilding files:  /custom_files/massachusetts-latest.osm.pbf

Would you be able to help?

nilsnolde commented 4 years ago

Hi @JackCaster ,

that's weird that valhalla.json is not respected on-the-fly. I'll have a look at it tmrw.

Can you share the docker-compose.yml and the output of tree -a . in the directory your docker-compose.yml is located in?

JackCaster commented 4 years ago

Here is the dir tree:

C:\DEV\VALHALLA_TEST
│   environment.yml
│   main.py
│   massachusetts-latest.osm.pbf
│   README.md
│
├───custom_files
│   │   .file_hashes.txt
│   │   massachusetts-latest.osm.pbf
│   │   valhalla.json
│   │   valhalla_tiles.tar
│   │
│   ├───admin_data
│   │       admins.sqlite
│   │
│   ├───elevation_data
│   └───timezone_data
└───sample
     ├───data_gps.csv

I am running docker with

docker run -t -v %cd%/custom_files:/custom_files -p 8002:8002 -e build_elevation=False -e build_admins=True -e build_time_zones=False --name valhalla_1 gisops/valhalla

I am on Windows 10.

nilsnolde commented 4 years ago

Ok I tried all problems you described with Andorra, on Linux it works fine. I did clean up the valhalla_configure.sh script a little, didn't change much more.

It's a bit tedious maybe at the moment (I think we're leaning too much on docker-compose here..), but it's safest to actually define all custom variables, at least the boolean ones. I'll update the README accordingly.

So, in your case, you have two problems:

  1. valhalla_tiles.tar is recreated even though it's there and the PBF didn't change:

can you try use_tiles_ignore_pbf=True and force_rebuild=False? If that doesn't work, I'm not sure what could be the problem. Might well be some Windows specific issue. We create some hashes from the files in custom_files and if they don't match with what is stored in .file_hashed.txt it tries to rebuild, unless you override with the above env vars.

  1. valhalla.json isn't respected

That suggests that your volume isn't mapped properly. When you change the valhalla.json and docker exec -it into the restarted (or new) container, can you see the change reflected in the container's /custom_files/valhalla.json?

JackCaster commented 4 years ago

Ok I tried all problems you described with Andorra, on Linux it works fine. I did clean up the valhalla_configure.sh script a little, didn't change much more.

It's a bit tedious maybe at the moment (I think we're leaning too much on docker-compose here..), but it's safest to actually define all custom variables, at least the boolean ones. I'll update the README accordingly.

So, in your case, you have two problems:

  1. valhalla_tiles.tar is recreated even though it's there and the PBF didn't change:

can you try use_tiles_ignore_pbf=True and force_rebuild=False? If that doesn't work, I'm not sure what could be the problem. Might well be some Windows specific issue. We create some hashes from the files in custom_files and if they don't match with what is stored in .file_hashed.txt it tries to rebuild, unless you override with the above env vars.

  1. valhalla.json isn't respected

That suggests that your volume isn't mapped properly. When you change the valhalla.json and docker exec -it into the restarted (or new) container, can you see the change reflected in the container's /custom_files/valhalla.json?

Using use_tiles_ignore_pbf=True and force_rebuild=False solved both problems: the tiles are not rebuilt and the config files is loaded correctly. Probably, it is not the best long term solution (maps are not rebuilt if updated) but for now it works perfectly. I will try on a Linux machine soon and let you know. Thanks!

nilsnolde commented 4 years ago

Ok, happy to hear that.

If you want to update the graph with fresh data at some point, execute valhalla_build_tiles -c /custom_files/valhalla.json /custom_files/<pbf>, should work.

rodsouto commented 3 years ago
  1. valhalla.json isn't respected

That suggests that your volume isn't mapped properly. When you change the valhalla.json and docker exec -it into the restarted (or new) container, can you see the change reflected in the container's /custom_files/valhalla.json?

Hi @nilsnolde , I have the same problem with valhalla.json being recreated with default values.

I see the changes reflected in the container, but I think that if use_tiles_ignore_pbf=True then here https://github.com/gis-ops/docker-valhalla/blob/3e1b7299569a46e3964209f6eecec12f9f90f1d3/scripts/runtime/configure_valhalla.sh#L223 build_config is called and valhalla.json is always overriden

In the logs you can see that the config file is being built

valhalla_latest | TEST: /custom_files
valhalla_latest | Valid valhalla_tiles.tar found with use_tiles_ignore_pbf: True!
valhalla_latest | Jumping directly to the tile loading!
valhalla_latest |
valhalla_latest | ============================================================================
valhalla_latest | = Valid bounding box and data elevation parameter added. Adding elevation! =
valhalla_latest | ============================================================================
valhalla_latest |   File "<string>", line 1
valhalla_latest |     import math; print int(math.floor(18))
valhalla_latest |                        ^
valhalla_latest | SyntaxError: invalid syntax
valhalla_latest |
valhalla_latest | ========================
valhalla_latest | = Adding admin regions =
valhalla_latest | ========================
valhalla_latest |
valhalla_latest | ========================
valhalla_latest | = Adding timezone data =
valhalla_latest | ========================
valhalla_latest |
valhalla_latest | =========================
valhalla_latest | = Build the config file =
valhalla_latest | =========================

Maybe we could call valhalla_build_config only if valhalla.json does not exist?

nilsnolde commented 3 years ago

yeah you're right.. skipping that if it already exists is the way to go. same for elevation (here it'd be enough to query if the entire elevation folder is empty), admin and timezones dbs really. I shuffled things around a bit lately, best intentions, but seems like I overlooked some things.

Do you have time to give it a go? I'm under pretty project pressure right now, would need to wait for a while.

rodsouto commented 3 years ago

Hi, I have just sent a pull request that solves the issue with valhalla.json

Regarding elevation/admin/timezones it is already possible to skip those with build_elevation/build_admins/build_time_zones so I don't think its neccesary to make any extra change, right?

nilsnolde commented 3 years ago

Yeah, you can, but you'd have to actively turn it off after you actively turned it on. Otherwise it'll just overwrite whatever was there before I think. valhalla_build_tiles does that at least. I didn't try but I don't think valhalla_build_elevation behaves different. Which is too much interaction for my taste;) Don't worry though, not as important as the config fix! Thanks for that!

I'll change the logic a little over the next weeks. It's unnecessarily complicated right now all in all.