search

mojodna / tl

An alternate command line interface to tilelive
38 stars 12 forks source link

Module breaks with error: Invalid tilesource protocol: tmsource #3

Closed mohsentaleb closed 9 years ago

mohsentaleb commented 9 years ago

Although tilelive-tmsource is installed globally (as a proof, Tessera server works fine with command: tessera tmstyle://./test.tm2 in which test.tm2 is internally linked to a tm2source).

logs this error:

/AppData/Roaming/npm/node_modules/tl/lib/commands/render.js:95 throw err; ^ Error: Invalid tilesource protocol: tmsource:

wilhelmberg commented 9 years ago

@mohsentaleb another thing for you to try.

Open a command prompt and enter

echo %PROJ_LIB%

If %PROJ_LIB% is set, meaning it shows a path, please do

SET PROJ_LIB=<FULL-PATH-TO-TL-DIR>\node_modules\mapnik\lib\binding\node-v11-win32-x64\share\proj

Then start your copy again from that very command prompt.

Does it work now?

mohsentaleb commented 9 years ago

@BergWerkGIS %PROJ_LIB% is not set to anything. shall I set it now?

wilhelmberg commented 9 years ago

@mohsentaleb strange. On the only machine I could reproduce the problem %PROJ_LIB% was set to the path of another program and setting it to the mapnik\share\proj dir solved it.

No, don't set it.

Could you download Process Explorer?

mohsentaleb commented 9 years ago

@BergWerkGIS

wilhelmberg commented 9 years ago

@mohsentaleb ah, that's interesting.

Do you have a C:\Users\Mohsen\AppData\Roaming\npm\node_modules\tl\node_modules\mapnik\lib\binding\node-v11-win32-x64\share directory?

If so, could you try setting those three environment variables before starting tl copy?

SET GDAL_DATA=C:\Users\Mohsen\AppData\Roaming\npm\node_modules\tl\node_modules\mapnik\lib\binding\node-v11-win32-x64\share\gdal
SET PROJ_LIB=C:\Users\Mohsen\AppData\Roaming\npm\node_modules\tl\node_modules\mapnik\lib\binding\node-v11-win32-x64\share\proj
SET ICU_DATA=C:\Users\Mohsen\AppData\Roaming\npm\node_modules\tl\node_modules\mapnik\lib\binding\node-v11-win32-x64\share\icu
mohsentaleb commented 9 years ago

@BergWerkGIS nope, C:\Users\Mohsen\AppData\Roaming\npm\node_modules\tl\node_modules\mapnik\lib\binding\node-v11-win32-x64\share is not a valid directory in my system.

wilhelmberg commented 9 years ago

@mohsentaleb woah, this is though :smirk:

I've got a special self contained package for you. It is a +200MB download and extracts to ~2.5GB (includes its own node.exe and debug symbols).

If you are still willing to help find the problem, please follow these steps:

SET GDAL_DATA=C:\tl-x86\node_modules\mapnik\lib\binding\node-v11-win32-ia32\share\gdal
SET PROJ_LIB=C:\tl-x86\node_modules\mapnik\lib\binding\node-v11-win32-ia32\share\proj
SET ICU_DATA=C:\tl-x86\node_modules\mapnik\lib\binding\node-v11-win32-ia32\share\icu

Try with a really small area (just a few tiles e.g. 0.005°x0.005°) at a high zoom level (e.g. 18) first. If that works, try increasing the area and/or adding zoom levels.

Last resort: WinDebug

If the process still crashes, it would be great if you could walk through the Debug Mapbox Studio Tutorial once again and provide the log and dump file afterwards. We should get a lot more information now with the debug symbols included.

Please use WinDbg x86 since the package includes x86 binaries.

mohsentaleb commented 9 years ago

@BergWerkGIS Thank you. Gives me error while running:

C:\tl-x86\node_modules\tilelive-tmsource\index.js:133
    var fields = new mapnik.Datasource(opts).describe().fields;
                 ^
Error: Postgis Plugin: ERROR:  syntax error at or near "WHERE"
LINE 1: SELECT ST_SRID("geometry") AS srid FROM map, WHERE "geometry...
                                                     ^
in executeQuery Full sql was: 'SELECT ST_SRID("geometry") AS srid FROM map, WHER
E "geometry" IS NOT NULL LIMIT 1;'

    at C:\tl-x86\node_modules\tilelive-tmsource\index.js:133:18
    at Array.map (native)
    at normalize (C:\tl-x86\node_modules\tilelive-tmsource\index.js:122:35)
    at C:\tl-x86\node_modules\tilelive-tmsource\index.js:210:17
    at fs.js:271:14
    at Object.oncomplete (fs.js:107:15)
springmeyer commented 9 years ago

@mohsentaleb - that error looks like it might have to do with a recent change in Mapnik (https://github.com/mapnik/mapnik/commit/4a1f4a9b5ea30b79dfd7dfa9f683ef57c52f3278#diff-886aede99d12b35b6b895c458c838a54). Could you please try to track down what the original Mapnik XML layer looks like for this failing layer? Then please created an issue at https://github.com/mapnik/mapnik/issues with the details.

flippmoke commented 9 years ago

@springmeyer not sure it has anything to do with that change, it simply looks like there is a comma after the name of the table being provided, not sure what is putting that there?

springmeyer commented 9 years ago

@flippmoke okay, thanks for looking. We'll assume then that this is an error in the tm2 source (inside its actually a Mapnik XML stylesheet). @mohsentaleb can you please share the postgis queries you are using?

mohsentaleb commented 9 years ago

@flippmoke @springmeyer Looked thoroughly into all my queries and didn't find that unnecessary comma you were referring to. Another proof is that I've been using the same queries for at least 5 months with no problem. @springmeyer I can send you my queries in a private email if you will.

springmeyer commented 9 years ago

@springmeyer I can send you my queries in a private email if you will.

Yes, could you do that? My email is dane@mapbox.com. If nothing changed with the queries I presume that one of the queries is complex enough that Mapnik is failing to detect the tablename used in the subquery. You'll likely need to solve this by passing geometry_table="some-table", but would be good to see your queries to try to confirm this is the problem.

springmeyer commented 9 years ago

@mohsentaleb thanks for sharing the sql queries. I think it was the #poi query which triggered this error. I think you can workaround by changing FROM map, poi to FROM map , poi in your SQL. Or by passing geometry_table="map" in your Datasource options.

Also upcoming Mapnik should be fixed and will avoid this error in the future: https://github.com/mapnik/node-mapnik/issues/388

mohsentaleb commented 9 years ago

@springmeyer Thanks, this fixed the query parsing problem. @BergWerkGIS breaks on another error: 

C:\tl-x86\lib\commands\copy.js:102
                        sink.on("tile", function (tile) {
                             ^
TypeError: Object #<FileBackend> has no method 'on'
    at C:\tl-x86\lib\commands\copy.js:102:9
    at C:\tl-x86\node_modules\async\lib\async.js:592:17
    at done (C:\tl-x86\node_modules\async\lib\async.js:135:19)
    at C:\tl-x86\node_modules\async\lib\async.js:32:16
    at C:\tl-x86\node_modules\async\lib\async.js:589:21
    at Object._onImmediate (C:\tl-x86\node_modules\tilelive-cache\node_modules\l
ocking-cache\index.js:74:25)
    at processImmediate [as _immediateCallback] (timers.js:345:15)
wilhelmberg commented 9 years ago

@mohsentaleb are you exporting to files? Just remove the event handler sink.on.

If you export to mbtiles it should work as is.

mohsentaleb commented 9 years ago

@BergWerkGIS thanks, fixed. works for a small boundary as expected. Tried for a larger area and breaks again. May I ask for a private email to send you the logs and my queries?

wilhelmberg commented 9 years ago

@mohsentaleb sure, wilhelm@mapbox.com

Maybe you could also include the parameters you call tl copy with? I suppose a db dump is still out of reach? Even privately shared?

springmeyer commented 9 years ago

I took a closer look at this and found that I can also crash tl copy on OS X with extreme memory usage. Profiling shows that all the threads (using mapnik) are idle and doing nothing. The problem is work being done in the main loop and in pure JS. I suspected that tilelive-cache is doing something wrong so I removed it like so:

diff --git a/bin/tl.js b/bin/tl.js
index d1c88b6..c5f4132 100755
--- a/bin/tl.js
+++ b/bin/tl.js
@@ -5,7 +5,7 @@ var fs = require("fs"),
     path = require("path");

 var parser = require("nomnom"),
-    tilelive = require("tilelive-cache")(require("tilelive-streaming")(require("tilelive")));
+    tilelive = require("tilelive-streaming")(require("tilelive"));

 parser.options({
   version: {

And then the export was able to finish in 40 seconds with a max memory usage of 160 MB.

So then I dug a bit deeper. @BergWerkGIS noticed that the Mapnik XML for @mohsentaleb's testcase is 3 MB. Not huge but also not small. I then noticed that tilelive-cache is using the entire source.uri inside the cache key and the XML string is part of the tilelive-vector uri. It looks like the memory bloat is because the entire 3 MB of XML is being copied around and held on to inside tilelive-cache. This is another workaround:

diff --git a/index.js b/index.js
index f1968ee..535fec4 100644
--- a/index.js
+++ b/index.js
@@ -24,6 +24,7 @@ var enableCaching = function(uri, source, locker) {
       properties[k] = context.callback[k];
     });

+    delete uri.xml
     var key = util.format("%s:%j@%j", name, uri, properties);

     // glue on any additional arguments using their JSON representation
wilhelmberg commented 9 years ago

@mohsentaleb @mojodna

I can confirm @springmeyer 's observations.

I used x64 version and was able to copy z8 to z18 without problems.

Both fixes worked for me, although the memory usage was quite different to the 160MB mentioned above. Each fix 1, fix 2 and fix1/fix2 combined maxed at ~2.5GB while staying considerably below 1GB most of the time.

Memory usage pattern looks reasonable to me: image

mojodna commented 9 years ago

@springmeyer @BergWerkGIS Ah, great. I'll take a deeper look at tilelive-cache.

mohsentaleb commented 9 years ago

@springmeyer @BergWerkGIS The patch also works for me. Thank you.

wilhelmberg commented 9 years ago

@mohsentaleb Can you comment on memory usage on your system?

mohsentaleb commented 9 years ago

@BergWerkGIS Almost linear usage around 100MB.

wilhelmberg commented 9 years ago

@mohsentaleb Thanks, great that it is working for you now.

mohsentaleb commented 9 years ago

@BergWerkGIS Yeah, thanks. Although exporting tiles with tl is way slower than with tilemill. I mean if exporting for a given area takes 20 minutes in tilemill, tl does that in more than 6-7hours. (same data, same styles). Any ideas how to improve this?

@mojodna Thanks for bearing with us. Are you planning for a patch for tl in near future?

wilhelmberg commented 9 years ago

@mohsentaleb

I think this is due to the way TileMill and Mapbox Studio work.

Improvement would be:

mohsentaleb commented 9 years ago

@BergWerkGIS Is it possible to create vector tiles mbtiles using tl ? Mapbox Studio crashes while I try to export that file.

wilhelmberg commented 9 years ago

@mohsentaleb I'm not too familiar with tl and don't know, if it supports vector tiles.

WRT to Mapbox Studio export crash, can you open an issue? https://github.com/mapbox/mapbox-studio/issues/

With a short description and if your test data is valid for this case too? Maybe already with a DMP file included? :smirk: Debug Mapbox Studio Tutorial

I was able to export the test data to vector tiles on two different machines without problems.

mohsentaleb commented 9 years ago

@mojodna May be you could comment on tl supporting vector mbtiles?

mojodna commented 9 years ago

@mojodna Thanks for bearing with us. Are you planning for a patch for tl in near future?

Yes. @springmeyer and I discussed this last week briefly; because the Mapnik XML configuration is an essential part of describing a source (and presumably present each time the source is requested), it's not as easy as just applying that patch.

@mojodna May be you could comment on tl supporting vector mbtiles?

See https://github.com/mojodna/tilelive-tmsource. When using this as the source, the output (to be written to an MBTiles archive (or file system)) will be vector tiles.

springmeyer commented 9 years ago

@mojodna - what I also wanted to ask: what is the big idea behind tilelive-cache? Is catching sources essential or just a performance improvement (and in what cases)?

mojodna commented 9 years ago

It's partially a performance improvement, as it allows getTile calls to have their results cached (some of the sources also manage this internally in different ways, so why not pull the functionality out and allow all sources to benefit).

It's really more of a convenience thing when working with the tilelive interface, as it allows calling code to get out of the business of pre-loading sources and managing references (i.e. no need to track state, so calling tilelive.load() will do the right thing each time and only initialize sources if they haven't already been).

The caching of sources is of dubious value though, as conflicts between references to Mapnik sources that are evicted (and "closed") has led me to increase the source cache size to the point where no sources are ever evicted.

In theory, it would facilitate something like Windshaft, which handles an effectively unbounded number of Mapnik configurations.

mojodna commented 9 years ago

I fixed the underlying issue in tilelive-cache by hashing keys to limit their length. It's published as tilelive-cache@0.4.3. Reinstalling tl will pick up the new version of the dependency.

Thank you all for your help with this issue.