feat(thumbnails): classify render failures as permanent or transient
Prerequisite for persisting negative verdicts, and the reason step 7 cannot start with the storage change. generate_and_persist collapses EVERY error into empty Bytes — timeouts and closed semaphores included. That is survivable only because the sentinel lives in moka, which evicts. Write the same signal to content_derived_blobs and a thumbnail that timed out once under load is unrenderable forever. So the classification lands first, on its own, before anything can persist it. It maps onto the existing variants without restructuring, because timeouts already surface as TaskError: ImageError, UnsupportedFormat -> permanent. The decoder rejected these bytes, or they exceed MAX_DECODE_PIXELS. Facts about the image. TaskError, IoError -> transient. Timeout, closed decode semaphore, join failure, unreadable source. Facts about the moment. The asymmetry sets the default: a wrongly-persisted transient marks a good image unrenderable for good, while a wrongly-omitted permanent merely costs a repeated decode. Anything not clearly a content property is therefore transient. Tested by pinning the mapping rather than trusting variant names to stay put — the timeout case especially, since it is the one that turns a load spike into data loss. Nothing consumes this yet; it exists so the storage change cannot be written without it.
This commit is contained in:
@@ -2023,6 +2023,37 @@ pub enum ThumbnailError {
|
||||
UnsupportedFormat,
|
||||
}
|
||||
|
||||
impl ThumbnailError {
|
||||
/// Is this failure a property of the CONTENT, rather than of the moment?
|
||||
///
|
||||
/// Prerequisite for persisting negative verdicts to
|
||||
/// `content_derived_blobs` (see `docs/plan/derived-blobs.md` §Negative
|
||||
/// verdicts). Only a permanent failure may be recorded: it will give the
|
||||
/// same answer forever, so remembering it saves a decode. A transient one
|
||||
/// must never be recorded — the next attempt may well succeed, and a row
|
||||
/// saying otherwise is silent, permanent data loss for that file.
|
||||
///
|
||||
/// The asymmetry is why the default is `false`. A wrongly-persisted
|
||||
/// transient marks a perfectly good image unrenderable for good; a
|
||||
/// wrongly-omitted permanent merely costs a repeated decode. So anything
|
||||
/// not clearly a content property is treated as transient.
|
||||
///
|
||||
/// * [`Self::ImageError`] — the decoder rejected these bytes, or they
|
||||
/// exceed `MAX_DECODE_PIXELS`. Both are facts about the image.
|
||||
/// * [`Self::UnsupportedFormat`] — likewise.
|
||||
/// * [`Self::TaskError`] — timeout, closed decode semaphore, join
|
||||
/// failure. All say the machine was busy, not that the image is bad. A
|
||||
/// timeout under load is the exact case that must not be cached.
|
||||
/// * [`Self::IoError`] — the source could not be read. Says nothing about
|
||||
/// whether it is renderable.
|
||||
pub fn is_permanent(&self) -> bool {
|
||||
match self {
|
||||
ThumbnailError::ImageError(_) | ThumbnailError::UnsupportedFormat => true,
|
||||
ThumbnailError::TaskError(_) | ThumbnailError::IoError(_) => false,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Statistics about the thumbnail cache
|
||||
#[derive(Debug, Clone)]
|
||||
pub struct ThumbnailStats {
|
||||
@@ -2229,6 +2260,27 @@ mod tier_selection_tests {
|
||||
);
|
||||
}
|
||||
|
||||
/// A timeout must never be recorded as a permanent verdict.
|
||||
///
|
||||
/// It is the case that turns a load spike into permanent data loss:
|
||||
/// `generate_and_persist` collapses every error into empty `Bytes`, which
|
||||
/// is survivable only while that sentinel lives in moka and evicts.
|
||||
/// Before any of it reaches `content_derived_blobs`, timeouts must
|
||||
/// classify as transient — so this pins the mapping rather than trusting
|
||||
/// the variant names to stay put.
|
||||
#[test]
|
||||
fn only_content_failures_are_permanent() {
|
||||
// Facts about the image — safe to remember.
|
||||
assert!(ThumbnailError::ImageError("decode failed".into()).is_permanent());
|
||||
assert!(ThumbnailError::UnsupportedFormat.is_permanent());
|
||||
|
||||
// Facts about the moment — must never be remembered. `timeout(...)`
|
||||
// and the decode semaphore both surface as TaskError.
|
||||
assert!(!ThumbnailError::TaskError("thumbnail generation timed out".into()).is_permanent());
|
||||
assert!(!ThumbnailError::TaskError("Decode semaphore closed".into()).is_permanent());
|
||||
assert!(!ThumbnailError::IoError("blob read failed".into()).is_permanent());
|
||||
}
|
||||
|
||||
/// Empty bytes are moka's negative-entry convention (a previous render
|
||||
/// failed). They must not be served as a thumbnail, or a failure gets
|
||||
/// cached and returned as success.
|
||||
|
||||
Reference in New Issue
Block a user