From 874fc84e999589f9268aa1e493078365e529b3f2 Mon Sep 17 00:00:00 2001 From: Bruce li Date: Sun, 29 Jun 2025 18:23:51 +0800 Subject: [PATCH 1/6] Fix CrontabSchedule task run before schedule time --- django_celery_beat/schedulers.py | 18 ++++++++++++------ t/unit/test_schedulers.py | 18 ++++++++++++++++++ 2 files changed, 30 insertions(+), 6 deletions(-) diff --git a/django_celery_beat/schedulers.py b/django_celery_beat/schedulers.py index 99d98f8c..815b5f72 100644 --- a/django_celery_beat/schedulers.py +++ b/django_celery_beat/schedulers.py @@ -94,13 +94,19 @@ def __init__(self, model, app=None): if not model.last_run_at: model.last_run_at = model.date_changed or self._default_now() - # if last_run_at is not set and - # model.start_time last_run_at should be in way past. - # This will trigger the job to run at start_time - # and avoid the heap block. + if self.model.start_time: - model.last_run_at = model.last_run_at \ - - datetime.timedelta(days=365 * 30) + if isinstance(model.schedule, schedules.schedule): + # if last_run_at is not set and + # model.start_time last_run_at should be in way past. + # This will trigger the job to run at start_time + # and avoid the heap block. + model.last_run_at = model.last_run_at \ + - datetime.timedelta(days=365 * 30) + else: + # last_run_at should be the time the task started. + model.last_run_at = model.start_time + self.last_run_at = model.last_run_at diff --git a/t/unit/test_schedulers.py b/t/unit/test_schedulers.py index f7dfac8b..adfa6d27 100644 --- a/t/unit/test_schedulers.py +++ b/t/unit/test_schedulers.py @@ -1035,6 +1035,24 @@ def test_crontab_with_start_time_after_crontab(self, app): assert not is_due assert next_check == pytest.approx(expected_delay, abs=60) + def test_crontab_with_start_time_before_crontab(self, app): + now = app.now() + delay_minutes = 2 + test_start_time = now - timedelta(minutes=delay_minutes) + crontab_time = now + timedelta(minutes=delay_minutes) + + # start_time(now - 2min) < now < crontab_time(now + 2min) + task = self.create_model_crontab( + crontab(minute=f'{crontab_time.minute}'), + start_time=test_start_time) + + entry = EntryTrackSave(task, app=app) + is_due, next_check = entry.is_due() + + expected_delay = delay_minutes * 60 + assert not is_due + assert next_check < expected_delay + def test_crontab_with_start_time_different_time_zone(self, app): now = app.now() From a582772bb99e545d2585354a9633742173ad2476 Mon Sep 17 00:00:00 2001 From: Bruce li Date: Sun, 29 Jun 2025 18:37:59 +0800 Subject: [PATCH 2/6] Remove blank line --- django_celery_beat/schedulers.py | 1 - 1 file changed, 1 deletion(-) diff --git a/django_celery_beat/schedulers.py b/django_celery_beat/schedulers.py index 815b5f72..58f240fe 100644 --- a/django_celery_beat/schedulers.py +++ b/django_celery_beat/schedulers.py @@ -107,7 +107,6 @@ def __init__(self, model, app=None): # last_run_at should be the time the task started. model.last_run_at = model.start_time - self.last_run_at = model.last_run_at def _disable(self, model): From bdea5ee9884a7197e48bc21aa024c876392c7290 Mon Sep 17 00:00:00 2001 From: Asif Saif Uddin Date: Sun, 29 Jun 2025 17:26:34 +0600 Subject: [PATCH 3/6] Update django_celery_beat/schedulers.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- django_celery_beat/schedulers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/django_celery_beat/schedulers.py b/django_celery_beat/schedulers.py index 58f240fe..379e95d9 100644 --- a/django_celery_beat/schedulers.py +++ b/django_celery_beat/schedulers.py @@ -96,7 +96,7 @@ def __init__(self, model, app=None): model.last_run_at = model.date_changed or self._default_now() if self.model.start_time: - if isinstance(model.schedule, schedules.schedule): + if isinstance(model.schedule, schedules.schedule) and not isinstance(model.schedule, schedules.crontab): # if last_run_at is not set and # model.start_time last_run_at should be in way past. # This will trigger the job to run at start_time From 6dd26e03117faadd468e3e7f73e89f575db32147 Mon Sep 17 00:00:00 2001 From: Bruce li Date: Mon, 30 Jun 2025 10:25:26 +0800 Subject: [PATCH 4/6] Fix unstable unittest The origin unittest use loop to confirm task m1 will not be blocked by task m2. This is not a stable way, the test sometimes passed, sometimes failed. Because m1 sechdued at every 3s, it may or may not schdeuled at after m2' start_time 1s, but current time is before m2' start_time 2s, which make the test failed, but not as expected. --- t/unit/test_schedulers.py | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/t/unit/test_schedulers.py b/t/unit/test_schedulers.py index adfa6d27..c49b2833 100644 --- a/t/unit/test_schedulers.py +++ b/t/unit/test_schedulers.py @@ -1101,31 +1101,42 @@ def test_crontab_with_start_time_different_time_zone(self, app): assert next_check == pytest.approx(expected_delay, abs=60) def test_crontab_with_start_time_tick(self, app): + # Ensure the heapq does not block by new task with start_time PeriodicTask.objects.all().delete() s = self.Scheduler(app=self.app) assert not s._heap m1 = self.create_model_interval(schedule(timedelta(seconds=3))) m1.save() + s.tick() + assert len(s._heap) == 2 now = timezone.now() start_time = now + timedelta(minutes=1) crontab_trigger_time = now + timedelta(minutes=2) + # now < start_time(now + 1min) < crontab_time(now + 2min) m2 = self.create_model_crontab( crontab(minute=f'{crontab_trigger_time.minute}'), start_time=start_time) m2.save() + s.tick() + assert len(s._heap) == 3 + assert s._heap[0][2].name == m1.name e2 = EntryTrackSave(m2, app=self.app) - is_due, _ = e2.is_due() - - max_iterations = 1000 - iterations = 0 - while (not is_due and iterations < max_iterations): - s.tick() - assert s._heap[0][2].name != m2.name - is_due, _ = e2.is_due() + is_due, delay = e2.is_due() + assert not is_due + assert 60 < delay < 120 + + # tick twice to make sure the heap is not blocked by m2 + # before it reaches its start_time + time.sleep(3) + s.tick() + assert s._heap[0][2].name == m1.name + time.sleep(54) + s.tick() + assert s._heap[0][2].name == m1.name @pytest.mark.django_db def test_crontab_exclusion_logic_basic(self): From c4486c858ed888543440c54175bb1d19516066e4 Mon Sep 17 00:00:00 2001 From: Bruce li Date: Mon, 30 Jun 2025 10:38:45 +0800 Subject: [PATCH 5/6] Fix code style --- django_celery_beat/schedulers.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/django_celery_beat/schedulers.py b/django_celery_beat/schedulers.py index 379e95d9..00ce516c 100644 --- a/django_celery_beat/schedulers.py +++ b/django_celery_beat/schedulers.py @@ -96,7 +96,8 @@ def __init__(self, model, app=None): model.last_run_at = model.date_changed or self._default_now() if self.model.start_time: - if isinstance(model.schedule, schedules.schedule) and not isinstance(model.schedule, schedules.crontab): + if isinstance(model.schedule, schedules.schedule) \ + and not isinstance(model.schedule, schedules.crontab): # if last_run_at is not set and # model.start_time last_run_at should be in way past. # This will trigger the job to run at start_time From 88c166f8bf9b4865382fe0548f4e1d50418b9b28 Mon Sep 17 00:00:00 2001 From: Asif Saif Uddin Date: Mon, 30 Jun 2025 14:59:07 +0600 Subject: [PATCH 6/6] Update t/unit/test_schedulers.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- t/unit/test_schedulers.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/unit/test_schedulers.py b/t/unit/test_schedulers.py index c49b2833..e0ac22d3 100644 --- a/t/unit/test_schedulers.py +++ b/t/unit/test_schedulers.py @@ -1134,8 +1134,8 @@ def test_crontab_with_start_time_tick(self, app): time.sleep(3) s.tick() assert s._heap[0][2].name == m1.name - time.sleep(54) - s.tick() + with patch('time.monotonic', side_effect=lambda: monotonic() + 54): + s.tick() assert s._heap[0][2].name == m1.name @pytest.mark.django_db