[OSM-dev] [PATCH] osm2pgsql tile expiry

[Jon Burgess]

On Sun, 2009-02-08 at 12:24 +0000, Steve Hill wrote:
> On Sun, 8 Feb 2009, Martijn van Oosterhout wrote:
> 
> > - The use of -o. Is there any other output osm2pgsql could produce
> > that would be more logical. Not really a big deal, the long option
> > expire-output is there which is good enough for me.
> 
> I can't think of anything else that osm2pgsql outputs, but I'm open to 
> suggestions for a more intuitive option.
> 
> > - There a whole new file added (expire-tiles.c) which seems to have
> > some kind of tree structure. Do you think you could add a comment at
> > the beginning what kind of structure it is and any relevent invariants
> > (like middle-ram.c does).
> 
> It is a simple binary tree of tiles' X coordinates with each node 
> containing a binary tree of Y coordinates.  I will add a comment 
> explaining the structure.
> 
> Its memory requirements are not capped, but daily diffs seem to result in 
> a few hundred thousand dirty tiles at zoom 17, which are easilly 
> accommodated.  Running with the -e option for the initial import of the 
> whole planet would be a Very Bad Thing though. :)
> 
> I was originally intending the binary tree to be a quick hack to be 
> replaced with a better structure (possibly a b-tree) once the rest of the 
> code was working, but it turned out not to pose any noticable performance 
> problems so I have left it as-is for now.

It might be good to take a look at using the generic tree implementation
in rb.c/h. Alternatively, I wrote something similar using bst.[ch] in 
applications/rendering/mapnik/all_tiles/C/tilecount.c


> > - Does middle.h need to include osmtypes.h?
> 
> It makes use of struct osmNode, which is defined in osmtypes.h.  I added 
> the include so that osmtypes.h didn't need to be explicitly included 
> before middle.h in .c files, but I can remove this again if that is 
> preferred.

It is probably best that each header file can be included on its own so
I think it is a good addition.

> > - You've got a definition of EARTH_CIRCUMFERENCE. Does that match with
> > what's used in the projection code?
> 
> Good question.  Looks like the projection code for Spherical Mercator 
> defines an Earth radius of 6378137m.  This gives us a circumference of:
>      2 * 6378137 * 3.14159265358979323846 = 40075016.68557848615314309804
> I'm using 40075016.68, which I'd say is as close as it needs to be.
> 
> Of course, this is going to break if someone is not using Spherical 
> Mercator - maybe I should add code to exit with an error if a different 
> projection is used (the OSM tiles are defined in terms of Spherical 
> Mercator coordinates so the tile list generated is going to be fairly 
> meaningless for other projections).

I'd say that this is the best answer, just abort with an error saying
that generating the tile expiry information is incompatible with
anything other than the spherical mercator projection. 

I have just started running the diff import on the OSM tile server. The
diff imports are a little slower than I'd ideally like but seem to be
working. I had started using the expire.rb code for tile expiry but your
osm2pgsql solution looks like it handles edge cases where a line crosses
a tile with no nodes inside the tile.

I might be tempted to add a metatile flag too. Since mod_tile only cares
about rendering 8x8 grids of tiles this can dramatically reduce the
number of tiles that we actually need to track.

I don't see anything wrong with you adding this into SVN. Thanks for the
new code.

	Jon