From ad05468257805d4e703e0be623a5471de739fd60 Mon Sep 17 00:00:00 2001 From: Valentin Lorentz Date: Sat, 2 May 2020 18:14:57 +0200 Subject: [PATCH] Scheduler: Preserve period offset on restarts. Partial fix for GH-397. --- plugins/Scheduler/plugin.py | 40 +++++++++++++++++++++++++++++++++---- plugins/Scheduler/test.py | 34 +++++++++++++++++++++---------- src/schedule.py | 24 ++++++++++++++-------- 3 files changed, 75 insertions(+), 23 deletions(-) diff --git a/plugins/Scheduler/plugin.py b/plugins/Scheduler/plugin.py index e8c279036..af33b499c 100644 --- a/plugins/Scheduler/plugin.py +++ b/plugins/Scheduler/plugin.py @@ -29,6 +29,7 @@ import time import os +import math import shutil import tempfile @@ -56,6 +57,14 @@ class Scheduler(callbacks.Plugin): self._restoreEvents(irc) world.flushers.append(self._flush) + def _getNextRunIn(self, first_run, now, period): + next_run_in = period - ((now - first_run) % period) + if next_run_in < 5: + # don't run immediatly, it might overwhelm the bot on + # startup. + next_run_in += period + return next_run_in + def _restoreEvents(self, irc): try: pkl = open(filename, 'rb') @@ -82,8 +91,21 @@ class Scheduler(callbacks.Plugin): self._add(ircobj, event['msg'], event['time'], event['command'], n) elif event['type'] == 'repeat': # repeating event + now = time.time() + first_run = event.get('first_run') + if first_run is None: + # old DBs don't have a "first_run"; let's take "now" as + # first_run. + first_run = now + + # Preserve the offset over restarts; eg. if event['time'] + # is 24hours, we want to keep running the command at the + # same time of day. + next_run_in = self._getNextRunIn( + first_run, now, event['time']) + self._repeat(ircobj, event['msg'], name, - event['time'], event['command'], False) + event['time'], event['command'], first_run, next_run_in) except AssertionError as e: if str(e) == 'An event with the same name has already been scheduled.': # we must be reloading the plugin, event is still scheduled @@ -166,14 +188,24 @@ class Scheduler(callbacks.Plugin): irc.error(_('Invalid event id.')) remove = wrap(remove, ['lowered']) - def _repeat(self, irc, msg, name, seconds, command, now=True): + def _repeat(self, irc, msg, name, seconds, command, first_run=None, next_run_in=None): f = self._makeCommandFunction(irc, msg, command, remove=False) - id = schedule.addPeriodicEvent(f, seconds, name, now) + f_wrapper = schedule.schedule.makePeriodicWrapper(f, seconds, name) + if next_run_in is None: + assert first_run is None + # run immediately + id = f_wrapper() + first_run = time.time() + else: + assert first_run is not None + id = schedule.addEvent(f_wrapper, time.time() + next_run_in, name) assert id == name self.events[name] = {'command':command, 'msg':msg, 'time':seconds, - 'type':'repeat'} + 'type':'repeat', + 'first_run': first_run, + } @internationalizeDocstring def repeat(self, irc, msg, args, name, seconds, command): diff --git a/plugins/Scheduler/test.py b/plugins/Scheduler/test.py index 108c8393d..660c373f5 100644 --- a/plugins/Scheduler/test.py +++ b/plugins/Scheduler/test.py @@ -122,25 +122,37 @@ class SchedulerTestCase(ChannelPluginTestCase): def testRepeatPersistence(self): self.assertRegexp( - 'scheduler repeat repeater 5 echo testRepeat', + 'scheduler repeat repeater 20 echo testRepeat', 'testRepeat') + self.assertNotError('unload Scheduler') + schedule.schedule.reset() + timeFastForward(30) + self.assertNoResponse(' ', timeout=1) + + self.assertNotError('load Scheduler') + self.assertNoResponse(' ', timeout=1) # T+30 to T+31 + timeFastForward(5) + self.assertNoResponse(' ', timeout=1) # T+36 to T+37 + timeFastForward(5) + self.assertResponse(' ', 'testRepeat', timeout=1) # T+42 + + timeFastForward(15) + self.assertNoResponse(' ', timeout=1) # T+57 to T+58 + timeFastForward(5) + self.assertResponse(' ', 'testRepeat', timeout=1) # T+64 + self.assertNotError('unload Scheduler') schedule.schedule.reset() timeFastForward(20) self.assertNoResponse(' ', timeout=1) self.assertNotError('load Scheduler') - self.assertNoResponse(' ', timeout=1) - timeFastForward(2) - self.assertNoResponse(' ', timeout=1) - timeFastForward(2) - self.assertResponse(' ', 'testRepeat') - - timeFastForward(3) - self.assertNoResponse(' ', timeout=1) - timeFastForward(2) - self.assertResponse(' ', 'testRepeat') + self.assertNoResponse(' ', timeout=1) # T+85 to T+86 + timeFastForward(10) + self.assertNoResponse(' ', timeout=1) # T+95 to T+96 + timeFastForward(10) + self.assertResponse(' ', 'testRepeat', timeout=1) # T+106 diff --git a/src/schedule.py b/src/schedule.py index 1a66cc98b..cbe21702a 100644 --- a/src/schedule.py +++ b/src/schedule.py @@ -108,19 +108,27 @@ class Schedule(drivers.IrcDriver): f = self.removeEvent(name) self.addEvent(f, t, name=name) - def addPeriodicEvent(self, f, t, name=None, now=True, args=[], kwargs={}, - count=None): - """Adds a periodic event that is called every t seconds.""" - def wrapper(count): + def makePeriodicWrapper( + self, f, t, name=None, args=[], kwargs={}, count=None): + """Returns a function that will run and re-schedule itself every t + seconds.""" + def wrapper(): + nonlocal count try: f(*args, **kwargs) finally: # Even if it raises an exception, let's schedule it. - if count[0] is not None: - count[0] -= 1 - if count[0] is None or count[0] > 0: + if count is not None: + count -= 1 + if count is None or count > 0: return self.addEvent(wrapper, time.time() + t, name) - wrapper = functools.partial(wrapper, [count]) + return wrapper + + def addPeriodicEvent( + self, f, t, name=None, now=True, args=[], kwargs={}, count=None): + """Adds a periodic event that is called every t seconds.""" + wrapper = self.makePeriodicWrapper( + f, t, name, args, kwargs, count) if now: return wrapper() else: