From 3752ac683220909fdd2af5bdbccb6d944252133b Mon Sep 17 00:00:00 2001 From: tsm-odoo Date: Thu, 12 Jan 2023 09:48:54 +0000 Subject: [PATCH] [FIX] bus: fix websocket timeout burst Before this commit, the keep alive timeout could occur for many websockets at the same time resulting in a burst of transaction (cursor is open when connecting, disconnecting a websocket). This issue was even worse because the cursor was opened even when it was not needed: we don't need to open a cursor if there is no callback registered for the lifecycle event (OPEN/CLOSE). This commit fixes those issues by: - adding a random delay to the keep alive timeout of every websocket. - not triggering lifecycle events if no callbacks are registered. closes odoo/odoo#110127 X-original-commit: b4fb1359021b6c882ece3037c82aaaf65cfc2e1e Signed-off-by: Julien Castiaux --- addons/bus/tests/test_websocket_caryall.py | 10 ++++++++-- addons/bus/websocket.py | 15 +++++++++++---- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/addons/bus/tests/test_websocket_caryall.py b/addons/bus/tests/test_websocket_caryall.py index 07bdc5740a2..23d0992e1a7 100644 --- a/addons/bus/tests/test_websocket_caryall.py +++ b/addons/bus/tests/test_websocket_caryall.py @@ -88,9 +88,9 @@ class TestWebsocketCaryall(WebsocketCase): def test_timeout_manager_keep_alive_timeout(self): with freeze_time('2022-08-19') as frozen_time: timeout_manager = TimeoutManager() - frozen_time.tick(delta=timedelta(seconds=TimeoutManager.KEEP_ALIVE_TIMEOUT / 2)) + frozen_time.tick(delta=timedelta(seconds=timeout_manager._keep_alive_timeout / 2)) self.assertFalse(timeout_manager.has_timed_out()) - frozen_time.tick(delta=timedelta(seconds=TimeoutManager.KEEP_ALIVE_TIMEOUT / 2)) + frozen_time.tick(delta=timedelta(seconds=timeout_manager._keep_alive_timeout / 2 + 1)) self.assertTrue(timeout_manager.has_timed_out()) self.assertEqual(timeout_manager.timeout_reason, TimeoutReason.KEEP_ALIVE) @@ -284,3 +284,9 @@ class TestWebsocketCaryall(WebsocketCase): 'data': {'channels': ['my_channel'], 'last': client_last_notification_id} })) subscribe_done_event.wait() + + def test_no_cursor_when_no_callback_for_lifecycle_event(self): + with patch.object(Websocket, '_event_callbacks', defaultdict(set)): + with patch('odoo.addons.bus.websocket.acquire_cursor') as mock: + self.websocket_connect() + self.assertFalse(mock.called) diff --git a/addons/bus/websocket.py b/addons/bus/websocket.py index aecd42d50a9..0878f599956 100644 --- a/addons/bus/websocket.py +++ b/addons/bus/websocket.py @@ -602,6 +602,8 @@ class Websocket: registered for this event type. Every callback is given both the environment and the related websocket. """ + if not type(self)._event_callbacks[event_type]: + return with closing(acquire_cursor(self._session.db)) as cr: env = api.Environment(cr, self._session.uid, self._session.context) for callback in type(self)._event_callbacks[event_type]: @@ -646,8 +648,8 @@ class TimeoutManager: """ This class handles the Websocket timeouts. If no response to a PING/CLOSE frame is received after `TIMEOUT` seconds or if the - connection is opened for more than `KEEP_ALIVE_TIMEOUT` seconds, the - connection is considered to have timed out. To determine if the + connection is opened for more than `self._keep_alive_timeout` seconds, + the connection is considered to have timed out. To determine if the connection has timed out, use the `has_timed_out` method. """ TIMEOUT = 15 @@ -660,6 +662,11 @@ class TimeoutManager: self._awaited_opcode = None # Time in which the connection was opened. self._opened_at = time.time() + # Custom keep alive timeout for each TimeoutManager to avoid multiple + # connections timing out at the same time. + self._keep_alive_timeout = ( + type(self).KEEP_ALIVE_TIMEOUT + random.uniform(0, type(self).KEEP_ALIVE_TIMEOUT / 2) + ) self.timeout_reason = None # Start time recorded when we started awaiting an answer to a # PING/CLOSE frame. @@ -689,10 +696,10 @@ class TimeoutManager: Determine whether the connection has timed out or not. The connection times out when the answer to a CLOSE/PING frame is not received within `TIMEOUT` seconds or if the connection - is opened for more than `KEEP_ALIVE_TIMEOUT` seconds. + is opened for more than `self._keep_alive_timeout` seconds. """ now = time.time() - if now - self._opened_at >= type(self).KEEP_ALIVE_TIMEOUT: + if now - self._opened_at >= self._keep_alive_timeout: self.timeout_reason = TimeoutReason.KEEP_ALIVE return True if self._awaited_opcode and now - self._waiting_start_time >= type(self).TIMEOUT: