Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: fix clocked schedule #341

Merged
merged 1 commit into from
Jul 24, 2020
Merged

fix: fix clocked schedule #341

merged 1 commit into from
Jul 24, 2020

Conversation

yywing
Copy link
Contributor

@yywing yywing commented May 14, 2020

Clocked schedule is stateless.
It can run once because of PeriodicTask.one_off.

Copy link
Member

@auvipy auvipy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you explain more of your reason for this approach?

@@ -186,12 +186,6 @@ class ClockedSchedule(models.Model):
verbose_name=_('Clock Time'),
help_text=_('Run the task at clocked time'),
)
enabled = models.BooleanField(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a breaking change

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you explain more of your reason for this approach?

I maybe handle it before this weekends(timezone: Beijing). too busy now.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can introduce swapalbe/ abstract base model as Django-activity-stream did

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or we can keep it but it always True.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about default=False?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But it act as always True.

@yywing
Copy link
Contributor Author

yywing commented Jun 22, 2020

I will work on this PR.
And I maybe pull a new feature for mult time schedule.

@yywing
Copy link
Contributor Author

yywing commented Jun 23, 2020

图片

@yywing
Copy link
Contributor Author

yywing commented Jun 23, 2020

  1. is _due return None will make schedule stuck. Return None will never update heap first object.
    celery heap

  2. Clocked schedule changed clocked schedule model, and it will make schedule reload. As picture I upload.

    1. I create many clocked schedule which work on the same time.
    2. first clocked schedule run ,and change model,and schedule reload.
    3. reload wiled call is_due and all clocked schedule set to false, but it olny called one
    4. schedule change and all schedule reload, all scheled set false at 2),so never called.

So only two task run.

@yywing
Copy link
Contributor Author

yywing commented Jun 23, 2020

I change clocked model, remove enabled field, use PeriodicTask.one_off.

@superandrew
Copy link

I am looking forward for this PR, @yywing it is great that you have been able to track, understand fix one of the most hideous and awkward bugs of this extension.

@superandrew
Copy link

just to add some hype: pip install git+https://github.com/yywing/django-celery-beat/@f2bdc5214b3c70ee3805654855ab08e55452df69
fixes the problem reported on #310

@superandrew
Copy link

hi, any news on when this PR is going to be released?

@auvipy
Copy link
Member

auvipy commented Jul 14, 2020

there are some concerns regarding backward compat

@superandrew
Copy link

too bad. My experience with my use case and django_celery_beat was that everything worked for days before stop working randomly (then I understood it was not literally randomly but there was a reason), and this fix definitely fixed everything. So I really hope this could go in production as soon as possible. I am not confident with celery or this package, but if I can help with some real world testing I'd be more than happy to do so.

@auvipy
Copy link
Member

auvipy commented Jul 14, 2020

please do some more real-world testing and report back

@superandrew
Copy link

my production environment started using this package about 10 days ago.

We were able to run hundreds of task, and the scheduled tasks never stopped once.

Before this fix, after a few days, some condition would make celery beat block on one task and stop process the tasks all together.

I hope this helps. Please let me know if there is anything specific I could test.

@auvipy auvipy merged commit 28ba977 into celery:master Jul 24, 2020
@sfdye
Copy link

sfdye commented Aug 6, 2020

Is it possible to release this fix please?

@auvipy
Copy link
Member

auvipy commented Aug 6, 2020

Next week Hopefully

@adl-asi
Copy link

adl-asi commented Aug 25, 2020

@auvipy Will this be released soon? Trying to use clocked schedule in a production setting and running into issues

@adl-asi
Copy link

adl-asi commented Sep 8, 2020

Just checking in again. Is there an eta for when this fix will be released?

@auvipy
Copy link
Member

auvipy commented Sep 12, 2020

hope full this week. ping me again

@saethor
Copy link

saethor commented Sep 18, 2020

Any updates about this?

@belongwqz
Copy link

still waiting

@Lotram
Copy link

Lotram commented Sep 22, 2020

@auvipy any news?

@revoteon
Copy link

@auvipy Any updates?

@ghazi-git
Copy link

I really wish I'd seen this PR earlier. I started using clocked schedule last month. Like everyone else, things started working fine and then at some point, the clocked tasks stopped being processed. What I found out is that whenever there is one task where PeriodicTask.enabled is True but ClockedSchedule.enabled is False, all tasks after it are never executed. So, the question became how did I end up in a situation where the task PeriodicTask.enabled is True but ClockedSchedule.enabled is False.

Later I was able to reproduce it consistently by scheduling a task, then stopping celery beat until the scheduled time of the task has passed. When celery beat is brought back up after that, the scheduled task, that should have executed when celery beat was down, is picked up and gets executed. So, ClockedSchedule.enabled is set to False, but then PeriodicTask.enabled is never changed to False, which results in the next tasks not executed. In production, that might be translated to "a clocked task needs to be scheduled to execute when there is some downtime/maintenance for the issue to start happening"

Anyway, right before deciding to switch to one-off tasks using crontab schedule, I saw that there is a new version and when checking the commits , I found this PR. Now, that ClockedSchedule.enabled is removed, I installed the new version and retested the issue and it is no longer happening. So, thank you @yywing for fixing this and @auvipy for releasing the fix.

@sfdye
Copy link

sfdye commented Oct 21, 2020

Looks like 2.1.0 was released yesterday
https://pypi.org/project/django-celery-beat/#history

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

troubleshooting a database scheduler which does nothing at all
10 participants