diff --git a/queue_job/jobrunner/__init__.py b/queue_job/jobrunner/__init__.py index e2561b0e7..267cac4e1 100644 --- a/queue_job/jobrunner/__init__.py +++ b/queue_job/jobrunner/__init__.py @@ -20,7 +20,7 @@ queue_job_config = config.misc.get("queue_job", {}) -from .runner import QueueJobRunner, _channels +from .runner import QueueJobRunner, _channels, _max_capacity _logger = logging.getLogger(__name__) @@ -87,7 +87,9 @@ def signal_time_expired_handler(self, n, stack): def _is_runner_enabled(): - return not _channels().strip().startswith("root:0") + if _channels().strip().startswith("root:0"): + return False + return _max_capacity() != 0 def _start_runner_thread(server_type): @@ -100,7 +102,8 @@ def _start_runner_thread(server_type): else: _logger.info( "jobrunner thread (in %s) NOT started, " - "because the root channel's capacity is set to 0", + "because the root channel's capacity or the max capacity " + "is set to 0", server_type, ) diff --git a/queue_job/jobrunner/channels.py b/queue_job/jobrunner/channels.py index e3cb48042..4d7b441e2 100644 --- a/queue_job/jobrunner/channels.py +++ b/queue_job/jobrunner/channels.py @@ -3,6 +3,7 @@ # License LGPL-3.0 or later (http://www.gnu.org/licenses/lgpl.html) import logging from collections import namedtuple +from dataclasses import asdict, dataclass from functools import total_ordering from heapq import heappop, heappush from weakref import WeakValueDictionary @@ -10,12 +11,24 @@ from ..exception import ChannelNotFound from ..job import CANCELLED, DONE, ENQUEUED, FAILED, PENDING, STARTED, WAIT_DEPENDENCIES +RELOAD_PAYLOAD = "reload" NOT_DONE = (WAIT_DEPENDENCIES, PENDING, ENQUEUED, STARTED, FAILED) JobSortingKey = namedtuple("SortingKey", "eta priority date_created seq") _logger = logging.getLogger(__name__) +@dataclass +class ChannelConfig: + """Configuration of a channel""" + + name: str + capacity: int = 0 + sequential: bool = False + throttle: int = 0 + paused: bool = False + + class PriorityQueue: """A priority queue that supports removing arbitrary objects. @@ -965,6 +978,11 @@ def simple_configure(self, config_string): for config in ChannelManager.parse_simple_config(config_string): self.get_channel_from_config(config) + def configure(self, configs): + """Configure the channel manager from list of :class:`ChannelConfig`""" + for config in configs: + self.get_channel_from_config(asdict(config)) + def get_channel_from_config(self, config): """Return a Channel object from a parsed configuration. @@ -1115,3 +1133,8 @@ def get_jobs_to_run(self, now): def get_wakeup_time(self): return self._root_channel.get_wakeup_time() + + @property + def running_count(self) -> int: + """Number of jobs currently running""" + return len(self._root_channel._running) diff --git a/queue_job/jobrunner/runner.py b/queue_job/jobrunner/runner.py index 95e134ba4..2adb12508 100644 --- a/queue_job/jobrunner/runner.py +++ b/queue_job/jobrunner/runner.py @@ -19,6 +19,7 @@ anonymous ``/queue_job/runjob`` HTTP request. """ +import fnmatch import logging import os import selectors @@ -34,7 +35,7 @@ from odoo.tools import config from . import queue_job_config -from .channels import ENQUEUED, NOT_DONE, ChannelManager +from .channels import ENQUEUED, NOT_DONE, RELOAD_PAYLOAD, ChannelConfig, ChannelManager SELECT_TIMEOUT = 60 ERROR_RECOVERY_DELAY = 5 @@ -57,6 +58,118 @@ class MasterElectionLost(Exception): # so we check it in addition to the environment variables. +def _server_side_channels_configured(): + return bool( + os.environ.get("ODOO_QUEUE_JOB_CHANNELS") or queue_job_config.get("channels") + ) + + +def _root_capacity_from_channels_config(config_string): + """Capacity of the root channel from the channels string + + >>> _root_capacity_from_channels_config('root:4,sub:2') + 4 + >>> _root_capacity_from_channels_config('sub:2') + 1 + >>> _root_capacity_from_channels_config('root:0') + 0 + """ + for channel_config in ChannelManager.parse_simple_config(config_string): + if channel_config["name"] == "root": + return channel_config.get("capacity", 1) + return 1 + + +def _max_capacity(channel_config_string: str | None = None) -> int: + """Maximum number of jobs running at the same time across all databases + + If a channels server-side configuration exists, it is equivalent to the + capacity of the root channel. + + Otherwise, it comes from the ``ODOO_QUEUE_JOB_MAX_CAPACITY`` environment + variable, then ``max_capacity`` in the ``[queue_job]`` section of the + configuration file. + + If none is configured, the max capacity is 0. + """ + if _server_side_channels_configured(): + if channel_config_string is None: + channel_config_string = _channels() + return _root_capacity_from_channels_config(channel_config_string) + + value = os.environ.get("ODOO_QUEUE_JOB_MAX_CAPACITY") or queue_job_config.get( + "max_capacity" + ) + if value: + return int(value) + return 0 + + +def _db_max_capacity() -> str: + return ( + os.environ.get("ODOO_QUEUE_JOB_DB_MAX_CAPACITY") + or queue_job_config.get("db_max_capacity") + or "" + ) + + +def parse_db_max_capacity(spec): + """Parse a per-database max capacity configuration string + + The string is a comma-separated list of ``pattern:capacity`` items, where + ``pattern`` matches database names with fnmatch wildcards. + + The first matching pattern wins, so specific patterns must be first in the + string. + + A single integer is applied to all databases, as a shorthand for + ``*:capacity``. + + >>> parse_db_max_capacity('prod_*:20,staging:2,*:5') + [('prod_*', 20), ('staging', 2), ('*', 5)] + >>> parse_db_max_capacity('8') + [('*', 8)] + >>> parse_db_max_capacity('') + [] + >>> parse_db_max_capacity(None) + [] + """ + rules = [] + if not spec: + return rules + for item in spec.replace("\n", ",").split(","): + item = item.strip() + if not item: + continue + pattern, sep, capacity = item.rpartition(":") + if not sep: + pattern = "*" + try: + rules.append((pattern.strip(), int(capacity))) + except ValueError as ex: + raise ValueError(f"Invalid db max capacity {spec}: {capacity}") from ex + return rules + + +def db_max_capacity_for(db_name, rules, default=None): + """Max capacity of a database, first match wins + + >>> rules = parse_db_max_capacity('prod_*:20,staging:2,*:5') + >>> db_max_capacity_for('prod_foo', rules) + 20 + >>> db_max_capacity_for('staging', rules) + 2 + >>> db_max_capacity_for('dev', rules) + 5 + >>> db_max_capacity_for('dev', [], default=7) + 7 + """ + for pattern, capacity in rules: + if fnmatch.fnmatch(db_name, pattern): + return capacity + return default + + def _channels(): return ( os.environ.get("ODOO_QUEUE_JOB_CHANNELS") @@ -182,6 +295,30 @@ def _initialize(self): with closing(self.conn.cursor()) as cr: cr.execute("LISTEN queue_job") + def load_channels_config(self): + """Return the channels configuration stored in the database""" + with closing(self.conn.cursor()) as cr: + cr.execute( + "SELECT complete_name, " + "COALESCE(capacity, 0), " + "COALESCE(sequential, false), " + "COALESCE(throttle, 0), " + "COALESCE(paused, false) " + "FROM queue_job_channel " + ) + rows = cr.fetchall() + configs = [ + ChannelConfig( + name=name, + capacity=capacity, + sequential=sequential, + throttle=throttle, + paused=paused, + ) + for name, capacity, sequential, throttle, paused in rows + ] + return configs + @contextmanager def select_jobs(self, where, args): # pylint: disable=sql-injection @@ -321,16 +458,37 @@ def __init__( user=None, password=None, channel_config_string=None, + max_capacity=None, + db_max_capacity=None, ): self.scheme = scheme self.host = host self.port = port self.user = user self.password = password - self.channel_manager = ChannelManager() + if channel_config_string is None: channel_config_string = _channels() - self.channel_manager.simple_configure(channel_config_string) + + self._server_side_channel_manager = None + if _server_side_channels_configured(): + channel_manager = ChannelManager() + channel_manager.simple_configure(channel_config_string) + self._server_side_channel_manager = channel_manager + + self._channel_manager_by_db = {} + self._channel_managers = [] + + if max_capacity is None: + max_capacity = _max_capacity() + self.max_capacity = max_capacity + + if db_max_capacity is None: + db_max_capacity = _db_max_capacity() + self.db_max_capacity_rules = parse_db_max_capacity(db_max_capacity) + + self._round_robin_offset = 0 + self.db_by_name = {} self._stop = False self._stop_pipe = os.pipe() @@ -387,11 +545,66 @@ def close_databases(self, remove_jobs=True): for db_name, db in self.db_by_name.items(): try: if remove_jobs: - self.channel_manager.remove_db(db_name) + self._channel_manager_by_db[db_name].remove_db(db_name) db.close() except Exception: _logger.warning("error closing database %s", db_name, exc_info=True) self.db_by_name = {} + self._channel_manager_by_db = {} + self._channel_managers = [] + + @staticmethod + def _unique_channel_managers(channel_managers): + seen = set() + result = [] + for channel_manager in channel_managers: + if id(channel_manager) not in seen: + seen.add(id(channel_manager)) + result.append(channel_manager) + return result + + def _build_channel_manager(self, db): + """Build and configure the channel manager of a database""" + db_max = db_max_capacity_for( + db.db_name, self.db_max_capacity_rules, default=self.max_capacity + ) + channels_config = db.load_channels_config() + channel_manager = ChannelManager() + + root_config = next( + (config for config in channels_config if config.name == "root"), None + ) + if root_config is None: + root_config = ChannelConfig("root") + channels_config.insert(0, root_config) + + if not db_max: + # if a database is set at 0, it does not run any jobs, pause it + root_config.paused = True + elif not root_config.capacity: + root_config.capacity = db_max + else: + root_config.capacity = min(root_config.capacity, db_max) + channel_manager.configure(channels_config) + return channel_manager + + def _reconfigure_db(self, db_name): + """Rebuild the channel manager for a database and reload its jobs""" + db = self.db_by_name.get(db_name) + if db is None: + return + if self._server_side_channel_manager: + channel_manager = self._server_side_channel_manager + else: + channel_manager = self._build_channel_manager(db) + with db.select_jobs("state in %s", (NOT_DONE,)) as cr: + for job_data in cr: + channel_manager.notify(db_name, *job_data) + self._channel_manager_by_db[db_name] = channel_manager + self._channel_managers = self._unique_channel_managers( + self._channel_manager_by_db.values() + ) + _logger.info("channels configuration loaded for db %s", db_name) def initialize_databases(self): for db_name in sorted(self.get_db_names()): @@ -399,9 +612,7 @@ def initialize_databases(self): db = Database(db_name) if db.has_queue_job: self.db_by_name[db_name] = db - with db.select_jobs("state in %s", (NOT_DONE,)) as cr: - for job_data in cr: - self.channel_manager.notify(db_name, *job_data) + self._reconfigure_db(db_name) _logger.info("queue job runner ready for db %s", db_name) else: db.close() @@ -411,24 +622,60 @@ def requeue_dead_jobs(self): if db.has_queue_job: db.requeue_dead_jobs() + def _all_running_count(self) -> int: + return sum( + channel_manager.running_count for channel_manager in self._channel_managers + ) + + def _dispatch_job(self, job): + _logger.info("asking Odoo to run job %s on db %s", job.uuid, job.db_name) + self.db_by_name[job.db_name].set_job_enqueued(job.uuid) + _async_http_get( + self.scheme, + self.host, + self.port, + self.user, + self.password, + job.db_name, + job.uuid, + ) + def run_jobs(self): + channel_managers = self._channel_managers + if not channel_managers: + return + now = _odoo_now() - for job in self.channel_manager.get_jobs_to_run(now): - if self._stop: - break - _logger.info("asking Odoo to run job %s on db %s", job.uuid, job.db_name) - self.db_by_name[job.db_name].set_job_enqueued(job.uuid) - _async_http_get( - self.scheme, - self.host, - self.port, - self.user, - self.password, - job.db_name, - job.uuid, - ) + + round_robin_offset = self._round_robin_offset % len(channel_managers) + self._round_robin_offset += 1 + + # rotate the channel managers order so that each database gets a chance + # to enqueue jobs under contention + for channel_manager in ( + channel_managers[round_robin_offset:] + + channel_managers[:round_robin_offset] + ): + jobs = channel_manager.get_jobs_to_run(now) + while True: + if self._stop: + break + if self.max_capacity and self._all_running_count() >= self.max_capacity: + # we trigger a round-robin only when the global max + # capacity is reached, before that, all channel managers can + # already enqueue jobs + _logger.debug( + "max capacity of %s reached, round-robin to next db", + self.max_capacity, + ) + return + job = next(jobs, None) + if job is None: + break + self._dispatch_job(job) def process_notifications(self): + reload_db_names = set() for db in self.db_by_name.values(): if not db.conn.notifies: # If there are no activity in the queue_job table it seems that @@ -440,13 +687,29 @@ def process_notifications(self): if self._stop: break notification = db.conn.notifies.pop() - uuid = notification.payload + payload = notification.payload + if payload == RELOAD_PAYLOAD and not self._server_side_channel_manager: + reload_db_names.add(db.db_name) + continue + + uuid = payload + channel_manager = self._channel_manager_by_db[db.db_name] with db.select_jobs("uuid = %s", (uuid,)) as cr: job_datas = cr.fetchone() if job_datas: - self.channel_manager.notify(db.db_name, *job_datas) + channel_manager.notify(db.db_name, *job_datas) else: - self.channel_manager.remove_job(uuid) + channel_manager.remove_job(uuid) + + for db_name in reload_db_names: + self._reconfigure_db(db_name) + + def next_wakeup_time(self): + wakeup_times = [ + channel_manager.get_wakeup_time() + for channel_manager in self._channel_managers + ] + return min(wakeup_times, default=0) def wait_notification(self): for db in self.db_by_name.values(): @@ -458,7 +721,7 @@ def wait_notification(self): conns = [db.conn for db in self.db_by_name.values()] conns.append(self._stop_pipe[0]) # look if the channels specify a wakeup time - wakeup_time = self.channel_manager.get_wakeup_time() + wakeup_time = self.next_wakeup_time() if not wakeup_time: # this could very well be no timeout at all, because # any activity in the job queue will wake us up, but diff --git a/queue_job/models/queue_job_channel.py b/queue_job/models/queue_job_channel.py index 4aabb0188..47e5bd62a 100644 --- a/queue_job/models/queue_job_channel.py +++ b/queue_job/models/queue_job_channel.py @@ -4,12 +4,19 @@ from odoo import _, api, exceptions, fields, models +from ..jobrunner.channels import RELOAD_PAYLOAD + class QueueJobChannel(models.Model): _name = "queue.job.channel" _description = "Job Channels" _rec_name = "complete_name" + # fields that trigger a reload of the jobrunner for this database when changed + _JOBRUNNER_CONFIG_FIELDS = frozenset( + ("capacity", "sequential", "throttle", "paused", "name", "parent_id") + ) + name = fields.Char() complete_name = fields.Char( compute="_compute_complete_name", store=True, readonly=True, recursive=True @@ -25,11 +32,44 @@ class QueueJobChannel(models.Model): removal_interval = fields.Integer( default=lambda self: self.env["queue.job"]._removal_interval, required=True ) + capacity = fields.Integer( + help="Maximum number of jobs running at the same time in this channel. " + "0 means no limit, but they are still limited by the capacity of the parent " + "channel. On the root channel, 0 is limited by the global server-side " + "configuration." + ) + sequential = fields.Boolean( + help="Jobs are executed one after the other and failed jobs block the channel. " + "Requires a capacity of 1." + ) + throttle = fields.Integer( + help="Minimum delay in seconds between the start of two jobs in this channel." + ) + paused = fields.Boolean( + help="A paused channel (an its sub-channels) do not execute any jobs until " + "resumed." + ) _sql_constraints = [ ("name_uniq", "unique(complete_name)", "Channel complete name must be unique") ] + @api.constrains("capacity", "sequential", "throttle") + def _check_jobrunner_configuration(self): + for record in self: + if record.capacity < 0: + raise exceptions.ValidationError( + self.env._("The capacity of a channel cannot be negative.") + ) + if record.throttle < 0: + raise exceptions.ValidationError( + self.env._("The throttle of a channel cannot be negative.") + ) + if record.sequential and record.capacity != 1: + raise exceptions.ValidationError( + self.env._("A sequential channel must have a capacity of 1.") + ) + @api.depends("name", "parent_id.complete_name") def _compute_complete_name(self): for record in self: @@ -70,6 +110,7 @@ def create(self, vals_list): new_vals_list.append(vals) vals_list = new_vals_list records |= super().create(vals_list) + records._notify_channel_config_changed() return records def write(self, values): @@ -80,10 +121,19 @@ def write(self, values): and ("name" in values or "parent_id" in values) ): raise exceptions.UserError(_("Cannot change the root channel")) - return super().write(values) + res = super().write(values) + if self._JOBRUNNER_CONFIG_FIELDS.intersection(values): + self._notify_channel_config_changed() + return res def unlink(self): for channel in self: if channel.name == "root": raise exceptions.UserError(_("Cannot remove the root channel")) - return super().unlink() + res = super().unlink() + self._notify_channel_config_changed() + return res + + def _notify_channel_config_changed(self): + """Notify the jobrunner to reload its configuration""" + self.env.cr.execute("SELECT pg_notify('queue_job', %s)", (RELOAD_PAYLOAD,)) diff --git a/queue_job/tests/test_model_job_channel.py b/queue_job/tests/test_model_job_channel.py index 20ebbc0bf..1277071ef 100644 --- a/queue_job/tests/test_model_job_channel.py +++ b/queue_job/tests/test_model_job_channel.py @@ -1,9 +1,12 @@ # copyright 2018 Camptocamp # license lgpl-3.0 or later (http://www.gnu.org/licenses/lgpl.html) +from unittest import mock + from psycopg2 import IntegrityError import odoo +from odoo import exceptions from odoo.tests import common @@ -57,3 +60,83 @@ def test_channel_display_name(self): {"name": "test", "parent_id": self.root_channel.id} ) self.assertEqual(channel.display_name, channel.complete_name) + + def test_capacity_should_not_be_negative(self): + with self.assertRaisesRegex( + exceptions.ValidationError, + "The capacity of a channel cannot be negative.", + ): + self.Channel.create( + { + "name": "test_capacity", + "parent_id": self.root_channel.id, + "capacity": -1, + } + ) + + def test_throttle_should_not_be_negative(self): + with self.assertRaisesRegex( + exceptions.ValidationError, + "The throttle of a channel cannot be negative.", + ): + self.Channel.create( + { + "name": "test_throttle", + "parent_id": self.root_channel.id, + "throttle": -1, + } + ) + + def test_sequential_should_have_capacity_one(self): + with self.assertRaisesRegex( + exceptions.ValidationError, + "A sequential channel must have a capacity of 1.", + ): + self.Channel.create( + { + "name": "test_sequential", + "parent_id": self.root_channel.id, + "sequential": True, + "capacity": 2, + } + ) + + def _patch_notify(self): + return mock.patch.object( + type(self.Channel), "_notify_channel_config_changed", autospec=True + ) + + def test_notify_create_channel(self): + with self._patch_notify() as notify: + self.Channel.create( + { + "name": "create_notify", + "parent_id": self.root_channel.id, + "capacity": 2, + } + ) + notify.assert_called_once() + + def test_notify_write_jobrunner_config(self): + channel = self.Channel.create( + {"name": "write_notify", "parent_id": self.root_channel.id} + ) + with self._patch_notify() as notify: + channel.capacity = 3 + notify.assert_called_once() + + with self._patch_notify() as notify: + channel.paused = True + notify.assert_called_once() + + with self._patch_notify() as notify: + channel.removal_interval = 60 + notify.assert_not_called() + + def test_notify_unlink_channel(self): + channel = self.Channel.create( + {"name": "unlink_notify", "parent_id": self.root_channel.id} + ) + with self._patch_notify() as notify: + channel.unlink() + notify.assert_called_once() diff --git a/queue_job/views/queue_job_channel_views.xml b/queue_job/views/queue_job_channel_views.xml index 50c245716..a67959643 100644 --- a/queue_job/views/queue_job_channel_views.xml +++ b/queue_job/views/queue_job_channel_views.xml @@ -19,6 +19,16 @@ + + + These properties will be used only if the server-side + channels configuration is not used. + + + + + + @@ -32,6 +42,8 @@ + + @@ -44,6 +56,11 @@ +
+ These properties will be used only if the server-side + channels configuration is not used. +