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

Dirty tiles cached for 7 days #441

Open
nrenner opened this issue Jun 11, 2024 · 4 comments
Open

Dirty tiles cached for 7 days #441

nrenner opened this issue Jun 11, 2024 · 4 comments

Comments

@nrenner
Copy link

nrenner commented Jun 11, 2024

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:

mod_tile/src/mod_tile.c

Lines 526 to 527 in 1f81434

/* Test if the tile we are serving is out of date, then set a low maxAge*/
if (state == tileOld) {

Determined in tile_state:

mod_tile/src/mod_tile.c

Lines 442 to 448 in 1f81434

if (stat.expired) {
if ((r->request_time - r->finfo.mtime) < scfg->veryold_threshold) {
return tileOld;
} else {
return tileVeryOld;
}
}

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:

  • // VERYOLD_THRESHOLD: defines how old a tile needs to be (in microseconds) to get rendering priority rather than renderingLow priority
    // 1000000*3600*24*365 = 31536000000000
    #define VERYOLD_THRESHOLD 31536000000000
@hummeltech
Copy link
Collaborator

Thank you @nrenner, I assume you are suggesting changing this:

if (state == tileOld) {

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
Copy link
Author

nrenner commented Jun 12, 2024

		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
Copy link
Author

nrenner commented Jun 12, 2024

The same issue applies to tile_handler_status:

(state == tileOld) ? "due to be rendered" : "clean", mtime_str, atime_str,

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
Copy link

Spiekerooger commented Jun 13, 2024

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.

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