Merge lp:~therp-nl/openobject-server/[e-mail]_6.1_lp1009014_new-6.1 into lp:openobject-server/6.1

[Ronald Portier (Therp)]

Fix vacuum cleaning on transient object.

This is a new proposal taking into account the remarks made by Vo Minh Thu.

Unmerged revisions

4290. By Ronald Portier (Therp) on 2012-10-24

[FIX] Fix vacuum cleaning both for time and count based cleaning,
      adding a safeguard against cleaning recently used rows.

=== modified file 'openerp/osv/orm.py'
--- openerp/osv/orm.py	2012-09-24 14:00:07 +0000
+++ openerp/osv/orm.py	2012-10-24 14:59:30 +0000
@@ -674,9 +674,6 @@
 
     # Transience
     _transient = False # True in a TransientModel
-    _transient_max_count = None
-    _transient_max_hours = None
-    _transient_check_time = 20
 
     # structure:
     #  { 'parent_model': 'm2o_field', ... }
@@ -4889,20 +4886,26 @@
 
     def _transient_clean_rows_older_than(self, cr, seconds):
         assert self._transient, "Model %s is not transient, it cannot be vacuumed!" % self._name
-        cr.execute("SELECT id FROM " + self._table + " WHERE"
-            " COALESCE(write_date, create_date, (now() at time zone 'UTC'))::timestamp <"
-            " ((now() at time zone 'UTC') - interval %s)", ("%s seconds" % seconds,))
-        ids = [x[0] for x in cr.fetchall()]
-        self.unlink(cr, SUPERUSER_ID, ids)
-
-    def _transient_clean_old_rows(self, cr, count):
-        assert self._transient, "Model %s is not transient, it cannot be vacuumed!" % self._name
+        '''Never delete rows used in last 5 minutes'''
+        seconds = max(seconds, 300)
+        now_str = "(now() at time zone 'UTC')"
         cr.execute(
-            "SELECT id, COALESCE(write_date, create_date, (now() at time zone 'UTC'))::timestamp"
-            " AS t FROM " + self._table +
-            " ORDER BY t LIMIT %s", (count,))
+        "SELECT id FROM " + self._table + " WHERE"
+        " COALESCE(write_date, create_date, " + now_str + ")::timestamp"
+        "< (" + now_str + " - interval %s)", ("%s seconds" % seconds,))
         ids = [x[0] for x in cr.fetchall()]
-        self.unlink(cr, SUPERUSER_ID, ids)
+        if  ids:
+            self.unlink(cr, SUPERUSER_ID, ids)
+
+    def _transient_clean_old_rows(self, cr, max_count):
+        # Check how many rows we have in the table
+        sql_statement = "SELECT count(*) as row_count FROM %s" % (self._table, )
+        cr.execute(sql_statement)
+        res = cr.fetchall()
+        row_count = res[0][0]
+        if  row_count <= max_count:
+            return  # max not reached, nothing to do
+        self._transient_clean_rows_older_than(cr, 300)
 
     def _transient_vacuum(self, cr, uid, force=False):
         """Clean the transient records.
@@ -4912,12 +4915,22 @@
         Actual cleaning will happen only once every "_transient_check_time" calls.
         This means this method can be called frequently called (e.g. whenever
         a new record is created).
+        Example with both max_hours and max_count active:
+        Suppose max_hours = 0.2 (e.g. 12 minutes), max_count = 20, there are 55 rows in the
+        table, 10 created/changed in the last 5 minutes, an additional 12 created/changed between
+        5 and 10 minutes ago, the rest created/changed more then 12 minutes ago.
+        - age based vacuum will leave the 22 rows created/changed in the last 12 minutes
+        - count based vacuum will wipe out another 12 rows. Not just 2, otherwise each addition
+          would immediately cause the maximum to be reached again.
+        - the 10 rows that have been created/changed the last 5 minutes will NOT be deleted
         """
         assert self._transient, "Model %s is not transient, it cannot be vacuumed!" % self._name
+        _transient_check_time = 20 # arbitrary limit on vacuum executions
         self._transient_check_count += 1
-        if (not force) and (self._transient_check_count % self._transient_check_time):
-            self._transient_check_count = 0
-            return True
+        if ((not force)
+        and (self._transient_check_count < _transient_check_time)):
+            return True  # no vacuum cleaning this time
+        self._transient_check_count = 0
 
         # Age-based expiration
         if self._transient_max_hours: