Skip to content

de-singleton tile extract in graphreader#3281

Merged
kevinkreiser merged 49 commits intomasterfrom
nn-desingleton-tile-extract
Oct 27, 2021
Merged

de-singleton tile extract in graphreader#3281
kevinkreiser merged 49 commits intomasterfrom
nn-desingleton-tile-extract

Conversation

@nilsnolde
Copy link
Copy Markdown
Member

@nilsnolde nilsnolde commented Aug 23, 2021

fixes #3117

I started with this on the side, working on it here and there. to re-iterate: we want to be able to load other tile extracts while a consumer application is running (e.g. valhalla_service). in #3117 @kevinkreiser proposed to use a fixed-width index file storing the tile_id and byte offset in the final tar file which the graphreader can read first (if available) to really fast initialize the tile_extract_t (faster because it does not have to scan through the entire tar file thanks to the index file).

tasks:

  • set up python script
  • tests python (needs some refactoring)
  • adapt graphreader to read the index file first if present
  • tests graphreader
  • some open questions:
    • this (currently) does not work for existing extracts, so far it only builds an extract from mjolnir.tile_dir. is that fine with you? we could also add that ability and it sorta was what @kevinkreiser initially suggested -> not necessary IMO
    • where to add this functionality: python bindings for sure IMO, what about valhalla_service? -> subsequent PRs

@kevinkreiser I didn't do much yet on the c++ side, will comment a bit on the files, a few pointers would be cool:)

example commandline: python scripts/valhalla_build_extract --config site/valhalla.json

@nilsnolde nilsnolde force-pushed the nn-desingleton-tile-extract branch from e656389 to 7bcc7b9 Compare August 23, 2021 12:57
…ve different tar file sizes with predicted traffic
…so we don't get a race condition with the test which actually creates the tar
@nilsnolde
Copy link
Copy Markdown
Member Author

I removed setting the tile_extract from the base test config again. as kevin pointed rightfully out, it'd create a race with other tests accessing the utrecht tiles

@kevinkreiser
Copy link
Copy Markdown
Member

@nilsnolde ill review this today, might be tonight by the time i have time but ill get to it. thanks for hanging in there!

@kevinkreiser
Copy link
Copy Markdown
Member

ok i made a couple requests for minor changes otherwise looks good!

@nilsnolde
Copy link
Copy Markdown
Member Author

thanks @kevinkreiser for final review. I changed all the things you requested:

  • sort files in the tar
  • left some todos

finally done 🥳 next up: integrate the ability to reload tiles to python bindings (what else?)

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

Successfully merging this pull request may close these issues.

De-Singleton tile_extract_t to Allow for Config (tileset) Reloading

2 participants