Wikimedia Bugzilla is closed!

Change 107419 had a related patch set uploaded by Aaron Schulz:
Limit attempts to render the same thumbnail after failures

https://gerrit.wikimedia.org/r/107419

Also improved in https://gerrit.wikimedia.org/r/#/c/127642/. This can be closed.

The patchset is a good first step, but I don't think it's enough.

First of all, if I'm reading the code right, the attempt key is keyed by original _and_ thumb size; my impression is that almost always, scaling errors are because of properties of the original file, not the thumb size specifically (very large, like the 100MB TIFFs, or very complex, like the animated DNA helix). I think that it'd be better to just count hits with just the original, as otherwise we are getting DoSed by people hitting the same image in different pages, e.g. Special:NewFiles' 120px vs. Special:ListFiles' 180px.

More importantly, the code seems to add a key in memcache with TTL 3600. This won't actually fix the effect that the /tmp listing presented in the original description, since those images were apart hours, or even days from each other. It will also not protect us from the general issue of "unscalable images", images that we can never render with our current thresholds but we will forever try to (hundreds of megabytes old multipage TIFFs, for instance).

I think a short memcached-based approach is fine for short temporary errors, like protecting us from bursts, but my original thought -and one that would probably save us from today's recurring rendering outages- would be to actually store this in a flag the image table in the database and *never* try again, unless the flag is reset by an admin or the epoch is bumped. The flag could even be set to the current date, as to be able to quickly reset it with a query, if our infrastructure goes nuts for a day and we get flooded with false positives.

(In reply to Faidon Liambotis from comment #3)
> The patchset is a good first step, but I don't think it's enough.
> 
> First of all, if I'm reading the code right, the attempt key is keyed by
> original _and_ thumb size; my impression is that almost always, scaling
> errors are because of properties of the original file, not the thumb size
> specifically (very large, like the 100MB TIFFs, or very complex, like the
> animated DNA helix). I think that it'd be better to just count hits with
> just the original, as otherwise we are getting DoSed by people hitting the
> same image in different pages, e.g. Special:NewFiles' 120px vs.
> Special:ListFiles' 180px.
> 

Sometiles thumb.php calls fail due to the requeted size being to big, with smaller sizes working. A per-file limit would punish all renderings. We already have per-user rate limiting on thumbnails/min and "non-standard" thumbnails/min. I'd hope that would help with the DOS aspect. For the non DOS aspect, the failure limiting should mostly be enough.

> More importantly, the code seems to add a key in memcache with TTL 3600.
> This won't actually fix the effect that the /tmp listing presented in the
> original description, since those images were apart hours, or even days from
> each other. It will also not protect us from the general issue of
> "unscalable images", images that we can never render with our current
> thresholds but we will forever try to (hundreds of megabytes old multipage
> TIFFs, for instance).
> 
> I think a short memcached-based approach is fine for short temporary errors,
> like protecting us from bursts, but my original thought -and one that would
> probably save us from today's recurring rendering outages- would be to
> actually store this in a flag the image table in the database and *never*
> try again, unless the flag is reset by an admin or the epoch is bumped. The
> flag could even be set to the current date, as to be able to quickly reset
> it with a query, if our infrastructure goes nuts for a day and we get
> flooded with false positives.

If the media handler class bound the the file knows that it cannot be rendered (based on the format, size, and configured resource limits, ect...) it could store it as img_metadata and bail out quickly. There isn't really a way for thumb.php itself to make that determination afaik. Maybe the Multimedia team can look into media handler tweaks.