AndTrigger does not work with IntervalTrigger

[Hanaasagi]

Recently, I learn how to use apscheduler, and I find something interesting. From the latest doc's example, AndTrigger can be used as:

from apscheduler.triggers.combining import AndTrigger
from apscheduler.triggers.interval import IntervalTrigger
from apscheduler.triggers.cron import CronTrigger


trigger = AndTrigger([IntervalTrigger(hours=2),
                      CronTrigger(day_of_week='sat,sun')])
scheduler.add_job(job_function, trigger)

Actually, it doesn't work. I have read #281 and #309, and know that the time generated by the two triggers never coincide.

So, if I sepcify a start_date like

import datetime
from tzlocal import get_localzone

from apscheduler.triggers.combining import AndTrigger
from apscheduler.triggers.interval import IntervalTrigger

tz = get_localzone()
now = datetime.datetime.now(tz=tz)

trigger = AndTrigger([IntervalTrigger(hours=2, start_date=now),
                      IntervalTrigger(hours=3, start_date=now)])
next_run_time = trigger.get_next_fire_time(None, now + datetime.timedelta(seconds=10))

print(now)
print(next_run_time)

It works as expected.

2019-02-24 22:23:26.052845+09:00
2019-02-25 04:23:26.052845+09:00

But when schedule a job, it does work.

import datetime
from tzlocal import get_localzone

from apscheduler.triggers.combining import AndTrigger
from apscheduler.triggers.interval import IntervalTrigger
from apscheduler.schedulers.blocking import BlockingScheduler

tz = get_localzone()
now = datetime.datetime.now(tz=tz)


def tick():
    print('tick')


scheduler = BlockingScheduler()
trigger = AndTrigger([IntervalTrigger(seconds=2, start_date=now),
                      IntervalTrigger(seconds=3, start_date=now)])
scheduler.add_job(tick, trigger)
scheduler.start()

The real problem it that AndTrigger may not support IntervalTrigger

In _process_jobs, it will calculate run times .

apscheduler/apscheduler/schedulers/base.py

Lines 970 to 972 in ab991eb

	run_times = job._get_run_times(now)
	run_times = run_times[-1:] if run_times and job.coalesce else run_times
	if run_times:

apscheduler/apscheduler/job.py

Lines 123 to 137 in ab991eb

	def _get_run_times(self, now):
	"""
	Computes the scheduled run times between ``next_run_time`` and ``now`` (inclusive).
	:type now: datetime.datetime
	:rtype: list[datetime.datetime]
	"""
	run_times = []
	next_run_time = self.next_run_time
	while next_run_time and next_run_time <= now:
	run_times.append(next_run_time)
	next_run_time = self.trigger.get_next_fire_time(next_run_time, now)
	return run_times

apscheduler/apscheduler/triggers/combining.py

Lines 53 to 62 in ab991eb

	def get_next_fire_time(self, previous_fire_time, now):
	while True:
	fire_times = [trigger.get_next_fire_time(previous_fire_time, now)
	for trigger in self.triggers]
	if None in fire_times:
	return None
	elif min(fire_times) == max(fire_times):
	return self._apply_jitter(fire_times[0], self.jitter, now)
	else:
	now = max(fire_times)

apscheduler/apscheduler/triggers/interval.py

Lines 52 to 66 in ab991eb

	def get_next_fire_time(self, previous_fire_time, now):
	if previous_fire_time:
	next_fire_time = previous_fire_time + self.interval
	elif self.start_date > now:
	next_fire_time = self.start_date
	else:
	timediff_seconds = timedelta_seconds(now - self.start_date)
	next_interval_num = int(ceil(timediff_seconds / self.interval_length))
	next_fire_time = self.start_date + self.interval * next_interval_num
	if self.jitter is not None:
	next_fire_time = self._apply_jitter(next_fire_time, self.jitter, now)
	if not self.end_date or next_fire_time <= self.end_date:
	return self.timezone.normalize(next_fire_time)

In the loop of calculating next_run_time in AndTrigger, we only change the value of now. But in IntervalTrigger, if we pass previous_fire_time which is not None, it will just add the interval. So it caused a dead loop.

Similarly, When we combine IntervalTrigger and CronTrigger, if previous_fire_time is not None, only CronTrigger will walk.

[Hanaasagi]

A quick but not elegant fix is changing

apscheduler/apscheduler/triggers/combining.py

Lines 54 to 56 in ab991eb

	while True:
	fire_times = [trigger.get_next_fire_time(previous_fire_time, now)
	for trigger in self.triggers]

to

        while True:
            fire_times = [trigger.get_next_fire_time(None, now)
                          for trigger in self.triggers]

[richardhanson]

Hi @Hanaasagi I also have this issue. For a list of interval triggers, the optimal way to find the next fire time seems to be to compute the least common multiple and just return that. Crons can be reduced to an integer of seconds (akin to IntervalTrigger.interval_length) and could be done the same way. For date triggers, it would seem that the only way forward would be to check if the other triggers intersect with the datetime, with the understanding that the max precision you can currently have for any comparison is on the order of seconds (so you'd have to drop anything more specific on both the interval and the cron). @agronholm would that be a solution?

...This happens because the times generated by the two triggers never coincide because IntervalTrigger starts from the current time which of course has milliseconds that are unlikely to be exact 0, while CronTrigger produces datetimes with the fractional part as 0. The question is, how to resolve this? Should CronTrigger ignore the fractional part when comparing datetimes, or should IntervalTrigger's start time drop the millisecond part? (from #309 )

[agronholm]

Something like this is coming to APScheduler 4.0.

[agronholm]

Closing as duplicate of #281.

[Hanaasagi]

I think this problem is not same as #281

The actual problem is that because IntervalTrigger starts counting from current time, it never agrees with the CronTrigger because CronTrigger never produces any nonzero seconds or microseconds that IntervalTrigger likely will. This problem needs to be documented and tools provided to debug triggers that hang this way.

Please test

import datetime
from tzlocal import get_localzone

from apscheduler.triggers.combining import AndTrigger
from apscheduler.triggers.interval import IntervalTrigger
from apscheduler.schedulers.blocking import BlockingScheduler

tz = get_localzone()
now = datetime.datetime.now(tz=tz)


def tick():
    print('tick')


scheduler = BlockingScheduler()
trigger = AndTrigger([IntervalTrigger(seconds=2, start_date=now),
                      IntervalTrigger(seconds=3, start_date=now)])
scheduler.add_job(tick, trigger)
scheduler.start()

I have specified a same start_date, and it could not get a time both agreed. It should run every six second.

[agronholm]

The reason is this:

Iteration 1: now + 2s, now + 3s
Iteration 2: now + 4s, now + 6s
...

This is why they never agree with each other. Maybe it should get a next value for the trigger that produced the earliest fire time and leave the others alone.

[grafolean]

First of all, thank you for such nice library, really love the clean interface and awesome documentation, not to mention that the features are exactly what I was looking for!

I have encountered an issue that I think has the same cause as this one. In my case, I have tried combining OrTrigger with multiple IntervalTriggers, but only the one with the shortest interval kept firing.

I think the reason is that OrTrigger supplies the same previous_fire_time to all its triggers, while it should instead remember the real previous_fire_time for each of them, and supply that. Because of that, when the trigger with the shortest interval fires, its time is saved. And the next time the triggers are asked about next fire time, they do not have their internally saved previous fire time, but instead use previous_fire_time as supplied to them via params. So shortest interval always wins, and other intervals never fire.

Situation is similar with AndTrigger.

Do you think this could be the reason, or am I missing something?

[agronholm]

Triggers are stateless and therefore it is not possible to save the previous fire time of each trigger. If you got to issue #281, you see the possibilities being discussed.

[grafolean]

I'm sorry, I don't see the relevance of #281 to this issue? #281 is about comparing the times (when are the two times equal) while this is about combining triggers. Or am I missing something?

However, you are right about the problem being in the stateless nature of triggers. For example this trigger doesn't work as expected:

trigger = OrTrigger([
    IntervalTrigger(seconds=2, start_date=now),
    IntervalTrigger(seconds=3, start_date=now),
])

I would expect this trigger to fire on seconds 2, 3, 4, 6, 8, 9, 10,... Instead, it fires on seconds 2, 4, 6, 8, 10,... - because the intervals always start from last fire. In other words, combining interval triggers never works, because the state would need to be saved somewhere (more than just previous_fire_time).

Note that I have solved my issue in the mean time by creating a custom (stateful) trigger and extending an executor, and it works nicely. It is however not a generic solution.

[agronholm]

Combining triggers were never really meant to be used with IntervalTriggers, but rather CronTriggers with which they are quite useful. For interval triggers to work as expected with combining triggers, they would either have to remember their previous calculated fire time (even if that didn't end up as the combined fire time) or start from the start time every time.

To fix this I would either have to make triggers stateful or add some kind of native combining mechanism.

[grafolean]

Sure - but as far as I'm concerned, just noting this limitation in documentation would be enough. There are ways around it after all.

[Voyz]

Would there be any chance of getting this reworked on the 3.x version? @agronholm

With all due respect - I love this library and all the work you do, thanks! - but I can't couldn't find any roadmap for 4.0 release, other than the 4.0 milestone, which currently is marked as 20% complete and contains open issues from as far as 4 years back. As much as I understand the wish to move ahead and drop legacy code behind, it would be fantastic to have support for the current version too, especially that this functionality is non-trivial. I'm only expressing some doubt regarding having to rely on the 4.0 being released to fix this type of issue without really knowing when that would happen.

I found one great statement from some other issue where you state:

Python 2 users could still keep using 3.x, which should get bug fixes when possible but no new features.

which leaves me hope that this bug could be dedicated some attention on this version too.

---

If you could provide some context on when we could be looking to expect 4.0 that would be great too!

Once again, massive thanks for all the work and hope these observations find you well :)

[agronholm]

I've already fixed this design issue in master, but understand that I am basically working on it alone on my free time, with at least 5 other F/OSS projects consuming my time as well.

Fixing this issue properly meant making a major design change – namely adding "thresholds" and making triggers stateful. This is not a change that can be backported to the 3.x series. With that in mind, what exactly do you expect me to do about this in the 3.x series? I can publish some sort of fix if I can be reasonably convinced that it doesn't break anything else.

[Voyz]

Truly, more than anything I hoped for some information on when we could expect the 4.0. Like I said, I understand the difficultly of providing the long term support and fighting design legacy. Please don't think I wanted to raise these points as an expectation claim; I merely wanted to get more information on when we could see this functionality working - either through a fix on 3.x or a 4.0 release. Apologies if that came across inappropriately.

You say that you already fixed this design issue on master - does master contain a stable version of APS that you encourage us to use?

Lastly, once again thank you for working on this project, even more so if that's only done in your free time and with other projects in parallel. Your work is appreciated.

[agronholm]

Truly, more than anything I hoped for some information on when we could expect the 4.0. Like I said, I understand the difficultly of providing the long term support and fighting design legacy. Please don't think I wanted to raise these points as an expectation claim; I merely wanted to get more information on when we could see this functionality working - either through a fix on 3.x or a 4.0 release. Apologies if that came across inappropriately.

You didn't – you were far more courteous than most people asking about this 😄

Here is the mid-term plan: I will first focus on my AnyIO project (getting v2.0 out the door). APScheduler 4.0 will depend on the features there to provide proper support for both async and sync schedulers. This process is fairly far along so I expect this part to be completed some time this month. Once that's done, I will resume work on shared job store support and updating the scheduler API. That's the hardest part, and once it's been dealt with, it should be fairly smooth sailing from there on. Barring no further major delays, I expect a beta release within the next three months. How's that for a roadmap?

You say that you already fixed this design issue on master - does master contain a stable version of APS that you encourage us to use?

No, apart from the trigger subsystem, the master branch is currently a mess due to some parts of the design having been updated and some parts not.

Lastly, once again thank you for working on this project, even more so if that's only done in your free time and with other projects in parallel. Your work is appreciated.

Thank you 😃

[Voyz]

That's fantastic, thank you for the update! Thanks for clarifying how your roadmap looks like. It may be useful for other users to see this information available more publicly too.

May I propose a naive suggestion? It seems like 4.0 contains quite a lot of things under its hood. Naturally, without understanding the source code this is just me hypothesising, but would you consider breaking up the releases into smaller stages? Would maybe the AnyIO integration , the trigger overhaul, and the shared job store support be doable as separate updates, eg. 4.0, 4.1, 4.2? I'm only suggesting this as it may help you streamline the releasing process if these indeed could be separated.

Best of luck with the AnyIO too, looks super cool!

[agronholm]

I'm trying to cram as many of the planned backwards incompatible changes into v4.0 as I possibly can. It may be years before I get around to making such a huge push again. Shared job stores is a core new feature in v4.0 so that has to go in. Enhanced async support is too. The trigger overhaul and broader serialization support is pretty much done already. What changes do you then propose I would leave to minor releases?

[Voyz]

What changes do you then propose I would leave to minor releases?

In my opinion - again very naively - I'd suggest to make each of these things you list a separate version update.

I'm trying to cram as many of the planned backwards incompatible changes into v4.0 as I possibly can.

Yeah, I understand if you want to go this way. I only wanted to suggest that having smaller independent releases would be easier and more reliable than one big all-breaking change. Both for you - as less work would be needed for each iteration and it would be easier to ensure that code is bug-free - and for the users - as the updates would be more frequent and easier to adapt to. That's just one point of view though, and I totally understand there may be good reasons for not doing it the way I suggest, especially if these changes you list rely on each other. Furthermore, if most of work is already done and tested, then my suggestions are even less relevant; maybe only as something to keep in mind in the future.

If there are some of these features you listed that you still haven't started working on - shared job stores, enhanced async support, or other - then my suggestion would be to see if they could become v4.1, v4.2, etc. or even v5.0. Again - only a suggestion.

[agronholm]

Making these changes incrementally, without changing the API, would be impossible since they alter the semantics so radically.

[agronholm]

Update: work on AnyIO v2.0 is virtually complete. There are a couple blocker issues I need to take care of before moving on with the release.

[Voyz]

Thanks for the update and congrats @agronholm! Looking forward to it big time 👌👌

[agronholm]

AnyIO v2.0.0b1 has been released. I will try out the new features first with my smtpproto side project. My main interest is using the new BlockingPortal feature to cheaply provide a synchronous alternative to the async API. If that works out, I will do the same with APScheduler.

[agronholm]

I've released smtpproto and the results of using BlockingPortal from that project were encouraging, so I will now focus on APScheduler. My first goal is to get a memory store and a simple executor working along with the async and sync schedulers. Persistent stores will follow.

[agronholm]

I've posted issue #465 to track v4.0 development so I will stop posting updates here.

[agronholm]

I've created #906 to track a better solution to this problem.