mirror of
https://github.com/alexta69/metube.git
synced 2026-09-21 13:35:01 +00:00
fix: stop stating the output file on every progress tick (#980)
yt-dlp documents 'filename' as always present in a progress hook, so the update_status branch that stats it ran on every forwarded tick: throttled to one every 0.5s per download, times MAX_CONCURRENT_DOWNLOADS. Those are blocking syscalls on the event loop, and when the filesystem is slow each one freezes every other request the server is serving -- which is what a bare GET timing out at >10s looks like from outside. The call was also useless while it was expensive. Until the download finishes the bytes live in tmpfilename; 'filename' is the destination, which does not exist yet, so os.path.exists() returned False and size stayed None. It only yields a real value on a terminal status, and the Downloading table has no size column, so nothing displayed it before completion either way. Stat only when the status is 'finished'. That covers both moments a file genuinely exists at that path: yt-dlp's own finished status, and the MoveFiles postprocessor reporting the final merged name. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -2,6 +2,7 @@
|
|||||||
|
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import asyncio
|
||||||
import os
|
import os
|
||||||
import pickle
|
import pickle
|
||||||
import signal
|
import signal
|
||||||
@@ -11,7 +12,8 @@ import threading
|
|||||||
import types
|
import types
|
||||||
import unittest
|
import unittest
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
from unittest.mock import MagicMock, patch
|
from concurrent.futures import ThreadPoolExecutor
|
||||||
|
from unittest.mock import AsyncMock, MagicMock, patch
|
||||||
|
|
||||||
fake_yt_dlp = types.ModuleType("yt_dlp")
|
fake_yt_dlp = types.ModuleType("yt_dlp")
|
||||||
fake_networking = types.ModuleType("yt_dlp.networking")
|
fake_networking = types.ModuleType("yt_dlp.networking")
|
||||||
@@ -1176,3 +1178,62 @@ class PotProviderUrlsTests(unittest.TestCase):
|
|||||||
if __name__ == "__main__":
|
if __name__ == "__main__":
|
||||||
|
|
||||||
unittest.main()
|
unittest.main()
|
||||||
|
|
||||||
|
|
||||||
|
class UpdateStatusFileStatTests(unittest.IsolatedAsyncioTestCase):
|
||||||
|
"""The progress path must not touch the filesystem on the event loop.
|
||||||
|
|
||||||
|
yt-dlp reports 'filename' on every progress tick, but until the download
|
||||||
|
finishes that path does not exist yet -- the bytes are in 'tmpfilename'.
|
||||||
|
Stating it per tick meant blocking syscalls on the event loop twice a
|
||||||
|
second per download, always answering None. See issue #980.
|
||||||
|
"""
|
||||||
|
|
||||||
|
async def _run_update_status(self, statuses):
|
||||||
|
import queue as _queue
|
||||||
|
|
||||||
|
download = _make_test_download()
|
||||||
|
download.download_dir = "/tmp"
|
||||||
|
source = _queue.Queue()
|
||||||
|
for status in statuses:
|
||||||
|
source.put(status)
|
||||||
|
source.put(None)
|
||||||
|
download.status_queue = source
|
||||||
|
download.loop = asyncio.get_running_loop()
|
||||||
|
download._executor = ThreadPoolExecutor(max_workers=1)
|
||||||
|
notifier = MagicMock()
|
||||||
|
notifier.updated = AsyncMock()
|
||||||
|
download.notifier = notifier
|
||||||
|
|
||||||
|
stat_calls = []
|
||||||
|
|
||||||
|
def record_exists(path):
|
||||||
|
stat_calls.append(path)
|
||||||
|
return False
|
||||||
|
|
||||||
|
try:
|
||||||
|
with patch("ytdl.os.path.exists", side_effect=record_exists):
|
||||||
|
await download.update_status()
|
||||||
|
finally:
|
||||||
|
download._executor.shutdown(wait=True)
|
||||||
|
return download, stat_calls
|
||||||
|
|
||||||
|
async def test_downloading_ticks_do_not_stat_the_output_file(self):
|
||||||
|
ticks = [
|
||||||
|
{"status": "downloading", "filename": "/tmp/v.mp4",
|
||||||
|
"tmpfilename": "/tmp/v.mp4.part", "downloaded_bytes": i}
|
||||||
|
for i in range(1, 6)
|
||||||
|
]
|
||||||
|
download, stat_calls = await self._run_update_status(ticks)
|
||||||
|
|
||||||
|
self.assertEqual(stat_calls, [])
|
||||||
|
self.assertEqual(download.info.filename, "v.mp4")
|
||||||
|
|
||||||
|
async def test_finished_status_still_stats_the_output_file(self):
|
||||||
|
download, stat_calls = await self._run_update_status([
|
||||||
|
{"status": "downloading", "filename": "/tmp/v.mp4", "downloaded_bytes": 1},
|
||||||
|
{"status": "finished", "filename": "/tmp/v.mp4"},
|
||||||
|
])
|
||||||
|
|
||||||
|
self.assertEqual(stat_calls, ["/tmp/v.mp4"])
|
||||||
|
self.assertIsNone(download.info.size)
|
||||||
|
|||||||
+11
-1
@@ -1027,7 +1027,17 @@ class Download:
|
|||||||
if not rel_name.lower().endswith(allowed_caption_exts):
|
if not rel_name.lower().endswith(allowed_caption_exts):
|
||||||
continue
|
continue
|
||||||
self.info.filename = rel_name
|
self.info.filename = rel_name
|
||||||
self.info.size = os.path.getsize(fileName) if os.path.exists(fileName) else None
|
# Stat only on a terminal status. yt-dlp documents 'filename' as
|
||||||
|
# always present in a progress hook, but until the download
|
||||||
|
# finishes the bytes are in tmpfilename and 'filename' is a
|
||||||
|
# destination that does not exist yet -- so this was two
|
||||||
|
# blocking syscalls on the event loop, twice a second per
|
||||||
|
# active download, to arrive at None. A stat that takes seconds
|
||||||
|
# on a contended filesystem stalls every other request the
|
||||||
|
# server is serving. Nothing displays the size before
|
||||||
|
# completion: the Downloading table has no size column.
|
||||||
|
if status.get('status') == 'finished':
|
||||||
|
self.info.size = os.path.getsize(fileName) if os.path.exists(fileName) else None
|
||||||
if getattr(self.info, 'download_type', '') == 'thumbnail':
|
if getattr(self.info, 'download_type', '') == 'thumbnail':
|
||||||
# The thumbnail convertor always emits a .jpg, but yt-dlp may
|
# The thumbnail convertor always emits a .jpg, but yt-dlp may
|
||||||
# report the pre-conversion media/thumbnail extension
|
# report the pre-conversion media/thumbnail extension
|
||||||
|
|||||||
Reference in New Issue
Block a user