[FIX] base: improve session existence check mechanism

The commit: odoo@6fb676a
adds the model `res.device.log` which will hold a lot of data.

In order to use this data efficiently, we decide to process data
with the boolean field `revoked` equal to `True` (via the indexes).

This boolean field indicates whether the session that generated the log
is still present on the disk.

The commit: odoo@e4c9d17
adds an automatic verification mechanism to change this value if necessary.

Between the time the model was created and the time the verification
mechanism was implemented, the table may have become too large.
This will result in a very long write within a transaction.

The purpose of this commit is to correct the  method for performing the batch writing.

closes odoo#232126

X-original-commit: e461c78
Signed-off-by: Denis Ledoux (dle) <[email protected]>
Signed-off-by: Thomas Lefebvre (thle) <[email protected]>

Author: thle-odoo

odoo/addons/base/models/res_device.py

@@ -1,11 +1,11 @@
 # Part of Odoo. See LICENSE file for full copyright and licensing details.
 
 from contextlib import nullcontext
-from datetime import datetime
+from datetime import datetime, timedelta
 import logging
 
 from odoo import api, fields, models, tools
-from odoo.http import GeoIP, request, root, STORED_SESSION_BYTES
+from odoo.http import GeoIP, get_session_max_inactivity, request, root, STORED_SESSION_BYTES
 from odoo.tools import SQL, OrderedSet, unique
 from odoo.tools.translate import _
 from .res_users import check_identity
@@ -137,24 +137,34 @@ def __update_revoked(self):
             Set the field ``revoked`` to ``True`` for ``res.device.log``
             for which the session file no longer exists on the filesystem.
         """
-        device_logs_by_session_identifier = {}
-        for session_identifier, device_logs in self.env['res.device.log']._read_group(
-            domain=[('revoked', '=', False)],
-            groupby=['session_identifier'],
-            aggregates=['id:recordset'],
-        ):
-            device_logs_by_session_identifier[session_identifier] = device_logs
-
-        revoked_session_identifiers = root.session_store.get_missing_session_identifiers(
-            device_logs_by_session_identifier.keys()
-        )
-        device_logs_to_revoke = self.env['res.device.log'].concat(*map(
-            device_logs_by_session_identifier.get,
-            revoked_session_identifiers
-        ))
-        # Initial run may take 5-10 minutes due to many non-revoked sessions,
-        # marking them enables index use on ``revoked IS NOT TRUE``.
-        device_logs_to_revoke.sudo().write({'revoked': True})
+        batch_size = 100_000
+        offset = 0
+
+        while True:
+            candidate_device_log_ids = self.env['res.device.log'].search_fetch(
+                [
+                    ('revoked', '=', False),
+                    ('last_activity', '<', datetime.now() - timedelta(seconds=get_session_max_inactivity(self.env))),
+                ],
+                ['session_identifier'],
+                order='id',
+                limit=batch_size,
+                offset=offset,
+            )
+            if not candidate_device_log_ids:
+                break
+            offset += batch_size
+            revoked_session_identifiers = root.session_store.get_missing_session_identifiers(
+                set(candidate_device_log_ids.mapped('session_identifier'))
+            )
+            if not revoked_session_identifiers:
+                continue
+            to_revoke = candidate_device_log_ids.filtered(
+                lambda candidate: candidate.session_identifier in revoked_session_identifiers
+            )
+            to_revoke.write({'revoked': True})
+            self.env.cr.commit()
+            offset -= len(to_revoke)
 
 
 class ResDevice(models.Model):

odoo/addons/test_http/tests/test_device.py

@@ -1,5 +1,6 @@
 # Part of Odoo. See LICENSE file for full copyright and licensing details.
 
+from datetime import datetime
 from freezegun import freeze_time
 from unittest.mock import patch
 
@@ -354,56 +355,37 @@ def _create_device_log_for_user(self, session, count):
                 'session_identifier': odoo.http.root.session_store.generate_key(),
                 'user_id': session.uid,
                 'revoked': False,
+                'first_activity': datetime.now(),
+                'last_activity': datetime.now(),
             })
 
-    def test_filesystem_reflexion_user(self):
+    def test_filesystem_reflexion(self):
         session = self.authenticate(self.user_admin.login, self.user_admin.login)
-        self._create_device_log_for_user(session, 10)
-        session = self.authenticate(self.user_internal.login, self.user_internal.login)
-        self._create_device_log_for_user(session, 10)
-
-        devices, logs = self.get_devices_logs(self.user_internal)
-        self.assertEqual(len(devices), 10)
-        self.assertEqual(len(logs), 10)
-        self.assertEqual(len(self.user_internal.device_ids), 10)
-
-        self.DeviceLog.with_user(self.user_internal)._ResDeviceLog__update_revoked()
-        self.DeviceLog.flush_model()  # Because write on ``res.device.log`` and so we have new values in cache
-        self.Device.invalidate_model()  # Because it depends on the ``res.device.log`` model (updated in database)
-
-        devices, _ = self.get_devices_logs(self.user_internal)
-        self.assertEqual(len(devices), 0)  # No file exist on the filesystem (``revoked`` equals to ``False``)
-        self.assertEqual(len(self.user_internal.device_ids), 0)
-
-        # Admin device logs are not updated
-        devices, _ = self.get_devices_logs(self.user_admin)
-        self.assertEqual(len(devices), 10)
-        self.assertEqual(len(self.user_admin.device_ids), 10)
-
-    def test_filesystem_reflexion_admin(self):
-        session = self.authenticate(self.user_admin.login, self.user_admin.login)
-        self._create_device_log_for_user(session, 10)
+        with freeze_time('2025-01-01 08:00:00'):
+            self._create_device_log_for_user(session, 10)
 
         devices, logs = self.get_devices_logs(self.user_admin)
         self.assertEqual(len(devices), 10)
         self.assertEqual(len(logs), 10)
         self.assertEqual(len(self.user_admin.device_ids), 10)
 
         session = self.authenticate(self.user_internal.login, self.user_internal.login)
-        self._create_device_log_for_user(session, 10)
+        with freeze_time('2025-01-01 08:00:00'):
+            self._create_device_log_for_user(session, 10)
 
         devices, logs = self.get_devices_logs(self.user_internal)
         self.assertEqual(len(devices), 10)
         self.assertEqual(len(logs), 10)
         self.assertEqual(len(self.user_internal.device_ids), 10)
 
-        # Admin can update all device logs
-        self.DeviceLog.with_user(self.user_admin)._ResDeviceLog__update_revoked()
-        self.DeviceLog.flush_model()
-        self.Device.invalidate_model()
+        # Update all device logs
+        with freeze_time('2025-02-01 08:00:00'), patch.object(self.cr, 'commit', lambda: ...):
+            self.DeviceLog.sudo()._ResDeviceLog__update_revoked()
+        self.DeviceLog.flush_model()  # Because write on ``res.device.log`` and so we have new values in cache
+        self.Device.invalidate_model()  # Because it depends on the ``res.device.log`` model (updated in database)
 
         devices, _ = self.get_devices_logs(self.user_admin)
-        self.assertEqual(len(devices), 0)
+        self.assertEqual(len(devices), 0)  # No file exist on the filesystem (``revoked`` equals to ``True``)
         self.assertEqual(len(self.user_admin.device_ids), 0)
 
         devices, _ = self.get_devices_logs(self.user_internal)

odoo/http.py

@@ -1306,21 +1306,6 @@ def get_missing_session_identifiers(self, identifiers):
         :type identifiers: iterable
         :return: the identifiers which are not present on the filesystem
         :rtype: set
-
-        Note 1:
-        Working with identifiers 42 characters long means that
-        we don't have to work with the entire sid session,
-        while maintaining sufficient entropy to avoid collisions.
-        See details in ``generate_key``.
-
-        Note 2:
-        Scans the session store for inactive (GC'd) sessions.
-        Works even if GC is done externally (not via ``vacuum()``).
-        Performance is acceptable for an infrequent background job:
-            - listing ``directories``: 1-5s on SSD
-            - iterating sessions:
-                - 25k on standard SSD: ~1.5 min
-                - 2M on RAID10 SSD: ~25s
         """
         # There are a lot of session files.
         # Use the param ``identifiers`` to select the necessary directories.