Fix Issue #388: Celery Beat scheduled tasks may be executed repeatedly

[BaiJiangJie]

Describe

Fix the problem that Celery Beat scheduled tasks may be executed repeatedly

Problem

Describe

If self.schedule is called, it may cause the data in the database to be retrieved before the self.save() method, instead of the temporarily set last_run_at data

If self.schedule is called here
Then it may cause the last_run_at of self.schedule[name] calling save to be the old data retrieved from the database
Instead of last_run_at temporarily set after task execution (set in __next__() method)
After the max_interval interval, the next task detection cycle will still execute the task again

Demo

Task information:

beat config: max_interval = 60s

Task name: cap
Task execution cycle: Execute every 3 minutes
Task last execution time: 18:00

The first execution of the task: 18:03 (set last_run_at = 18:03 when executing, and it is in memory at this time)

After the task execution is completed,
It is detected that sync is required, and self.schedule is called in sync,
If schedule_changed() is found to be True in self.schedule, all_as_schedule() needs to be called
At this point, the last_run_at of self.schedule[name] called in sync is 18:00
At this time, self.save() is performed in self.sync()

beat: Waking up 60s...

The second execution of the task: 18:04 (because the last_run_at obtained is 18:00, entry.is_due() == True)

Test

There are currently no test cases.

Effect

After modifying the code, the problem of repeated execution no longer appeared in our project JumpServer.

[BaiJiangJie]

#388

jumpserver/jumpserver#10936

[auvipy]

you can try to add new tests for this. also if yyou can fix the failing test in this or another PR, it will be really helpful

[BaiJiangJie]

你从代码逻辑上来看这样修改是对的吗？
如果没问题，我再考虑增加它的测试用例。

Is it right to modify it in this way from the logical point of view of code?

If there is no problem, I will consider adding its test cases.

[auvipy]

I think you can proceed with tests

[BaiJiangJie]

Okay, it might take a while.

[BaiJiangJie]

This is the result of the test. Normally, None should be the last_run_at value updated last time, but after querying it, it is found that it is not.

[BaiJiangJie]

It is normal if self.schedule is changed to self._schedule.

[BaiJiangJie]

The test case has been added, you can review it, thank you.

@auvipy