mapnik / node-mapnik

Bindings to mapnik for node.js
http://mapnik.org/documentation/node-mapnik
BSD 3-Clause "New" or "Revised" License
532 stars 165 forks source link

Intermittent errors from the GDAL driver starting in 3.2.0 #437

Closed mojodna closed 9 years ago

mojodna commented 9 years ago

I'm getting intermittent errors from the GDAL driver that eventually result in segfaults and aborts (I'm going to try to put together a repro for this tomorrow):

Error: /home/ubuntu/.../data/WA/n47.5w122.3.tif, band 1: IReadBlock failed at X offset 2, Y offset 5

The files that cause problems may be consistent. This also appears between 3.1.6 and 3.2.0 and makes me wonder about "Mapnik GDAL plugin now keeps datasets open for the lifetime of the datasource (rather than per featureset)" (from the change log).

rouault commented 9 years ago

should I consider "ERROR 1: VRTSourcedRasterBand::IRasterIO() called recursively on the same band" a diagnostic warning (when I know there are no cycles) or something more severe?

@mojodna Not sure how to explain more than my previous explanations ;-) Basically when this warning occurs and that there's no cycles, then it means there is a concurrent use of a VRT dataset, which is not good. With one level of VRT and GDAL 2.0.0 and VRT_SHARED_SOURCE=0, this shouldn't happen. And with my latest fix in the 2.0 branch and VRT_SHARED_SOURCE=0, this should also no longer happen with multiple levels of VRTs.

mojodna commented 9 years ago

@rouault That helps, thank you.

springmeyer commented 9 years ago

Great work @rouault and @mojodna

Awesome re: crash repro! I'll rebuild soon w/ new bits and give it a shot.

@mojodna let me know when you know on this. Per https://github.com/mapnik/node-mapnik/issues/445#issuecomment-118111512 I'm thinking about publishing testing binaries from a branch and will need to decide if I use 1) GDAL 2.0.0 or 2) GDAL trunk or 3) GDAL 2.0.0 with just https://github.com/OSGeo/gdal/commit/379ccad895a48016574d90945ca84545a60dec97 applied (inclined to choose 3)

springmeyer commented 9 years ago

@rouault - wow, just noticed libtiff 4.0.4 is out! (http://download.osgeo.org/libtiff/). Any reason not to upgrade to that (I'm currently still using 4.0.4beta)?

rouault commented 9 years ago

@rouault - wow, just noticed libtiff 4.0.4 is out! (http://download.osgeo.org/libtiff/). Any reason not to upgrade to that (I'm currently still using 4.0.4beta)?

@springmeyer Should be OK to upgrade to 4.0.4. Nothing dramatically new, but Bob Friensenham fixed a few warnings (mostly pedantic AFAIR) emitted from Coverity Scan.

mojodna commented 9 years ago

@springmeyer I followed option 3 (see https://github.com/mojodna/mapnik-packaging/commit/9a61520d55f276412f80b580890b5e8cbcc077fd) and can confirm that things are working.

Given that node-mapnik will always be threaded, does it make sense to always set (in node-mapnik) VRT_SHARED_SOURCE=0 when GDAL is used from Node?

springmeyer commented 9 years ago

Great. I just gave you commit access to mapnik-packaging. Can you add you patch commit to the gdal-branch?

As far as the option: yes seems reasonable to set it always.

mojodna commented 9 years ago

@springmeyer pushed to the gdal-2 branch: https://github.com/mapnik/mapnik-packaging/commit/9a61520d55f276412f80b580890b5e8cbcc077fd

springmeyer commented 9 years ago

@mojodna thanks. Just pushed binaries based on your patch. Want to give them a try and make sure the problem is still fixed with them? Once https://travis-ci.org/mapnik/node-mapnik/builds/69403464 is all green do:

"mapnik": "https://github.com/mapnik/node-mapnik/tarball/gdal2"
mojodna commented 9 years ago

:+1: Looks all good (setting VRT_SHARED_SOURCE=0 externally).

springmeyer commented 9 years ago

@mojodna awesome. So sounds like we can:

brianreavis commented 9 years ago

No more tiff errors here! I'll get going on the 2.0 move on node-gdal.

rouault commented 9 years ago

create a node-mapnik patch that starts setting VRT_SHARED_SOURCE=0 always. Not sure the cleanest way to do this...

Not sure if it answers the question, but in C/C++, that can be done with CPLSetConfigOption("VRT_SHARED_SOURCE", "0") (the latest point to do that is just before opening a dataset)

springmeyer commented 9 years ago

@rouault - thanks for that tip. I think for now I'm most comfortable setting in in JS. That is now done in 30ab8aa

create a node-mapnik issue that tracks when to upgrade to GDAL 2.x for the official binaries - refs naturalatlas/node-gdal#110 since that will need to happen at the same time ideally.

This is already done in the latest Mapnik 3 SDK used: https://github.com/mapnik/node-mapnik/commit/3d966538cf84ed45a1678b7b5c5ead6a29e12082. Noted in changelog after d8e9b27