openstreetmap / mod_tile

Renders map tiles with mapnik and serves them using apache
http://wiki.openstreetmap.org/wiki/Mod_tile
GNU General Public License v2.0
288 stars 190 forks source link

Dirty tiles cached for 7 days #441

Closed nrenner closed 1 month ago

nrenner commented 3 months ago

See also operations#1096 for examples and trying to infer from indirect hints.

From just looking at the code¹, I guess state in add_expiry is actually tileVeryOld² and not tileOld for dirty tiles:

https://github.com/openstreetmap/mod_tile/blob/1f81434203d4f6977f8458a7ea42f31523cfa0e0/src/mod_tile.c#L526-L527

Determined in tile_state:

https://github.com/openstreetmap/mod_tile/blob/1f81434203d4f6977f8458a7ea42f31523cfa0e0/src/mod_tile.c#L442-L448

Assuming the default³ of 1 year for scfg->veryold_threshold, 20-year-old expired tiles will be older and therefore tileVeryOld.

So, dirty tiles would not get a low maxAge as originally intended. Instead, maxAge would be derived from modified time (-20 years) and clamped to ModTileCacheDurationMax, resulting in a Cache-Control: max-age=604800.

¹ not entirely sure if this is the code actually used for OSMF render servers and in what version; permalinks for current master ² tileVeryOld was introduced 11 years ago, apperently without adjusting the add_expiry code ³ searching for ModTileVeryOldThreshold in chef does not find any overriding config, default should be one year:

hummeltech commented 3 months ago

Thank you @nrenner, I assume you are suggesting changing this: https://github.com/openstreetmap/mod_tile/blob/1f81434203d4f6977f8458a7ea42f31523cfa0e0/src/mod_tile.c#L527

To something like this:

        if (state == tileOld || state == tileVeryOld) {

Otherwise, please feel free to offer up a suggested fix for your issue. I am still not entirely clear about what your proposed solution would be.

nrenner commented 3 months ago
        if (state == tileOld || state == tileVeryOld) {

Yes, that should fix the issue and is what I had in mind.

But I'm just theoretically reading on GitHub, don't really know the code and don't have a running instance to test, so someone else should probably confirm.

I'm just investigating why people were still seeing vandalized tiles days after the revert, and I think this bug might have been one of the main reasons.

nrenner commented 3 months ago

The same issue applies to tile_handler_status:

https://github.com/openstreetmap/mod_tile/blob/1f81434203d4f6977f8458a7ea42f31523cfa0e0/src/mod_tile.c#L1150

The tile /status call always says "Tile is clean", even though the tile is clearly expired by an old date, e.g.: Tile is clean. Last rendered at Fri Jun 11 15:54:33 2004 https://ysera.openstreetmap.org/15/29466/16885.png/status

Spiekerooger commented 3 months ago

I can confirm this problem, you will also see it in the mod tile stats, e.g. at ysera.osm.org/mod_tile

ModTileCacheDurationDirty is never applied in modern buildups as the planet-import-complete time system is not used but dirtying of tiles via expiry from osm2pgsql, pushing them 20 years back and by that tileVeryOld and not tileOld.

This means that dirty/stale tiles returned in case of overloaded tile renderers are send out to the cdn with a way-too-large expire header timeframe and not the time set by ModTileCacheDurationDirty.