From 4c0efe9db00f43039b9d84312b7374860d89c099 Mon Sep 17 00:00:00 2001 From: catborise Date: Fri, 6 Nov 2026 09:02:54 +0300 Subject: [PATCH] fix(worker): a database error skips a tick instead of ending the worker Any error in the manager loop ended the worker at once with os._exit, without the drain: a PostgreSQL restart or a SQLite write lock held past its timeout cut every running migration, block copy and backup the worker drove, and their jobs went to recovery, some ending uncertain. A database error now skips that tick and drops the broken connection; the next tick tries again. A lease lost meanwhile still ends the worker when renew sees it, and other errors still end it as before. --- operations/tests/test_worker.py | 23 ++++++++++++++++++++++- operations/worker.py | 13 ++++++++++--- 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/operations/tests/test_worker.py b/operations/tests/test_worker.py index 9b9f0991..5947d4e7 100644 --- a/operations/tests/test_worker.py +++ b/operations/tests/test_worker.py @@ -256,12 +256,33 @@ class WorkerTestCase(TransactionTestCase): self.assertIn("no context", words(Operation.objects.get(pk=op.pk).error)) self.assertFalse(Claim.objects.filter(operation=op).exists()) + def test_a_database_error_skips_a_tick(self): + directory = tempfile.TemporaryDirectory() + self.addCleanup(directory.cleanup) + self.worker = Worker(data=directory.name, tick=0) + errors = [OperationalError("database is locked"), OperationalError("server closed the connection")] + + def tick(): + if errors: + raise errors.pop() + self.worker.stopping.set() + with ( + patch.object(self.worker, "tick_once", side_effect=tick), + patch("operations.worker.signal.signal"), + patch("operations.worker.logging.shutdown"), + patch("operations.worker.os._exit", side_effect=SystemExit) as exit_, + self.assertLogs("operations.worker", "ERROR"), + ): + with self.assertRaises(SystemExit): + self.worker.run() + exit_.assert_called_once_with(0) + def test_a_failing_manager_loop_ends_the_process(self): directory = tempfile.TemporaryDirectory() self.addCleanup(directory.cleanup) self.worker = Worker(data=directory.name) with ( - patch.object(self.worker, "tick_once", side_effect=OperationalError("database is down")), + patch.object(self.worker, "tick_once", side_effect=KeyError("a bug")), patch("operations.worker.signal.signal"), patch("operations.worker.logging.shutdown"), patch("operations.worker.os._exit", side_effect=SystemExit) as exit_, diff --git a/operations/worker.py b/operations/worker.py index d87446bd..9e9efd07 100644 --- a/operations/worker.py +++ b/operations/worker.py @@ -19,7 +19,7 @@ from importlib import import_module from pathlib import Path from django.conf import settings -from django.db import close_old_connections +from django.db import DatabaseError, close_old_connections from django.utils import timezone from django.utils.translation import gettext_noop @@ -116,7 +116,8 @@ class Worker: def run(self): """Until SIGTERM. Returns 1 if another worker runs on this host. Ends with os._exit, so job threads past the grace period end with it; a - failing manager loop ends the process the same way (status 1).""" + manager loop failing other than on the database ends the process the + same way (status 1).""" lock = open(self.lock_path, "a") try: fcntl.flock(lock, fcntl.LOCK_EX | fcntl.LOCK_NB) @@ -133,7 +134,13 @@ class Worker: logger.info("Worker %s started on %s", self.identity, socket.gethostname()) self.recovery.schedule_orphans() while not self.stopping.is_set(): - self.tick_once() + try: + self.tick_once() + except DatabaseError as err: + # a database restart or a long lock ends no job: the next tick tries + # again, and a lease lost meanwhile still ends the worker (renew) + logger.error("Worker %s tick failed: %s", self.identity, utils.safe_text(err)) + close_old_connections() self.stopping.wait(self.tick) self._drain() except Exception as err: