From c88f97c36e096203c22aa6ccc21a19dbd54d26b8 Mon Sep 17 00:00:00 2001 From: Evgenii Alekseev Date: Fri, 5 Jan 2024 16:11:32 +0200 Subject: [PATCH] refactor: simplify lock processing in worker trigger --- .../core/distributed/worker_trigger.py | 20 ++++++++----------- .../core/distributed/test_worker_trigger.py | 14 +++++-------- 2 files changed, 13 insertions(+), 21 deletions(-) diff --git a/src/ahriman/core/distributed/worker_trigger.py b/src/ahriman/core/distributed/worker_trigger.py index f18c8409..1bf77573 100644 --- a/src/ahriman/core/distributed/worker_trigger.py +++ b/src/ahriman/core/distributed/worker_trigger.py @@ -17,7 +17,6 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . # -from collections import deque from threading import Lock, Timer from ahriman.core.configuration import Configuration @@ -47,15 +46,14 @@ class WorkerTrigger(DistributedSystem): self.ping_interval = configuration.getint(section, "time_to_live", fallback=60) / 4.0 self._lock = Lock() - self._timers: deque[Timer] = deque() # because python doesn't have atomics + self._timer: Timer | None = None def create_timer(self) -> None: """ create timer object and put it to queue """ - timer = Timer(self.ping_interval, self.ping) - timer.start() - self._timers.append(timer) + self._timer = Timer(self.ping_interval, self.ping) + self._timer.start() def on_start(self) -> None: """ @@ -71,21 +69,19 @@ class WorkerTrigger(DistributedSystem): """ self.logger.info("removing instance %s in %s", self.worker, self.address) with self._lock: - current_timers = self._timers.copy() # will be used later - self._timers.clear() # clear timer list + if self._timer is None: + return - for timer in current_timers: - timer.cancel() # cancel remaining timers + self._timer.cancel() # cancel remaining timers + self._timer = None # reset state def ping(self) -> None: """ register itself as alive worker and update the timer """ with self._lock: - if not self._timers: # make sure that there is related specific timer + if self._timer is None: # no active timer set, exit loop return - self._timers.popleft() # pop first timer - self.register() self.create_timer() diff --git a/tests/ahriman/core/distributed/test_worker_trigger.py b/tests/ahriman/core/distributed/test_worker_trigger.py index 4c340a32..3754a10b 100644 --- a/tests/ahriman/core/distributed/test_worker_trigger.py +++ b/tests/ahriman/core/distributed/test_worker_trigger.py @@ -1,8 +1,6 @@ +from pytest_mock import MockerFixture from threading import Timer -from pytest_mock import MockerFixture - -from ahriman.core.configuration import Configuration from ahriman.core.distributed import WorkerTrigger @@ -11,10 +9,8 @@ def test_create_timer(worker_trigger: WorkerTrigger) -> None: must create a timer and put it to queue """ worker_trigger.create_timer() - - timer = worker_trigger._timers.popleft() - assert timer.function == worker_trigger.ping - timer.cancel() + assert worker_trigger._timer.function == worker_trigger.ping + worker_trigger._timer.cancel() def test_on_start(worker_trigger: WorkerTrigger, mocker: MockerFixture) -> None: @@ -31,7 +27,7 @@ def test_on_stop(worker_trigger: WorkerTrigger, mocker: MockerFixture) -> None: must unregister itself as worker """ run_mock = mocker.patch("threading.Timer.cancel") - worker_trigger._timers.append(Timer(1, print)) # doesn't matter + worker_trigger._timer = Timer(1, print) # doesn't matter worker_trigger.on_stop() run_mock.assert_called_once_with() @@ -50,7 +46,7 @@ def test_ping(worker_trigger: WorkerTrigger, mocker: MockerFixture) -> None: """ run_mock = mocker.patch("ahriman.core.distributed.WorkerTrigger.register") timer_mock = mocker.patch("threading.Timer.start") - worker_trigger._timers.append(Timer(1, print)) # doesn't matter + worker_trigger._timer = Timer(1, print) # doesn't matter worker_trigger.ping() run_mock.assert_called_once_with()