rt/wrolpi
1
0
Fork 0
mirror of https://github.com/lrnselfreliance/wrolpi.git synced 2026-10-26 19:01:39 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions

Fixing bug where next download would be calculated before download was marked as complete, causing infinite retries.

Browse source
This commit is contained in:
Roland 2026-03-25 16:41:26 -06:00
parent d61b9d2ef9
commit dd9906ee04
No known key found for this signature in database
GPG key ID: 1BA620CE6C9A4680
2 changed files with 76 additions and 14 deletions
Download patch file Download diff file Expand all files Collapse all files

29
wrolpi/downloader.py
View file

@ -1455,6 +1455,23 @@ async def signal_download_download(download_id: int, download_url: str):
download.location = result.location or download.location or None
# Clear any old errors if the download succeeded.
download.error = result.error if result.error else None
# Set status BEFORE calculating next_download. calculate_next_download uses
# the download's status and last_successful_download to determine scheduling.
# If status is still "pending" (stale), the overdue spread logic can return
# now() for the most overdue download, causing an infinite re-download loop.
if result.error and 'outside download window' in result.error:
# Download was paused due to window closing; reset to new so it resumes later.
download.renew()
download.error = result.error
elif try_again is False and not download.frequency:
# Only once-downloads can fail.
download.fail()
elif result.success:
download.complete()
else:
download.defer()
if isinstance(result.retry_seconds, int):
download.next_download = now() + timedelta(seconds=result.retry_seconds)
else:
@ -1468,18 +1485,6 @@ async def signal_download_download(download_id: int, download_url: str):
download_manager.create_downloads(session, urls, downloader_name=download.sub_downloader,
settings=child_settings)
if result.error and 'outside download window' in result.error:
# Download was paused due to window closing; reset to new so it resumes later.
download.renew()
download.error = result.error
elif try_again is False and not download.frequency:
# Only once-downloads can fail.
download.fail()
elif result.success:
download.complete()
else:
download.defer()
# Remove this domain from the running list.
download_manager._delete_processing_domain(download_domain)
# Allow the download to resume.

61
wrolpi/test/test_downloader.py
View file

@ -335,6 +335,63 @@ async def test_calculate_next_download_overdue_spreading(test_session, test_down
assert d3_result == datetime(2000, 1, 16, 18, tzinfo=pytz.UTC) # now + 1.75 days
@pytest.mark.asyncio
async def test_overdue_recurring_download_not_requeued_after_completion(test_session, test_download_manager, fake_now,
test_downloader, await_switches):
"""When multiple same-frequency downloads are overdue and the most overdue one completes,
it should NOT be immediately renewed. Reproduces a bug where next_download was set to now()
because calculate_next_download was called before complete(), leaving the download with stale
pending status and last_successful_download. This caused an infinite loop on production where
doomandbloom.net/feed reached 3,379 attempts."""
test_downloader.set_test_success()
# Create 3 daily recurring downloads.
d1 = test_download_manager.recurring_download(test_session, 'https://example.com/overdue1',
DownloadFrequency.daily, test_downloader.name)
d2 = test_download_manager.recurring_download(test_session, 'https://example.com/overdue2',
DownloadFrequency.daily, test_downloader.name)
d3 = test_download_manager.recurring_download(test_session, 'https://example.com/overdue3',
DownloadFrequency.daily, test_downloader.name)
# Set them as if they were last downloaded at different times and are all overdue.
d1.last_successful_download = datetime(2020, 1, 1, tzinfo=pytz.UTC)
d1.next_download = datetime(2020, 1, 2, tzinfo=pytz.UTC)
d1.status = 'complete'
d2.last_successful_download = datetime(2020, 1, 2, tzinfo=pytz.UTC)
d2.next_download = datetime(2020, 1, 3, tzinfo=pytz.UTC)
d2.status = 'complete'
d3.last_successful_download = datetime(2020, 1, 3, tzinfo=pytz.UTC)
d3.next_download = datetime(2020, 1, 4, tzinfo=pytz.UTC)
d3.status = 'complete'
test_session.commit()
# It's now Jan 10 — all three are overdue.
fake_now(datetime(2020, 1, 10, tzinfo=pytz.UTC))
# Renew overdue downloads and let one run.
test_download_manager.renew_recurring_downloads()
await test_download_manager.wait_for_all_downloads()
test_session.expire_all()
# At least one download should have completed.
completed = [d for d in [d1, d2, d3] if d.is_complete]
assert len(completed) >= 1, \
f'Expected at least one completed download, got statuses: {[d.status for d in [d1, d2, d3]]}'
# Advance 30 seconds — simulating the real download cycle interval. In production, real time
# advances between when calculate_next_download sets next_download=now() and when
# renew_recurring_downloads checks if next_download < now().
fake_now(datetime(2020, 1, 10, 0, 0, 30, tzinfo=pytz.UTC))
# The completed download should NOT be renewed — its next_download should be in the future.
test_download_manager.renew_recurring_downloads()
test_session.expire_all()
for d in completed:
assert not d.is_new, \
f'{d.url} was renewed 30 seconds after completing — infinite re-download loop!'
@pytest.mark.asyncio
async def test_calculate_next_download_single_overdue(test_session, test_download_manager, fake_now):
"""A single overdue download should use normal scheduling (not spread logic)."""
@ -438,8 +495,8 @@ async def test_recurring_downloads(test_session, test_download_manager, fake_now
# Download is deferred, last successful download remains the same.
assert download.is_deferred, download.status_code
assert download.last_successful_download == now_
# Download should be retried after the DEFAULT_RETRY_FREQUENCY.
expected = datetime(2020, 1, 1, 3, 0, 0, 997200, tzinfo=pytz.UTC)
# Download should be retried using exponential backoff: now + min(3^attempts * hour, frequency).
expected = datetime(2020, 1, 1, 3, 0, 1, tzinfo=pytz.UTC)
assert download.next_download == expected
# Try the download again, it finally succeeds.