pengyuthon/odoo_source
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[IMP] website, test_website: strengthen the perf testing suite

Browse Source 
This commit ensure the expected tables are accessed when a page (not
only) is requested.

This is needed because without that, we can only check the query count
number which might be broken without being noticed:
- Commit 1 reduce queries by 2 (no need to access table X anymore)
- Commit 2 later reduce queries by 2 (less access on a table) but
  without noticing it, it also now introduce back the access of table X.

At the end the query count is still fine, but the code is not: it broke
a previous improvement while it was not necessary.

Worst, it could even lower the query count despite still breaking a
previous improvement (-3 queries +2 queries back).

Also, since the query count of a page is not exactly the same inside a
test and in real use cases, an `EXTRA_REQUEST` param is used in those
tests to abstract that change and still use the "real use case" query
count in the test to fit the reality, be human readable and easier to
write/debug.
Indeed, in test mode there is more queries due to the test cursor
rollback and savepoint, but there is one less query (the cache one).

Some commit wrongly reduced that `EXTRA_REQUEST` param making it looks
like there was an improvement while there wasn't.

This commit will help preventing all of that, ensuring the correct
tables are accessed and only those, on top of checking the query count
number (we still need to check the query count in case there is a query
not catched in the SQL table logs).

Some of the PRs/commits where it happened:
- https://github.com/odoo/odoo/commit/aa1c1b0bcb5327894c7b00d0454eebae06eaa005
- https://github.com/odoo/odoo/pull/112000/files#r1160851862

closes odoo/odoo#124939

X-original-commit: 26e1fc3abd54a1baa16cf1c99d75d501a8dc6906
Signed-off-by: Romain Derie (rde) <rde@odoo.com>
This commit is contained in:
Romain Derie
2023-06-15 18:51:18 +02:00
parent 202f79dec0
commit 37cdff405e
2 changed files with 165 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files
addons/test_website/tests/test_performance.py
+4 -1
View File
@@ -7,5 +7,8 @@ from odoo.addons.website.tests.test_performance import UtilPerf
class TestPerformance(UtilPerf):
def test_10_perf_sql_website_controller_minimalist(self):
url = '/empty_controller_test'
self.assertEqual(self._get_url_hot_query(url), 1)
select_tables_perf = {
'base_registry_signaling': 1,
}
self._check_url_hot_query(url, 1, select_tables_perf)
self.assertEqual(self._get_url_hot_query(url, cache=False), 1)
addons/website/tests/test_performance.py
+161 -18
View File
@@ -1,6 +1,10 @@
# -*- coding: utf-8 -*-
# Part of Odoo. See LICENSE file for full copyright and licensing details.
import copy
from contextlib import nullcontext
from odoo.tools import mute_logger
from odoo.tests.common import HttpCase, tagged
@@ -17,7 +21,28 @@ be added:
class UtilPerf(HttpCase):
def _get_url_hot_query(self, url, cache=True):
def _get_url_hot_query(self, url, cache=True, table_count=False):
""" This method returns the number of SQL Queries used inside a request.
The returned query number will be the same as a "real" (outside of test
mode) case: the method takes care of removing the extra queries related
to the testing mode and to add the missing one from "real" use case.
The goal is to ease the code reading and debugging of those perf testing
methods, as one will have the same query count written in the test than
it shows in "real" case logs.
eg: if a page is taking X SQL query count to be loaded outside test mode
in "real" case, the test is expected to also use X as query count
value to be asserted/checked.
:param str url: url to be checked
:param bool cache: whether the QWeb `t-cache` should be disabled or not
:param bool table_count: whether the method should also return data
about the queried table
:return: the query count plus the queried table data if ``table_count``
is ``True``
:rtype: int|tuple(int, dict)
"""
url += ('?' not in url and '?' or '')
if cache:
url += '&debug='
@@ -27,10 +52,37 @@ class UtilPerf(HttpCase):
# ensure worker is in hot state
self.url_open(url)
self.url_open(url)
sql_count = self.registry.test_cr.sql_log_count
self.url_open(url)
return self.registry.test_cr.sql_log_count - sql_count - EXTRA_REQUEST
sql_count_before = self.cr.sql_log_count
with (self.cr._enable_table_tracking() if table_count else nullcontext()):
if table_count:
sql_from_log_before = copy.deepcopy(self.cr.sql_from_log)
sql_into_log_before = copy.deepcopy(self.cr.sql_into_log)
self.url_open(url)
sql_count = self.cr.sql_log_count - sql_count_before - EXTRA_REQUEST
if table_count:
sql_from_tables = {'base_registry_signaling': 1} # see EXTRA_REQUEST
sql_into_tables = {}
for table, stats in self.cr.sql_from_log.items():
query_count = stats[0] - sql_from_log_before.get(table, [0])[0]
if query_count:
sql_from_tables[table] = query_count
for table, stats in self.cr.sql_into_log.items():
query_count = stats[0] - sql_into_log_before.get(table, [0])[0]
if query_count:
sql_into_tables[table] = query_count
return sql_count, sql_from_tables, sql_into_tables
return sql_count
def _check_url_hot_query(self, url, expected_query_count, select_tables_perf=None, insert_tables_perf=None):
query_count, select_tables, insert_tables = self._get_url_hot_query(url, table_count=True)
self.assertEqual(query_count, expected_query_count)
self.assertEqual(select_tables, select_tables_perf or {})
self.assertEqual(insert_tables, insert_tables_perf or {})
class TestStandardPerformance(UtilPerf):
@@ -48,16 +100,34 @@ class TestStandardPerformance(UtilPerf):
self.authenticate('demo', 'demo')
self.env['res.users'].sudo().browse(2).website_published = True
url = '/web/image/res.users/2/image_256'
self.assertEqual(self._get_url_hot_query(url), 6)
select_tables_perf = {
'base_registry_signaling': 1,
'res_users': 2,
'res_partner': 1,
'ir_attachment': 2,
}
self._check_url_hot_query(url, 6, select_tables_perf)
self.assertEqual(self._get_url_hot_query(url, cache=False), 6)
@mute_logger('odoo.http')
def test_20_perf_sql_img_controller_bis(self):
url = '/web/image/website/1/favicon'
self.assertEqual(self._get_url_hot_query(url), 5)
select_tables_perf = {
'base_registry_signaling': 1,
'website': 2,
# 1. `_find_record()` performs an access right check through
# `exists()` which perform a request on the website.
# 2. `_get_stream_from` ends up reading the requested record to
# give a name to the file (downloaded_name)
'ir_attachment': 2,
# 1. `_record_to_stream()` does a `search()`..
# 2. ..followed by a `_read()`
}
self._check_url_hot_query(url, 5, select_tables_perf)
self.assertEqual(self._get_url_hot_query(url, cache=False), 5)
self.authenticate('portal', 'portal')
self.assertEqual(self._get_url_hot_query(url), 5)
self._check_url_hot_query(url, 5, select_tables_perf)
self.assertEqual(self._get_url_hot_query(url, cache=False), 5)
@@ -95,30 +165,82 @@ class TestWebsitePerformance(UtilPerf):
def test_10_perf_sql_queries_page(self):
# standard untracked website.page
self.assertEqual(self._get_url_hot_query(self.page.url), 6)
select_tables_perf = {
'base_registry_signaling': 1,
'ir_attachment': 1,
# `_get_serve_attachment` dispatcher fallback
'website_page': 2,
# 1. `_serve_page` search page matching URL..
# 2. ..then reads it (`is_visible`)
'website': 1,
'ir_ui_view': 1,
# Check if `view.track` to track visitor or not
}
self._check_url_hot_query(self.page.url, 6, select_tables_perf)
self.assertEqual(self._get_url_hot_query(self.page.url, cache=False), 10)
self.menu.unlink()
self.assertEqual(self._get_url_hot_query(self.page.url), 6)
self.menu.unlink() # page being or not in menu shouldn't add queries
self._check_url_hot_query(self.page.url, 6, select_tables_perf)
self.assertEqual(self._get_url_hot_query(self.page.url, cache=False), 10)
def test_15_perf_sql_queries_page(self):
# standard tracked website.page
select_tables_perf = {
'base_registry_signaling': 1,
'ir_attachment': 1,
# `_get_serve_attachment` dispatcher fallback
'website_page': 2,
# 1. `_serve_page` search page matching URL..
# 2. ..then reads it (`is_visible`)
'website': 1,
'ir_ui_view': 1,
# Check if `view.track` to track visitor or not
}
insert_tables_perf = {
'website_visitor': 1,
# Visitor upsert
}
self.page.track = True
self.assertEqual(self._get_url_hot_query(self.page.url), 7)
self._check_url_hot_query(self.page.url, 7, select_tables_perf, insert_tables_perf)
self.assertEqual(self._get_url_hot_query(self.page.url, cache=False), 11)
self.menu.unlink()
self.assertEqual(self._get_url_hot_query(self.page.url), 7)
self.menu.unlink() # page being or not in menu shouldn't add queries
self._check_url_hot_query(self.page.url, 7, select_tables_perf, insert_tables_perf)
self.assertEqual(self._get_url_hot_query(self.page.url, cache=False), 11)
def test_20_perf_sql_queries_homepage(self):
# homepage "/" has its own controller
self.assertEqual(self._get_url_hot_query('/'), 7)
select_tables_perf = {
'base_registry_signaling': 1,
'website_menu': 1,
# homepage controller is prefetching all menus for perf in one go
'website_page': 2,
# 1. the menu prefetching is also prefetching all menu's pages
# 2. find page matching the `/` url
'website': 1,
'ir_ui_view': 1,
# Check if `view.track` to track visitor or not
}
insert_tables_perf = {
'website_visitor': 1,
# Visitor upsert
}
self._check_url_hot_query('/', 7, select_tables_perf, insert_tables_perf)
self.assertEqual(self._get_url_hot_query('/', cache=False), 9)
def test_30_perf_sql_queries_page_no_layout(self):
# website.page with no call to layout templates
# untrack website.page with no call to layout templates
self.page.arch = '<div>I am a blank page</div>'
self.assertEqual(self._get_url_hot_query(self.page.url), 5)
select_tables_perf = {
'base_registry_signaling': 1,
'ir_attachment': 1,
# `_get_serve_attachment` dispatcher fallback
'website_page': 2,
# 1. `_serve_page` search page matching URL..
# 2. ..then reads it (`is_visible`)
'ir_ui_view': 1,
# Check if `view.track` to track visitor or not
}
self._check_url_hot_query(self.page.url, 5, select_tables_perf)
self.assertEqual(self._get_url_hot_query(self.page.url, cache=False), 5)
def test_40_perf_sql_queries_page_multi_level_menu(self):
@@ -137,7 +259,18 @@ class TestWebsitePerformance(UtilPerf):
menu_bb.parent_id = menu_b
menu_aa.parent_id = menu_a
self.assertEqual(self._get_url_hot_query(self.page.url), 6)
select_tables_perf = {
'base_registry_signaling': 1,
'ir_attachment': 1,
# `_get_serve_attachment` dispatcher fallback
'website_page': 2,
# 1. `_serve_page` search page matching URL..
# 2. ..then reads it (`is_visible`)
'website': 1,
'ir_ui_view': 1,
# Check if `view.track` to track visitor or not
}
self._check_url_hot_query(self.page.url, 6, select_tables_perf)
self.assertEqual(self._get_url_hot_query(self.page.url, cache=False), 10)
@@ -148,5 +281,15 @@ class TestWebsitePerformancePost(UtilPerf):
# assets route /web/assets/..
self.env['ir.qweb']._generate_asset_links('web.assets_frontend_lazy', css=False, js=True)
assets_url = self.env['ir.attachment'].search([('url', '=like', '/web/assets/%/web.assets_frontend_lazy%.js')], limit=1).url
self.assertEqual(self._get_url_hot_query(assets_url), 4)
select_tables_perf = {
'base_registry_signaling': 1,
'ir_attachment': 3,
# All 3 coming from the /web/assets and ir.binary stack
# 1. `_find_record()` performs an access right check through
# `exists()` which perform a request on the ir.attachment.
# 2. `validate_access` reads `public` field of ir.attachment with
# prefetch=False (so only that field)
# 3. `_record_to_stream` reads the other attachment fields
}
self._check_url_hot_query(assets_url, 4, select_tables_perf)
self.assertEqual(self._get_url_hot_query(assets_url, cache=False), 4)