[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) <juc@odoo.com>
This commit is contained in:
Xavier-Do
2023-07-20 14:23:15 +02:00
parent d5ed47a11f
commit eb708e5cad
2 changed files with 83 additions and 7 deletions
+33 -7
View File
@@ -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
+50
View File
@@ -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):