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

"render_expired" doesn't use TILEDIR from map name in /etc/renderd.conf when deciding if tiles exist already #286

Open
SomeoneElseOSM opened this issue Jun 5, 2022 · 16 comments

Comments

@SomeoneElseOSM
Copy link
Contributor

SomeoneElseOSM commented Jun 5, 2022

An example:

With /etc/renderd.conf containing
[s2o]
URI=/hot/
TILEDIR=/var/lib/mod_tile
XML=/path/to/mapnik.xml
HOST=localhost
TILESIZE=256
MAXZOOM=20

and a "dirty_files.txt" containing:
18/131010/87170

This command
sudo -u _renderd render_expired --map=s2o --min-zoom=1 --max-zoom=20 -s /var/run/renderd/renderd.sock < /path/to/dirty_tiles.txt

returns

Total for all tiles rendered
Meta tiles rendered: Rendered 0 tiles in 0.00 seconds (0.00 tiles/s)
Total tiles rendered: Rendered 0 tiles in 0.00 seconds (0.00 tiles/s)
Total tiles in input: 1
Total tiles expanded from input: 18
Total meta tiles deleted: 0
Total meta tiles touched: 0
Total tiles ignored (not on disk): 18

even when the corresponding metatile /var/lib/mod_tile/s2o/18/17/245/244/200/0.meta does exist.
However, when I comment out "TILEDIR", delete all cached tiles and generate the new tile for http:///hot/18/131010/87170.png (which creates a metatile below /var/cache - the default location), and rerun

sudo -u _renderd render_expired --map=s2o --min-zoom=1 --max-zoom=20 -s /var/run/renderd/renderd.sock < /path/to/dirty_tiles.txt

I get

Rendering client
Starting 1 rendering threads
render: file:///var/cache/renderd/tiles/s2o/18/17/245/244/200/0.meta
Waiting for rendering threads to finish

Total for all tiles rendered
Meta tiles rendered: Rendered 1 tiles in 12.67 seconds (0.08 tiles/s)
Total tiles rendered: Rendered 64 tiles in 12.67 seconds (5.05 tiles/s)
Total tiles in input: 1
Total tiles expanded from input: 18
Total meta tiles deleted: 0
Total meta tiles touched: 0
Total tiles ignored (not on disk): 17

It looks like whatever's deciding whether a tile already exists or not isn't looking at the location indicated by the map section in /etc/renderd.conf but always below /var/cache

This is with Ubuntu 22.04 set up as per the version of https://switch2osm.org/serving-tiles/manually-building-a-tile-server-ubuntu-22-04-lts/ that exists now** - the render_expired is from the mod_tile diistributed with 22.04.

** As I write this it has "TILEDIR=/var/lib/mod_tile" in the example there, which I suspect I should remove as it doesn't matter for that guide where tiles are created. Historically they were below /var/lb/mod_tile, but there's no reason they have to be there.

SomeoneElseOSM added a commit to SomeoneElseOSM/switch2osm.github.io that referenced this issue Aug 10, 2022
I removed "and you’re not worried about automatically marking updated tiles as “dirty” to force re-rendering" from the "osmosis" instructions.  At the time I wrote that I hadn't got osm2pgsql-replication to handle dirty tiles at all because I hadn't yet identified the problem as openstreetmap/mod_tile#286 .  With the instructions in place elsewhere in the switch2osm documentation there's no need for this caveat.
SomeoneElseOSM added a commit to switch2osm/switch2osm that referenced this issue Aug 10, 2022
I removed "and you’re not worried about automatically marking updated tiles as “dirty” to force re-rendering" from the "osmosis" instructions.  At the time I wrote that I hadn't got osm2pgsql-replication to handle dirty tiles at all because I hadn't yet identified the problem as openstreetmap/mod_tile#286 .  With the instructions in place elsewhere in the switch2osm documentation there's no need for this caveat.
@hummeltech
Copy link
Collaborator

hummeltech commented Mar 10, 2023

@SomeoneElseOSM It looks like you need to specify the tile directory as part of the command with -t if it differs from the default.

\fB\-t\fR|\-\-tile-dir=DIR
Specify the base directory where the rendered tiles are. The default is '/var/cache/renderd/tiles'

@SomeoneElseOSM
Copy link
Contributor Author

The point I was trying to make above was that "I've already told it where the tiles are, in '/etc/renderd.conf'; I shouldn't need to tell it again".

However, if you're happy that "render_expired" is well enough documented then I guess that this can be closed; I've already changed the switch2osm documentation so that new users following that won't hit this problem.

@hummeltech
Copy link
Collaborator

hummeltech commented Mar 10, 2023

Aha, I apologize for not understanding completely, I see your point, yes that is indeed true. Perhaps the renderd.conf parsing code needs to be placed in its own file so that it can be included, that's probably why this support wasn't added to render_*.

I.E.:

mod_tile/src/daemon.c

Lines 898 to 1032 in f521540

g_logger(G_LOG_LEVEL_DEBUG, "Parsing map config section(s)");
for (int section = 0; section < iniparser_getnsec(ini); section++) {
const char *name = iniparser_getsecname(ini, section);
if (strncmp(name, "renderd", 7) && strcmp(name, "mapnik")) {
/* this is a map config section */
if (config.num_threads == NULL || config.tile_dir == NULL) {
g_logger(G_LOG_LEVEL_CRITICAL, "No valid (active) renderd config section available");
exit(7);
}
iconf++;
g_logger(G_LOG_LEVEL_DEBUG, "Parsing map config section %i: %s", iconf, name);
if (iconf >= XMLCONFIGS_MAX) {
g_logger(G_LOG_LEVEL_CRITICAL, "Config: more than %d configurations found", XMLCONFIGS_MAX);
exit(7);
}
if (strlen(name) >= (XMLCONFIG_MAX - 1)) {
g_logger(G_LOG_LEVEL_CRITICAL, "XML name too long: %s", name);
exit(7);
}
strcpy(maps[iconf].xmlname, name);
snprintf(buffer, sizeof(buffer), "%s:uri", name);
const char *ini_uri = iniparser_getstring(ini, buffer, (char *)"");
if (strlen(ini_uri) >= (PATH_MAX - 1)) {
g_logger(G_LOG_LEVEL_CRITICAL, "URI too long: %s", ini_uri);
exit(7);
}
strcpy(maps[iconf].xmluri, ini_uri);
snprintf(buffer, sizeof(buffer), "%s:xml", name);
const char *ini_xmlpath = iniparser_getstring(ini, buffer, (char *)"");
if (strlen(ini_xmlpath) >= (PATH_MAX - 1)) {
g_logger(G_LOG_LEVEL_CRITICAL, "XML path too long: %s", ini_xmlpath);
exit(7);
}
strcpy(maps[iconf].xmlfile, ini_xmlpath);
snprintf(buffer, sizeof(buffer), "%s:host", name);
const char *ini_hostname = iniparser_getstring(ini, buffer, (char *) "");
if (strlen(ini_hostname) >= (PATH_MAX - 1)) {
g_logger(G_LOG_LEVEL_CRITICAL, "Host name too long: %s", ini_hostname);
exit(7);
}
strcpy(maps[iconf].host, ini_hostname);
snprintf(buffer, sizeof(buffer), "%s:htcphost", name);
const char *ini_htcpip = iniparser_getstring(ini, buffer, (char *) "");
if (strlen(ini_htcpip) >= (PATH_MAX - 1)) {
g_logger(G_LOG_LEVEL_CRITICAL, "HTCP host name too long: %s", ini_htcpip);
exit(7);
}
strcpy(maps[iconf].htcpip, ini_htcpip);
snprintf(buffer, sizeof(buffer), "%s:tilesize", name);
const char *ini_tilesize = iniparser_getstring(ini, buffer, (char *) "256");
maps[iconf].tile_px_size = atoi(ini_tilesize);
if (maps[iconf].tile_px_size < 1) {
g_logger(G_LOG_LEVEL_CRITICAL, "Tile size is invalid: %s", ini_tilesize);
exit(7);
}
snprintf(buffer, sizeof(buffer), "%s:scale", name);
const char *ini_scale = iniparser_getstring(ini, buffer, (char *) "1.0");
maps[iconf].scale_factor = atof(ini_scale);
if (maps[iconf].scale_factor < 0.1 || maps[iconf].scale_factor > 8.0) {
g_logger(G_LOG_LEVEL_CRITICAL, "Scale factor is invalid: %s", ini_scale);
exit(7);
}
snprintf(buffer, sizeof(buffer), "%s:tiledir", name);
const char *ini_tiledir = iniparser_getstring(ini, buffer, (char *) config.tile_dir);
if (strlen(ini_tiledir) >= (PATH_MAX - 1)) {
g_logger(G_LOG_LEVEL_CRITICAL, "Tiledir too long: %s", ini_tiledir);
exit(7);
}
strcpy(maps[iconf].tile_dir, ini_tiledir);
snprintf(buffer, sizeof(buffer), "%s:maxzoom", name);
const char *ini_maxzoom = iniparser_getstring(ini, buffer, "18");
maps[iconf].max_zoom = atoi(ini_maxzoom);
if (maps[iconf].max_zoom > MAX_ZOOM) {
g_logger(G_LOG_LEVEL_CRITICAL, "Specified max zoom (%i) is too large. Renderd currently only supports up to zoom level %i", maps[iconf].max_zoom, MAX_ZOOM);
exit(7);
}
snprintf(buffer, sizeof(buffer), "%s:minzoom", name);
const char *ini_minzoom = iniparser_getstring(ini, buffer, "0");
maps[iconf].min_zoom = atoi(ini_minzoom);
if (maps[iconf].min_zoom < 0) {
g_logger(G_LOG_LEVEL_CRITICAL, "Specified min zoom (%i) is too small. Minimum zoom level has to be greater or equal to 0", maps[iconf].min_zoom);
exit(7);
}
if (maps[iconf].min_zoom > maps[iconf].max_zoom) {
g_logger(G_LOG_LEVEL_CRITICAL, "Specified min zoom (%i) is larger than max zoom (%i).", maps[iconf].min_zoom, maps[iconf].max_zoom);
exit(7);
}
snprintf(buffer, sizeof(buffer), "%s:parameterize_style", name);
const char *ini_parameterize = iniparser_getstring(ini, buffer, "");
if (strlen(ini_parameterize) >= (PATH_MAX - 1)) {
g_logger(G_LOG_LEVEL_CRITICAL, "Parameterize_style too long: %s", ini_parameterize);
exit(7);
}
strcpy(maps[iconf].parameterization, ini_parameterize);
/* Pass this information into the rendering threads,
* as it is needed to configure mapniks number of connections
*/
maps[iconf].num_threads = config.num_threads;
}
}

@hummeltech
Copy link
Collaborator

@SomeoneElseOSM, here's a first attempt at implementing this functionality:
master...hummeltech:mod_tile:RendedDConfProcessing
Let me know if it works for you.

@SomeoneElseOSM
Copy link
Contributor Author

Let me know if it works for you.

Thanks, but I don't think I've got an environment to test that in right now.

Andygol pushed a commit to Andygol/switch2osm.github.io that referenced this issue Nov 6, 2023
I removed "and you’re not worried about automatically marking updated tiles as “dirty” to force re-rendering" from the "osmosis" instructions.  At the time I wrote that I hadn't got osm2pgsql-replication to handle dirty tiles at all because I hadn't yet identified the problem as openstreetmap/mod_tile#286 .  With the instructions in place elsewhere in the switch2osm documentation there's no need for this caveat.
@hummeltech
Copy link
Collaborator

hummeltech commented Mar 8, 2024

Hello @SomeoneElseOSM, I have some good news for you. render_expired et. al. now support an additional option --config which directs the application to read from the given renderd.conf file in order to obtain values to use for most of the available command line options (including max-zoom, min-zoom, num-threads, socket & tile-dir [which you are most interested in].) Any passed option will override those values.

This would make the command you need to run more like this:

sudo -u _renderd render_expired --config /etc/renderd.conf --map=s2o --min-zoom=1 < /path/to/dirty_tiles.txt

Of course, if you still don't have an environment to build and test it out, hopefully it resolves your issue when the next release is made, otherwise, let me know if it is of assistance to you!

Thanks!

if (config_file_name_passed) {
int map_section_num = -1;
process_config_file(config_file_name, 0, G_LOG_LEVEL_DEBUG);
for (int i = 0; i < XMLCONFIGS_MAX; ++i) {
if (maps[i].xmlname && strcmp(maps[i].xmlname, mapname) == 0) {
map_section_num = i;
}
}
if (map_section_num < 0) {
g_logger(G_LOG_LEVEL_CRITICAL, "Map section '%s' does not exist in config file '%s'.", mapname, config_file_name);
return 1;
}
if (!max_zoom_passed) {
max_zoom = maps[map_section_num].max_zoom;
max_zoom_passed = 1;
}
if (!min_zoom_passed) {
min_zoom = maps[map_section_num].min_zoom;
min_zoom_passed = 1;
}
if (!num_threads_passed) {
num_threads = maps[map_section_num].num_threads;
num_threads_passed = 1;
}
if (!socketname_passed) {
socketname = strndup(config.socketname, PATH_MAX);
socketname_passed = 1;
}
if (!tile_dir_passed) {
tile_dir = strndup(maps[map_section_num].tile_dir, PATH_MAX);
tile_dir_passed = 1;
}
}

@SomeoneElseOSM
Copy link
Contributor Author

One thing I have been wondering about is how to test all this before it gets packaged into the new Ubuntu / Debian releases. I expect I'll want to create a new page at https://switch2osm.org/serving-tiles/ for Ubuntu 24.04 LTS when that comes out; will changes such as this be in it, or has what will be in that already been frozen?

@hummeltech
Copy link
Collaborator

Well, the latest "release" does not include the aforementioned code and also has not been packaged, however if you would like to test these changes, I can configure the CI pipeline to push up .deb files.

@SomeoneElseOSM
Copy link
Contributor Author

SomeoneElseOSM commented Mar 10, 2024

Thanks - https://packages.ubuntu.com/noble/renderd suggests that Ubuntu 24.04 will contain 0.6.1-2 which is the same as https://packages.debian.org/bookworm/renderd (which I'm already running live), so right now I don't need to test any post-packaging changes before April (which was what I was worried about).

With regard to testing the above changes, I'll do that next time I install a server, since I build from source (in order to increase max zoom levels).

@mxdog
Copy link

mxdog commented Mar 21, 2024

has this been resolved in anyway ? This seems to be the same issue with render_list for instance in /var/lib/mod_tile/default i have my pre-compiled tiles but if i run render_list -m default -a -z 0 -Z 1 --num-threads=1 it recreates the tiles every time, even though they already exists and are not marked expired in any way that i know of ?
1 osm osm 7351 Mar 21 13:24 ./0/0/0/0/0.meta /first run
1 osm osm 7351 Mar 21 13:32 ./0/0/0/0/0.meta /a second run .. rebuilt..

if that is normal for render-list -a .. then a feature request for -M missing only might be appropriate

@SomeoneElseOSM
Copy link
Contributor Author

@mxdog Just checking - have you seen #286 (comment) ? I think that that answers your question before you asked it?

@hummeltech
Copy link
Collaborator

Yes, it shouldn't be re-rendering, unless you're using --force.

mod_tile/src/render_list.c

Lines 382 to 389 in 2a4532f

if (!force) {
s = store->tile_stat(store, mapname, "", x, y, z);
}
if (force || (s.size < 0) || (s.expired)) {
enqueue(mapname, x, y, z);
num_render++;
}

It's likely that it's being re-rendered because the default value for tile_dir is /var/cache/renderd/tiles and your tiles are under /var/lib/mod_tile so it is unable to find the file in order to stat it and determine whether it needs to be rendered or not. You'll need to specify --tile-dir /var/lib/mod_tile in order to tell it to look for tiles in the right spot.

The comment mentioned by @SomeoneElseOSM also refers to some recent updates allowing you to override the defaults for several options with the corresponding values from the specified renderd.conf file, but that has not been released. So, unless you are compiling this yourself, you won't be able to use the new option.

@mxdog
Copy link

mxdog commented Mar 21, 2024

Thanks for the answers adding --tile-dir /var/lib/mod_tile to the command line fixed my current use case , It seems to be a self inflicted gotcha because the tiles are being rendered to /var/lib/mod_tile ( and always have ), I have to assume from the settings in /etc/renderd.conf (Debian) where I specifically use that path , so the case where it can not find the files seems like a bug to me IF it is using that config file to write to that location it seems like it should use that config file to read from that location and not the default path . /var/cache/renderd/tiles is empty and has never had any tiles in it .

@hummeltech
Copy link
Collaborator

hummeltech commented Mar 21, 2024

That's good to hear @mxdog! Yes, that's basically what I was intending to resolve with the updated code mentioned in the comment (#286 (comment)).

The default directory is actually printed in the --help output, which should help:

$ render_expired --help
Usage: render_expired [OPTION] ...
...
  -t, --tile-dir=TILE_DIR           tile cache directory (default is '/var/cache/renderd/tiles')
...

But perhaps some additional notes are needed in the man page and/or the --help output.

Let me know if you have any ideas.

Thanks!

@mxdog
Copy link

mxdog commented Mar 21, 2024

I am going to remove the path lines from /etc/renderd.conf and see where tiles get written too .
OK .... so my case where you have a bug with config paths just proved itself in my eyes or my config is wrong and am confusing the program.

here is my config file and what I have been using since I started trying to set up this server right or wrong, notice I commented out my path tile_dir=/var/lib/mod_tile

[renderd]
stats_file=/run/renderd/renderd.stats
socketname=/run/renderd/renderd.sock
num_threads=12
;tile_dir=/var/lib/mod_tile

[mapnik]
plugins_dir=/usr/lib/mapnik/3.1/input/
font_dir=/usr/share/fonts/
font_dir_recurse=true

[default]
;TILEDIR=/var/lib/mod_tile
XML=/home/osm/openstreetmap-carto/mapnik.xml
HOST=maps.mxdog.net
URI=/hot/
TILESIZE=256
MINZOOM=0
MAXZOOM=20

after restarting renderd the tiles are getting written to the default /var/cache/renderd/tiles/default, which proves to me that the WRITE routines are using the config file path from /etc/renderd.conf BUT the READ routines are still using the DEFAULT hard-coded path , hence the reason I have been always recreating tiles .

I can live with just adding the path to the command line ... again I reiterate if it will write to one place it should be reading from the same place .

in my experience of course, this all depends on that config file actually being correct, with the many guides I have looked at that, this config may be wrong and that is the problem and am just wasting everyone's time.

@hummeltech
Copy link
Collaborator

hummeltech commented Mar 21, 2024

Yes, that is correct, the default value for RENDERD_TILE_DIR is /var/cache/renderd/tiles and has been for as long as I can remember:

#define RENDERD_TILE_DIR "/var/cache/renderd/tiles"

The issue you are running into is that render_expired, render_list and render_speedtest have never supported reading anything from a renderd.conf file until very recently (7389b16) and even now (with that latest code which has not yet been released), still require specifying which renderd.conf file to read from. So, if you use a non-default tile dir, you must specify it using --tile-dir when using those applications (unless of course you are compiling it yourself from any commit starting with the aforementioned one and are also using the --config /path/to/renderd.conf option.)

Furthermore, if you are using mod_tile with a non-default tile dir, I believe you will also need to specify ModTileTileDir (unless you have provided the proper TILEDIR value for the map layer in your renderd.conf file (as you appear to have done)):

ModTileTileDir /var/cache/renderd/tiles

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

No branches or pull requests

3 participants