From eb708e5cadb879f834961462dc7e796b56583875 Mon Sep 17 00:00:00 2001 From: Xavier-Do Date: Thu, 20 Jul 2023 09:27:30 +0000 Subject: [PATCH] [IMP] base: avoid cache_invalidation A clear_cache was added in _update_xmlids in pr 119813. If it was needed in case of update, it is not useful when creating an xmlid or when the value is unchanged. The initial idea was to detect if an update or an insert was done, maybe using create_date and write_date. Unfortunately the create_date is the same as the write_date in the same transaction. This is unlikely but in this case, we could update the cache in place. The final behavior is to update the cache in all case, and notify other workers only if a model was updated. This will help to avoid invalidating the cache too mush during module loading. Even if the impact on time is small, the increase in queries was visible. This solution would even be a slight improvement on previous query count closes odoo/odoo#129029 Related: odoo/enterprise#44349 Signed-off-by: Julien Castiaux (juc) --- odoo/addons/base/models/ir_model.py | 40 ++++++++++++++++---- odoo/addons/base/tests/test_ir_model.py | 50 +++++++++++++++++++++++++ 2 files changed, 83 insertions(+), 7 deletions(-) diff --git a/odoo/addons/base/models/ir_model.py b/odoo/addons/base/models/ir_model.py index 24160a2eff6..bc9a993310e 100644 --- a/odoo/addons/base/models/ir_model.py +++ b/odoo/addons/base/models/ir_model.py @@ -2113,7 +2113,19 @@ class IrModelData(models.Model): query = self._build_update_xmlids_query(sub_rows, update) try: self.env.cr.execute(query, [arg for row in sub_rows for arg in row]) - self.env.registry.clear_cache() + result = self.env.cr.fetchall() + if result: + for module, name, model, res_id, create_date, write_date in result: + # small optimisation: during install a lot of xmlid are created/updated. + # Instead of clearing the cache, set the correct value in the cache to avoid a bunch of query + self._xmlid_lookup.cache.add_value(self, f"{module}.{name}", cache_value=(model, res_id)) + if create_date != write_date: + # something was updated, notify other workers + # it is possible that create_date and write_date + # have the same value after an update if it was + # created in the same transaction, no need to invalidate other worker cache + # cache in this case. + self.env.registry.cache_invalidated.add('default') except Exception: _logger.error("Failed to insert ir_model_data\n%s", "\n".join(str(row) for row in sub_rows)) @@ -2124,18 +2136,32 @@ class IrModelData(models.Model): # NOTE: this method is overriden in web_studio; if you need to make another # override, make sure it is compatible with the one that is there. + def _build_insert_xmlids_values(self): + return { + 'module': '%s', + 'name': '%s', + 'model': '%s', + 'res_id': '%s', + 'noupdate': '%s', + } + def _build_update_xmlids_query(self, sub_rows, update): - rowf = "(%s, %s, %s, %s, %s)" + rows = self._build_insert_xmlids_values() + row_names = f"({','.join(rows.keys())})" + row_placeholders = f"({','.join(rows.values())})" + row_placeholders = ", ".join([row_placeholders] * len(sub_rows)) return """ - INSERT INTO ir_model_data (module, name, model, res_id, noupdate) - VALUES {rows} + INSERT INTO ir_model_data {row_names} + VALUES {row_placeholder} ON CONFLICT (module, name) DO UPDATE SET (model, res_id, write_date) = (EXCLUDED.model, EXCLUDED.res_id, now() at time zone 'UTC') - {where} + WHERE (ir_model_data.res_id != EXCLUDED.res_id OR ir_model_data.model != EXCLUDED.model) {and_where} + RETURNING module, name, model, res_id, create_date, write_date """.format( - rows=", ".join([rowf] * len(sub_rows)), - where="WHERE NOT ir_model_data.noupdate" if update else "", + row_names=row_names, + row_placeholder=row_placeholders, + and_where="AND NOT ir_model_data.noupdate" if update else "", ) @api.model diff --git a/odoo/addons/base/tests/test_ir_model.py b/odoo/addons/base/tests/test_ir_model.py index 5cae7d99c9a..a613ce69fee 100644 --- a/odoo/addons/base/tests/test_ir_model.py +++ b/odoo/addons/base/tests/test_ir_model.py @@ -172,6 +172,56 @@ class TestXMLID(TransactionCase): with self.assertRaisesRegex(IntegrityError, 'ir_model_data_name_nospaces'): model._load_records(data_list) + def test_update_xmlid(self): + def assert_xmlid(xmlid, value, message): + expected_values = (value._name, value.id) + with self.assertQueryCount(0): + self.assertEqual(self.env['ir.model.data']._xmlid_lookup(xmlid), expected_values, message) + module, name = xmlid.split('.') + self.env.cr.execute("SELECT model, res_id FROM ir_model_data where module=%s and name=%s", [module, name]) + self.assertEqual((value._name, value.id), self.env.cr.fetchone(), message) + + xmlid = 'base.test_xmlid' + records = self.env['ir.model.data'].search([], limit=6) + with self.assertQueryCount(1): + self.env['ir.model.data']._update_xmlids([ + {'xml_id': xmlid, 'record': records[0]}, + ]) + assert_xmlid(xmlid, records[0], f'The xmlid {xmlid} should have been created with record {records[0]}') + + with self.assertQueryCount(1): + self.env['ir.model.data']._update_xmlids([ + {'xml_id': xmlid, 'record': records[1]}, + ], update=True) + assert_xmlid(xmlid, records[1], f'The xmlid {xmlid} should have been updated with record {records[1]}') + + with self.assertQueryCount(1): + self.env['ir.model.data']._update_xmlids([ + {'xml_id': xmlid, 'record': records[2]}, + ]) + assert_xmlid(xmlid, records[2], f'The xmlid {xmlid} should have been updated with record {records[1]}') + + # noupdate case + # note: this part is mainly there to avoid breaking the current behaviour, not asserting that it makes sence + xmlid = 'base.test_xmlid_noupdates' + with self.assertQueryCount(1): + self.env['ir.model.data']._update_xmlids([ + {'xml_id': xmlid, 'record': records[3], 'noupdate':True}, # record created as noupdate + ]) + + assert_xmlid(xmlid, records[3], f'The xmlid {xmlid} should have been created for record {records[2]}') + + with self.assertQueryCount(1): + self.env['ir.model.data']._update_xmlids([ + {'xml_id': xmlid, 'record': records[4]}, + ], update=True) + assert_xmlid(xmlid, records[3], f'The xmlid {xmlid} should not have been updated (update mode)') + + with self.assertQueryCount(1): + self.env['ir.model.data']._update_xmlids([ + {'xml_id': xmlid, 'record': records[5]}, + ]) + assert_xmlid(xmlid, records[5], f'The xmlid {xmlid} should have been updated with record (not an update) {records[1]}') class TestIrModel(TransactionCase):