fix(thumbnails): keep the new preview, probe encoders only when gone, sweep reused PIDs
- Convert never removes a path that is the preview it just wrote. It compares the device and inode, or the resolved and normcased paths when the disk has no inode numbers. This covers x_thumb.JPG on a case-insensitive disk. - A JPEG XL encode failure re-runs encoder detection only when the encoder's binary or module is gone. A "no encoder" result is checked again on the settings read, on a jxl save, and after 60 s, so one slow probe no longer turns JPEG XL off until a restart. - The startup sweep now also removes temp files that carry this process's PID but predate its start. Containers reuse PIDs.
This commit is contained in:
@@ -361,6 +361,55 @@ class TestConvertOutdated(ConvertCase):
|
||||
self.assertTrue(self._thumb_path("f3", ".jpg").exists(), "the new format stays")
|
||||
self.assertEqual(sum(1 for p in olds if p.exists()), 3)
|
||||
|
||||
async def test_the_new_preview_is_never_removed_as_an_old_one(self):
|
||||
# Re-review N1: on a case-insensitive disk x_thumb.JPG (an old
|
||||
# spelling) can be the x_thumb.jpg just written. A hard link stands
|
||||
# in for that second name of one file.
|
||||
PromptManagerConfig.update_config(
|
||||
{"thumbnails": {"format": "jpeg", "quality": 80, "edge": 512}}
|
||||
)
|
||||
src = self.output_dir / "x.jpg"
|
||||
Image.new("RGB", (64, 64), (200, 10, 10)).save(src, "JPEG")
|
||||
alias = self._thumb_path("x", ".JPG")
|
||||
alias.parent.mkdir(parents=True, exist_ok=True)
|
||||
Image.new("RGB", (16, 16)).save(alias, "JPEG")
|
||||
later = src.stat().st_mtime + 5
|
||||
os.utime(alias, (later, later))
|
||||
real = self.api._generate_one
|
||||
|
||||
def same_file(source, dst, *args, **kwargs):
|
||||
made = real(source, dst, *args, **kwargs)
|
||||
os.remove(alias)
|
||||
os.link(made["path"], alias)
|
||||
return made
|
||||
|
||||
self.api._generate_one = same_file
|
||||
_, job = await self._convert()
|
||||
self.assertTrue(self._thumb_path("x", ".jpg").exists())
|
||||
self.assertTrue(alias.exists(), "the new preview's other name is kept")
|
||||
self.assertEqual(job.counters["errors"], 0)
|
||||
|
||||
def test_same_file_falls_back_to_the_path_without_inode_numbers(self):
|
||||
from py.api import thumbnail_convert as tc
|
||||
|
||||
a = self.output_dir / "a_thumb.jpg"
|
||||
b = self.output_dir / "b_thumb.jpg"
|
||||
for path in (a, b):
|
||||
path.write_bytes(b"x")
|
||||
real_stat = os.stat
|
||||
|
||||
def no_inode(path, *args, **kwargs):
|
||||
result = real_stat(path, *args, **kwargs)
|
||||
return os.stat_result(
|
||||
(result.st_mode, 0, result.st_dev) + tuple(result)[3:]
|
||||
)
|
||||
|
||||
with patch.object(tc.os, "stat", side_effect=no_inode):
|
||||
self.assertFalse(tc._same_file(a, b))
|
||||
self.assertTrue(tc._same_file(a, a))
|
||||
self.assertTrue(tc._same_file(a, self.output_dir / "." / "a_thumb.jpg"))
|
||||
self.assertFalse(tc._same_file(a, self.output_dir / "gone.jpg"))
|
||||
|
||||
def test_byte_sizes_are_readable(self):
|
||||
from py.api.thumbnail_convert import readable_bytes
|
||||
|
||||
|
||||
@@ -341,7 +341,8 @@ class TestStrayTemps(TempCase):
|
||||
self._touch(".c_thumb.part-x-2.webp"),
|
||||
self._touch(".young_thumb.part-7-8.webp", age=0),
|
||||
]
|
||||
own = self._touch(f".mine_thumb.part-{os.getpid()}-9.webp")
|
||||
# A temp of this process written since it started may be in use.
|
||||
own = self._touch(f".mine_thumb.part-{os.getpid()}-9.webp", age=0)
|
||||
count, size = te.sweep_stray_temps(self.tmp, min_age=60)
|
||||
self.assertEqual((count, size), (3, 30))
|
||||
for path in strays:
|
||||
@@ -363,6 +364,17 @@ class TestStrayTemps(TempCase):
|
||||
if link is not None:
|
||||
self.assertTrue(link.is_symlink())
|
||||
|
||||
def test_an_own_pid_temp_older_than_this_process_is_swept(self):
|
||||
# Re-review N3: under stable PIDs (Docker) the previous run's crash
|
||||
# leftovers carry this process's PID; they predate its start.
|
||||
before = te.PROCESS_STARTED - 3600
|
||||
stale = self._touch(f".old_thumb.part-{os.getpid()}-9.webp")
|
||||
os.utime(stale, (before, before))
|
||||
live = self._touch(f".new_thumb.part-{os.getpid()}-9.webp", age=0)
|
||||
self.assertEqual(te.sweep_stray_temps(self.tmp, min_age=60)[0], 1)
|
||||
self.assertFalse(stale.exists())
|
||||
self.assertTrue(live.exists())
|
||||
|
||||
def test_a_missing_folder_sweeps_nothing(self):
|
||||
self.assertEqual(te.sweep_stray_temps(self.tmp / "none"), (0, 0))
|
||||
|
||||
@@ -373,9 +385,10 @@ class TestEncoderVanishes(TempCase):
|
||||
availability Appearance reads is refreshed."""
|
||||
|
||||
def test_a_missing_binary_falls_back_to_webp_and_refreshes(self):
|
||||
found = iter(["cjxl", None])
|
||||
found = iter(["cjxl", None, None])
|
||||
with (
|
||||
patch.object(te, "_detect", side_effect=lambda: next(found)),
|
||||
patch.object(te.shutil, "which", return_value=None),
|
||||
patch.object(te, "run_encoder", side_effect=FileNotFoundError("cjxl")),
|
||||
self.assertLogs("prompt_manager.thumbnails", logging.WARNING),
|
||||
):
|
||||
@@ -405,6 +418,11 @@ class TestEncoderVanishes(TempCase):
|
||||
|
||||
with (
|
||||
patch.object(te, "_detect", side_effect=lambda: next(found)),
|
||||
patch.object(
|
||||
te.shutil,
|
||||
"which",
|
||||
side_effect=lambda name: None if name == "cjxl" else name,
|
||||
),
|
||||
patch.object(te, "run_encoder", side_effect=fake),
|
||||
patch.object(te, "_pillow_decodes_jxl", return_value=False),
|
||||
):
|
||||
@@ -417,10 +435,13 @@ class TestEncoderVanishes(TempCase):
|
||||
self.assertEqual(used, ["cjxl", "ffmpeg"])
|
||||
self.assertEqual(self._leftovers(), [])
|
||||
|
||||
def test_the_same_encoder_failing_again_raises(self):
|
||||
def test_a_failure_with_the_encoder_present_does_not_detect_again(self):
|
||||
# Re-review N2: one bad file never runs a probe (which may time out
|
||||
# under load and turn JPEG XL off).
|
||||
failed = subprocess.CompletedProcess([], 1, b"", b"bad input")
|
||||
with (
|
||||
patch.object(te, "_detect", return_value="cjxl") as detect,
|
||||
patch.object(te.shutil, "which", return_value="/usr/bin/cjxl"),
|
||||
patch.object(te, "run_encoder", return_value=failed),
|
||||
):
|
||||
with self.assertRaises(te.EncoderFailed):
|
||||
@@ -429,7 +450,29 @@ class TestEncoderVanishes(TempCase):
|
||||
self.tmp / "x_thumb.jxl",
|
||||
ThumbnailSettings("jxl", 80, 64),
|
||||
)
|
||||
self.assertEqual(detect.call_count, 2, "detected again once")
|
||||
self.assertEqual(detect.call_count, 1, "no second probe")
|
||||
|
||||
def test_no_encoder_found_is_checked_again_on_the_settings_read(self):
|
||||
# Re-review N2: a probe that found nothing (a timeout) recovers.
|
||||
found = iter([None, "cjxl"])
|
||||
with patch.object(te, "_detect", side_effect=lambda: next(found)):
|
||||
self.assertIsNone(te.detect_jxl_encoder())
|
||||
self.assertEqual(te.encoder_status()["jxl"]["encoder"], "cjxl")
|
||||
self.assertEqual(te.detect_jxl_encoder(), "cjxl")
|
||||
|
||||
def test_no_encoder_found_is_checked_again_after_a_while(self):
|
||||
found = iter([None, "ffmpeg"])
|
||||
clock = [1000.0]
|
||||
with (
|
||||
patch.object(te, "_detect", side_effect=lambda: next(found)) as detect,
|
||||
patch.object(te.time, "monotonic", side_effect=lambda: clock[0]),
|
||||
):
|
||||
self.assertIsNone(te.detect_jxl_encoder())
|
||||
self.assertIsNone(te.detect_jxl_encoder(), "cached within the TTL")
|
||||
clock[0] += te.MISSING_ENCODER_TTL + 1
|
||||
self.assertEqual(te.detect_jxl_encoder(), "ffmpeg")
|
||||
self.assertEqual(te.detect_jxl_encoder(), "ffmpeg")
|
||||
self.assertEqual(detect.call_count, 2)
|
||||
|
||||
|
||||
class TestEncoderProcesses(TempCase):
|
||||
|
||||
Reference in New Issue
Block a user