From bee8fade5ecc39770120c4ea1b865c6e97e9f4b5 Mon Sep 17 00:00:00 2001 From: Xavier-Do Date: Mon, 20 Apr 2026 12:46:17 +0200 Subject: [PATCH] [IMP] runbot: delayed global state and result This pr gives the responsability to compute the global state and result to the scheduler, avoiding the need to write on a parent when updating a child build. This should avoid concurrent updates while ensuring a good consistency. --- runbot/models/build.py | 89 ++++++++++++++++---------- runbot/models/runbot.py | 4 ++ runbot/tests/test_build.py | 28 ++++++++ runbot/tests/test_build_config_step.py | 2 + runbot/tests/test_upgrade.py | 4 ++ 5 files changed, 94 insertions(+), 33 deletions(-) diff --git a/runbot/models/build.py b/runbot/models/build.py index 12ecc007d..117f6ed5d 100644 --- a/runbot/models/build.py +++ b/runbot/models/build.py @@ -291,9 +291,9 @@ class BuildResult(models.Model): priority_level = fields.Integer('Priority', related='create_batch_id.priority_level', store=True, index=True) # state machine - global_state = fields.Selection(make_selection(state_order), string='Status', compute='_compute_global_state', store=True, recursive=True) + global_state = fields.Selection(make_selection(state_order), string='Status', default='pending') local_state = fields.Selection(make_selection(state_order), string='Build Status', default='pending', required=True, index=True) - global_result = fields.Selection(make_selection(result_order), string='Result', compute='_compute_global_result', store=True, recursive=True) + global_result = fields.Selection(make_selection(result_order), string='Result', default='ok') local_result = fields.Selection(make_selection(result_order), string='Build Result', default='ok') requested_action = fields.Selection([('wake_up', 'To wake up'), ('deathrow', 'To kill')], string='Action requested', index=True) @@ -371,6 +371,9 @@ class BuildResult(models.Model): access_token = fields.Char('Token', default=lambda self: uuid.uuid4().hex) + _global_state_idx = models.Index("(global_state) WHERE global_state != 'done'") + + @api.depends('description', 'params_id.config_id') def _compute_display_name(self): for build in self: @@ -382,8 +385,7 @@ def _compute_host_id(self): for record in self: record.host_id = get_host(record.host) - @api.depends('children_ids.global_state', 'local_state') - def _compute_global_state(self): + def _update_global_state(self): for record in self: waiting_score = record._get_state_score('waiting') children_ids = [child for child in record.children_ids if not child.orphan_result] @@ -396,6 +398,21 @@ def _compute_global_state(self): else: record.global_state = record.local_state + def _update_global_result(self): + for record in self: + if record.local_result and record._get_result_score(record.local_result) >= record._get_result_score('ko'): + record.global_result = record.local_result + else: + children_ids = [child for child in record.children_ids if not child.orphan_result] + if children_ids: + children_result = record._get_worst_result([child.global_result for child in children_ids], max_res='ko') + if record.local_result: + record.global_result = record._get_worst_result([record.local_result, children_result]) + else: + record.global_result = children_result + else: + record.global_result = record.local_result + @api.depends('message_ids') def _compute_to_kill(self): for record in self: @@ -432,22 +449,6 @@ def _get_youngest_state(self, states): def _get_state_score(self, result): return state_order.index(result) - @api.depends('children_ids.global_result', 'local_result', 'children_ids.orphan_result') - def _compute_global_result(self): - for record in self: - if record.local_result and record._get_result_score(record.local_result) >= record._get_result_score('ko'): - record.global_result = record.local_result - else: - children_ids = [child for child in record.children_ids if not child.orphan_result] - if children_ids: - children_result = record._get_worst_result([child.global_result for child in children_ids], max_res='ko') - if record.local_result: - record.global_result = record._get_worst_result([record.local_result, children_result]) - else: - record.global_result = children_result - else: - record.global_result = record.local_result - @api.depends('build_error_link_ids') def _compute_build_error_ids(self): for record in self: @@ -493,6 +494,17 @@ def copy_data(self, default=None): }) return [values] + def create(self, vals): + records = super().create(vals) + parents = records.parent_id + # it doesn't make sense to create a build in another state than pending, and ok result, + # so we can assume that we don't need to update the created records globals, + # but we need to update the parents global state and result as the new build can impact them + if parents: + parents._update_global_state() + parents._update_global_result() + return records + def write(self, values): # some validation to ensure db consistency if 'local_state' in values: @@ -506,22 +518,33 @@ def write(self, values): values.pop('local_result') else: raise ValidationError('Local result cannot be set to a less critical level') - init_global_results = self.mapped('global_result') - init_global_states = self.mapped('global_state') - init_local_states = self.mapped('local_state') + if "global_result" in values: + init_global_results = self.mapped('global_result') + if "global_state" in values: + init_global_states = self.mapped('global_state') + if "local_state" in values: + init_local_states = self.mapped('local_state') res = super(BuildResult, self).write(values) - for init_global_result, build in zip(init_global_results, self): - if init_global_result != build.global_result: - build._github_status() + if 'local_state' in values: + self._update_global_state() + if 'local_result' in values: + self._update_global_result() + + if "global_result" in values: + for init_global_result, build in zip(init_global_results, self): + if init_global_result != build.global_result: + build._github_status() - for init_local_state, build in zip(init_local_states, self): - if init_local_state not in ('done', 'running') and build.local_state in ('done', 'running'): - build.build_end = now() + if "local_state" in values: + for init_local_state, build in zip(init_local_states, self): + if init_local_state not in ('done', 'running') and build.local_state in ('done', 'running'): + build.build_end = now() - for init_global_state, build in zip(init_global_states, self): - if init_global_state not in ('done', 'running') and build.global_state in ('done', 'running'): - build._github_status() + if "global_state" in values: + for init_global_state, build in zip(init_global_states, self): + if init_global_state not in ('done', 'running') and build.global_state in ('done', 'running'): + build._github_status() return res @@ -641,7 +664,7 @@ def _rebuild(self, message=None): new_build = self.create(values) if self.parent_id: - new_build._github_status() + new_build._github_status() # not sure this is needed since creating a child should trigger an update of parent global state. user = self.env.user new_build._log('rebuild', 'Rebuild initiated by %s%s' % (user.name, (' :%s' % message) if message else '')) diff --git a/runbot/models/runbot.py b/runbot/models/runbot.py index b470e1df9..5e04912b6 100644 --- a/runbot/models/runbot.py +++ b/runbot/models/runbot.py @@ -72,6 +72,10 @@ def _scheduler(self, host): self._commit() processed += self._assign_pending_builds(host, host.nb_worker and host.nb_worker + 1, [('build_type', '=', 'priority')]) self._commit() + for build in host._get_builds([('global_state', 'in', ['pending', 'testing', 'waiting', 'running'])]): + build._update_global_state() + build._update_global_result() + self._commit() self._gc_running(host) self._commit() self._reload_nginx() diff --git a/runbot/tests/test_build.py b/runbot/tests/test_build.py index 3fc8cef24..776dfaf2a 100644 --- a/runbot/tests/test_build.py +++ b/runbot/tests/test_build.py @@ -618,6 +618,11 @@ def test_children(self): build1_1_2.local_state = 'done' + # simulate scheduler ran + build1_1._update_global_state() + build1_2._update_global_state() + build1._update_global_state() + self.assertEqual('done', build1.global_state) self.assertEqual('done', build1_1.global_state) self.assertEqual('done', build1_2.global_state) @@ -647,6 +652,14 @@ def test_rebuild_sub_sub_build(self): build1_1_1.local_result = 'ko' build1_1_1.local_state = 'done' + + + # simulate scheduler ran + build1_1._update_global_state() + build1._update_global_state() + build1_1._update_global_result() + build1._update_global_result() + self.assertEqual('done', build1.global_state) self.assertEqual('done', build1_1.global_state) self.assertEqual('done', build1_1_1.global_state) @@ -660,6 +673,12 @@ def test_rebuild_sub_sub_build(self): }) build1_1_1.orphan_result = True + # simulate scheduler ran + build1_1._update_global_state() + build1._update_global_state() + build1_1._update_global_result() + build1._update_global_result() + self.assertEqual('ok', build1.global_result) self.assertEqual('ok', build1_1.global_result) self.assertEqual('ko', build1_1_1.global_result) @@ -671,6 +690,13 @@ def test_rebuild_sub_sub_build(self): rebuild1_1_1.local_result = 'ok' rebuild1_1_1.local_state = 'done' + + # simulate scheduler ran + build1_1._update_global_state() + build1._update_global_state() + build1_1._update_global_result() + build1._update_global_result() + self.assertEqual('ok', build1.global_result) self.assertEqual('ok', build1_1.global_result) self.assertEqual('ko', build1_1_1.global_result) @@ -812,11 +838,13 @@ def github_status(build): with patch('odoo.addons.runbot.models.build.BuildResult._github_status', github_status): self.callcount = 0 self.build.local_state = 'testing' + self.assertEqual(self.build.global_state, 'testing') self.assertEqual(self.callcount, 0, "_github_status shouldn't have been called") self.callcount = 0 self.build.local_state = 'running' + self.assertEqual(self.build.global_state, 'running') self.assertEqual(self.callcount, 1, "_github_status should have been called") diff --git a/runbot/tests/test_build_config_step.py b/runbot/tests/test_build_config_step.py index 2e52171a3..a5889e061 100644 --- a/runbot/tests/test_build_config_step.py +++ b/runbot/tests/test_build_config_step.py @@ -315,6 +315,8 @@ def test_config_step_create_results(self): child_build.local_result = 'ko' self.assertEqual(child_build.global_result, 'ko') + # simulate sheduler ran + self.parent_build._update_global_result() self.assertEqual(self.parent_build.global_result, 'ko') diff --git a/runbot/tests/test_upgrade.py b/runbot/tests/test_upgrade.py index dbd7bb48d..22a4d256a 100644 --- a/runbot/tests/test_upgrade.py +++ b/runbot/tests/test_upgrade.py @@ -576,6 +576,10 @@ def docker_run_upgrade(cmd, *args, ro_volumes=False, **kwargs): self.assertEqual(current_build.global_state, 'done') # self.assertEqual(current_build.global_result, 'ok') + # simulate scheduler ran + from_version_builds._update_global_state() + to_version_builds._update_global_state() + self.assertEqual(from_version_builds.mapped('global_state'), ['done'] * 10) self.assertEqual(to_version_builds.mapped('global_state'), ['done'] * 5)