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

Runtime error: feature referenced out of range key #795

Closed
doubotis opened this issue Feb 10, 2023 · 13 comments · Fixed by #2460
Closed

Runtime error: feature referenced out of range key #795

doubotis opened this issue Feb 10, 2023 · 13 comments · Fixed by #2460
Labels
bug Something isn't working js-parity

Comments

@doubotis
Copy link

doubotis commented Feb 10, 2023

Describe the bug
On some specific PBF files, the Android and iOS native lib crashes with a std::runtime error :

feature referenced out of range key

I attached a sample of a PBF file that triggers the crash : 5512.pbf.zip.
If you need the MBTiles that originated this PBF file and the styling, I can host it.
The PBF file is reported as a valid Vector Mapbox Tile.
This file is read and handled without any problem by the Javascript library maplibre-gl-js.

To Reproduce

  1. Go to the location that leads to this PBF file. Coordinates: 50.625487, 5.570648 (50°37'31.8"N 5°34'14.3"E)
  2. Zoom to level 18.
  3. See crash.

Expected behavior
App still working nicely and displaying maps.

Platform information (please complete the following information):

  • OS: iOS16+ and Android 10+, not tested before these versions.
  • Platform: iOS/Android (native)
  • Version: org.maplibre.gl:android-sdk:9.6.0

Additional context
It seems the issue is bound to some duplicated properties on some PBF files, but I can't tell how to solve the issue.
The reported crash message seems to be located here. I have found some evidences on this bug report too. Sadly, i struggle to decide if I have to solve the issue from Maplibre GL Native or if this is the MBTiles's fault.
For reference, I filled the issue in openmaptiles-tools github repos too (as the MBTiles was generated from these tools).

@xabbu42
Copy link
Contributor

xabbu42 commented Feb 10, 2023

I reported the linked bug and also authored a possible fix (with test) in a pull request. Sadly the pull request was never applied. All vector tiles generated with postgis are potentially affected, depending of the parallelization of the query. Currently we artificially reduce parallel query execution in our tools to work around the bug. I'm very interested to get a proper fix into maplibre-gl-native as this would allow us to remove that limitation.

As described in the bug report there are some performance implications with the possible fixes.

The standard discourages but allows such tiles, so Maplibre GL Native should either support them or document that it uses a stricter standard. Maplibre GL JS on the other hand handles such tiles without problems, so for feature compatibility Native should as well.

@ovivoda
Copy link
Contributor

ovivoda commented Feb 12, 2023

@xabbu42 can you please point me to the PR that should have fixed this problem?

@xabbu42
Copy link
Contributor

xabbu42 commented Feb 12, 2023

mapbox/vector-tile#56 or mapbox/vector-tile#53 should both fix this issue (from memory and what I wrote in mapbox/vector-tile#55, I did not recheck any of this). The two pull requests do have different performance implications.

@doubotis
Copy link
Author

doubotis commented Feb 13, 2023

All vector tiles generated with postgis are potentially affected, depending of the parallelization of the query.

I can confirm this statement. For every MBTiles creation attempt, issues don't occur in the same place. Sometimes, vector tiles generated at zoom level 4-6 got the issue, leading to the crash of the app instantly.

Currently we artificially reduce parallel query execution in our tools...

Besides the fact I am greatly interested by an update of Maplibre GL Native to handle duplicate property keys, can you explain more in details how do you reduce the parallel query execution on MBTiles generation, and how much this impacts generation time ?

More specifically, which configuration files are needed to modify to reduce parallel query execution ?

Thanks!

@xabbu42
Copy link
Contributor

xabbu42 commented Feb 13, 2023

We already use a custom function in all our mbtiles queries and we just removed the "parallel save" declaration in that function (https://www.postgresql.org/docs/current/parallel-safety.html) which has the side effect of inhibiting parallel execution exactly where we currently don't want it. A clean way to achieve a similar result may be https://www.postgresql.org/docs/current/runtime-config-resource.html#GUC-MAX-PARALLEL-WORKERS-PER-GATHER

@AJJackGIS
Copy link

I'm using node:v-5.2.0 and find vector-tile hash id is bc2f7f44cdb0d9af2b282ea4a755f85a13ba713a , the latest submission records show that this problem has been solved, is it possible to fix this problem by compiling the latest code ?

TomPohys pushed a commit to openmaptiles/openmaptiles that referenced this issue Jul 10, 2023
VectorTiles generated via the [openmaptiles-tools](https://github.com/openmaptiles/openmaptiles-tools) toolchain and using the OpenMaptiles-SQL-Layers as the query-source result in tiles which can contain duplicated keys.

This is because PostGIS `ST_AsMVT` is used to generate the Tiles and due to the way they [implemented parallelization](https://trac.osgeo.org/postgis/ticket/4310).

Although it is not prohibited by the Mapbox-Vector-Tile-Specification MapBox as well as MapLibre depending on version either throw a lot of warnings or even crash when encountering tiles with duplicated keys.
mapbox/vector-tile#55
maplibre/maplibre-native#795

After investigating I identified the boundary layer in zoom-leves 1 - 4 as the only layers containing duplicated keys.
@CQGISer
Copy link

CQGISer commented May 16, 2024

The use of GDAL to make mvt will still appear"feature referenced out of range key",
Is there a way to solve the problem?

@nathreed
Copy link

mapbox/vector-tile#56 or mapbox/vector-tile#53 should both fix this issue (from memory and what I wrote in mapbox/vector-tile#55, I did not recheck any of this). The two pull requests do have different performance implications.

@louwers would MapLibre be open to forking https://github.com/mapbox/vector-tile to apply one of these two fixes for this issue? The license seems compatible and the upstream project appears to be abandoned/have very little activity, and in any case little interest in merging either of the proposed fixes.

@louwers
Copy link
Collaborator

louwers commented May 28, 2024

@nathreed Absolutely!

Do you want to kick off the process, or shall I do it? The New Repository Checklist is here: https://github.com/maplibre/maplibre/issues/new/choose

@nathreed
Copy link

@louwers I went ahead and opened an issue to kick off discussion about this.

@acalcutt
Copy link
Collaborator

acalcutt commented Jun 4, 2024

@louwers Should I cherry pick the vector-tile change to opengl-2 for node? I seem to recall we had worked around a few errors caused by vector-tile already, but I would imagine some issues still exist. I am having trouble finding the threads right now but I will look.

@louwers
Copy link
Collaborator

louwers commented Jun 4, 2024

@acalcutt Sounds like a good idea.

@acalcutt
Copy link
Collaborator

acalcutt commented Jun 4, 2024

For testing with node, does anyone have the original mbtiles and style file I could look at and try in tileserver-gl? i see there is a pbf but the mbtiles would be much easier to test with.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working js-parity
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants