diff --git a/wrolpi/common.py b/wrolpi/common.py index 94f10f43..66ddae03 100644 --- a/wrolpi/common.py +++ b/wrolpi/common.py @@ -1330,13 +1330,14 @@ ZIG_TYPE = Union[int, float, complex, Decimal, datetime] def zig_zag(low: ZIG_TYPE, high: ZIG_TYPE) -> Generator[ZIG_TYPE, None, None]: """ Generate numbers between `low` and `high` that are - spread out evenly. Produces infinite results. + spread out evenly. Produces infinite unique results. - >>> list(zig_zag(0, 10)) - [0, 5, 2, 7, 1, 3, 6, 8, 0, 1, 3, 4, 5, 6, 8, 9] - >>> list(zig_zag(0, 5)) - [0, 2, 1, 3, 0, 1, 3, 4] - >>> list(zig_zag(50.0, 100.0)) + >>> import itertools + >>> list(itertools.islice(zig_zag(0, 10), 10)) + [0, 5, 2, 7, 1, 3, 6, 8, 4, 9] + >>> list(itertools.islice(zig_zag(0, 5), 5)) + [0, 2, 1, 3, 4] + >>> list(itertools.islice(zig_zag(50.0, 100.0), 11)) [50.0, 75.0, 62.5, 87.5, 56.25, 68.75, 81.25, 93.75, 53.125, 59.375, 65.625] """ output_type = type(low) @@ -1352,9 +1353,10 @@ def zig_zag(low: ZIG_TYPE, high: ZIG_TYPE) -> Generator[ZIG_TYPE, None, None]: divisor = 2 diff = high - low while True: - if num not in results: - yield output_type(num) - results.add(num) + output_value = output_type(num) + if output_value not in results: + yield output_value + results.add(output_value) num += diff / divisor if num >= high: divisor *= 2 diff --git a/wrolpi/downloader.py b/wrolpi/downloader.py index 2455a984..45c4293b 100644 --- a/wrolpi/downloader.py +++ b/wrolpi/downloader.py @@ -1036,6 +1036,10 @@ class DownloadManager: If the download is "complete" and the download has a frequency, schedule the download in it's next iteration. (Next week, month, etc.) + + When multiple same-frequency downloads are overdue (e.g., after service restart), they are spread + dynamically from NOW using zig_zag. The most overdue download (by last_successful_download) runs + immediately, others are spread across the frequency period. """ if download.is_deferred: # Increase next_download slowly at first, then by large gaps later. The largest gap is the download @@ -1053,7 +1057,40 @@ class DownloadManager: raise ValueError('Cannot get next download when Download is not in DB.') freq = download.frequency + now_ = now() + # Get all downloads with same frequency, ordered by last_successful_download + # (NULL/oldest first - these are most overdue) + same_freq_downloads = session.query(Download).filter_by(frequency=freq).order_by( + Download.last_successful_download.asc().nullsfirst(), + Download.id # Tiebreaker for consistent ordering + ).all() + + # Identify overdue downloads (next_download in past, NOT new downloads with NULL next_download) + overdue_downloads = [ + d for d in same_freq_downloads + if d.next_download is not None and d.next_download < now_ + ] + + # New download added while others are overdue: run immediately + if download.next_download is None and len(overdue_downloads) >= 1: + return now_ + + # Multiple overdue: spread dynamically from NOW using zig_zag + if len(overdue_downloads) > 1 and download in overdue_downloads: + overdue_index = overdue_downloads.index(download) + + # Most overdue runs immediately + if overdue_index == 0: + return now_ + + # Others spread across frequency period from NOW using zig_zag + end_time = now_ + timedelta(seconds=freq) + zagger = zig_zag(now_, end_time) + slots = [slot for slot, _ in zip(zagger, range(len(overdue_downloads)))] + return slots[overdue_index] + + # Normal case: use fixed iteration boundaries with zig_zag # Keep this Download in it's position within the like-frequency downloads. downloads = [i.id for i in session.query(Download).filter_by(frequency=freq).order_by(Download.id)] index = downloads.index(download.id) @@ -1061,7 +1098,7 @@ class DownloadManager: # Download was successful. Spread the same-frequency downloads out over their iteration. start_date = datetime(2000, 1, 1).replace(tzinfo=pytz.UTC) # Weeks/months/etc. since start_date. - iterations = ((now() - start_date) // freq).total_seconds() + iterations = ((now_ - start_date) // freq).total_seconds() # Download slots start the next nearest iteration since 2000-01-01. # For example, if a weekly download was performed on 2000-01-01 it will get the same slot within # 2000-01-01 to 2000-01-08. @@ -1070,9 +1107,7 @@ class DownloadManager: # Get this downloads position in the next iteration. If a download is performed at the 3rd slot this week, # it will be downloaded the 3rd slot of next week. zagger = zig_zag(start_date, end_date) - next_download = [i for i, j in zip(zagger, range(len(downloads)))][index] - next_download = next_download - return next_download + return [i for i, j in zip(zagger, range(len(downloads)))][index] @staticmethod def _delete_downloads_q(once: bool = False, status: str = None, returning=Download.id) -> Delete: diff --git a/wrolpi/test/test_common.py b/wrolpi/test/test_common.py index e8dbdbb7..7d25e4fe 100644 --- a/wrolpi/test/test_common.py +++ b/wrolpi/test/test_common.py @@ -244,17 +244,17 @@ def test_chdir(): (0, 10000, [0, 5000, 2500, 7500, 1250, 3750, 6250, 8750, 625, 1875, 3125, 4375, 5625, 6875, 8125, 9375]), (0, 1000, [0, 500, 250, 750, 125, 375, 625, 875, 62, 187, 312, 437, 562, 687, 812, 937]), (0, 100, [0, 50, 25, 75, 12, 37, 62, 87, 6, 18, 31, 43, 56, 68, 81, 93]), - # Values repeat when there are not enough. - (0, 10, [0, 5, 2, 7, 1, 3, 6, 8, 0, 1, 3, 4, 5, 6, 8, 9]), - (0, 5, [0, 2, 1, 3, 0, 1, 3, 4]), - (0, 2, [0, 1, 0, 1, 0, 0, 1, 1, 0, 0, 0, 0, 1, 1, 1, 1]), + # Values are unique - no duplicates after the fix. + (0, 10, [0, 5, 2, 7, 1, 3, 6, 8, 4, 9]), + (0, 5, [0, 2, 1, 3, 4]), + (0, 2, [0, 1]), (50, 100, [50, 75, 62, 87, 56, 68, 81, 93, 53, 59, 65]), (8, 98, [8, 53, 30, 75, 19, 41, 64]), # Floats are supported. Output is identical to above. (50.0, 100.0, [50.0, 75.0, 62.5, 87.5, 56.25, 68.75, 81.25, 93.75, 53.125, 59.375, 65.625]), # Decimals are supported. Output is identical to above. (Decimal('50.0'), Decimal('100.0'), [ - Decimal('50'), Decimal('75'), Decimal('62.5'), Decimal('87.5'), Decimal('56.25'), Decimal('68.75'), + Decimal('50.0'), Decimal('75.0'), Decimal('62.5'), Decimal('87.5'), Decimal('56.25'), Decimal('68.75'), Decimal('81.25'), Decimal('93.75'), Decimal('53.125'), Decimal('59.375'), Decimal('65.625') ]), # Datetimes are supported. diff --git a/wrolpi/test/test_downloader.py b/wrolpi/test/test_downloader.py index 37460a98..51927c35 100644 --- a/wrolpi/test/test_downloader.py +++ b/wrolpi/test/test_downloader.py @@ -177,6 +177,94 @@ async def test_calculate_next_download(test_session, test_download_manager, fake assert test_download_manager.calculate_next_download(test_session, d3) == datetime(2000, 1, 9, 18, tzinfo=pytz.UTC) +@pytest.mark.asyncio +async def test_calculate_next_download_overdue_spreading(test_session, test_download_manager, fake_now): + """When multiple same-frequency downloads are overdue (e.g., after service restart), + they should be spread dynamically from NOW instead of bunching up.""" + # Set up 3 weekly downloads that WERE scheduled in the past (overdue) + d1 = Download(url='https://example.com/overdue1', frequency=DownloadFrequency.weekly) + d1.last_successful_download = datetime(2000, 1, 1, tzinfo=pytz.UTC) # Oldest + d1.next_download = datetime(2000, 1, 8, tzinfo=pytz.UTC) # Was due Jan 8 + + d2 = Download(url='https://example.com/overdue2', frequency=DownloadFrequency.weekly) + d2.last_successful_download = datetime(2000, 1, 2, tzinfo=pytz.UTC) # Middle + d2.next_download = datetime(2000, 1, 9, tzinfo=pytz.UTC) # Was due Jan 9 + + d3 = Download(url='https://example.com/overdue3', frequency=DownloadFrequency.weekly) + d3.last_successful_download = datetime(2000, 1, 3, tzinfo=pytz.UTC) # Newest + d3.next_download = datetime(2000, 1, 10, tzinfo=pytz.UTC) # Was due Jan 10 + + test_session.add_all([d1, d2, d3]) + test_session.commit() + + # It's now Jan 15 - all three are overdue + now_ = fake_now(datetime(2000, 1, 15, tzinfo=pytz.UTC)) + + # Most overdue (d1, oldest last_successful_download) should run immediately + assert test_download_manager.calculate_next_download(test_session, d1) == now_ + + # Others spread across the week from NOW using zig_zag pattern + # zig_zag(now, now+week) with 3 items gives: [now, now+3.5days, now+1.75days] + d2_result = test_download_manager.calculate_next_download(test_session, d2) + d3_result = test_download_manager.calculate_next_download(test_session, d3) + + # d2 gets second slot (middle of week), d3 gets third slot (1/4 of week) + assert d2_result == datetime(2000, 1, 18, 12, tzinfo=pytz.UTC) # now + 3.5 days + assert d3_result == datetime(2000, 1, 16, 18, tzinfo=pytz.UTC) # now + 1.75 days + + +@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).""" + d1 = Download(url='https://example.com/overdue', frequency=DownloadFrequency.weekly) + d1.last_successful_download = datetime(2000, 1, 1, tzinfo=pytz.UTC) + d1.next_download = datetime(2000, 1, 8, tzinfo=pytz.UTC) # Was due Jan 8 + + d2 = Download(url='https://example.com/not_overdue', frequency=DownloadFrequency.weekly) + d2.last_successful_download = datetime(2000, 1, 10, tzinfo=pytz.UTC) + d2.next_download = datetime(2000, 1, 20, tzinfo=pytz.UTC) # Not overdue (due Jan 20) + + test_session.add_all([d1, d2]) + test_session.commit() + + # It's now Jan 15 - only d1 is overdue + fake_now(datetime(2000, 1, 15, tzinfo=pytz.UTC)) + + # Single overdue download uses normal zig_zag scheduling (next iteration boundary) + result = test_download_manager.calculate_next_download(test_session, d1) + # Should be in the next iteration window, not NOW + assert result >= datetime(2000, 1, 15, tzinfo=pytz.UTC) + assert result != datetime(2000, 1, 15, tzinfo=pytz.UTC) # Not immediately + + +@pytest.mark.asyncio +async def test_calculate_next_download_new_download_while_overdue(test_session, test_download_manager, fake_now): + """A new download added while other same-frequency downloads are overdue should run first.""" + # Existing download that is overdue + d1 = Download(url='https://example.com/overdue', frequency=DownloadFrequency.weekly) + d1.last_successful_download = datetime(2000, 1, 1, tzinfo=pytz.UTC) + d1.next_download = datetime(2000, 1, 8, tzinfo=pytz.UTC) # Was due Jan 8 + + # Another overdue download + d2 = Download(url='https://example.com/overdue2', frequency=DownloadFrequency.weekly) + d2.last_successful_download = datetime(2000, 1, 2, tzinfo=pytz.UTC) + d2.next_download = datetime(2000, 1, 9, tzinfo=pytz.UTC) # Was due Jan 9 + + # New download with no history + d3 = Download(url='https://example.com/new', frequency=DownloadFrequency.weekly) + d3.last_successful_download = None # Never downloaded + d3.next_download = None # Never scheduled + + test_session.add_all([d1, d2, d3]) + test_session.commit() + + # It's now Jan 15 - d1 and d2 are overdue, d3 is new + now_ = fake_now(datetime(2000, 1, 15, tzinfo=pytz.UTC)) + + # New download (d3) should run immediately because there are overdue downloads + assert test_download_manager.calculate_next_download(test_session, d3) == now_ + + @pytest.mark.asyncio async def test_recurring_downloads(test_session, test_download_manager, fake_now, test_downloader, await_switches): """A recurring Download should be downloaded forever."""