From d41a234599e6d214761d6344e70a37051bff9d87 Mon Sep 17 00:00:00 2001 From: Ronald Portier Date: Wed, 24 Oct 2012 09:28:07 +0200 Subject: [PATCH 1/3] [FIX] Fix vacuum cleaning both for time and count based cleaning, adding a safeguard against cleaning recently used rows. bzr revid: ronald@therp.nl-20121024072807-vsvhub1m0y3j5ude --- openerp/osv/orm.py | 41 +++++++++++++++++++++++------------------ 1 file changed, 23 insertions(+), 18 deletions(-) diff --git a/openerp/osv/orm.py b/openerp/osv/orm.py index 4c3330c97b7..6916f94730a 100644 --- a/openerp/osv/orm.py +++ b/openerp/osv/orm.py @@ -684,9 +684,6 @@ class BaseModel(object): # Transience _transient = False # True in a TransientModel - _transient_max_count = None - _transient_max_hours = None - _transient_check_time = 20 # structure: # { 'parent_model': 'm2o_field', ... } @@ -5067,20 +5064,26 @@ class BaseModel(object): 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, 600) + 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 + cr.execute( + "SELECT count(*) as row_count FROM %s", (self._table, )) + 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, 600) def _transient_vacuum(self, cr, uid, force=False): """Clean the transient records. @@ -5092,10 +5095,12 @@ class BaseModel(object): a new record is created). """ 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: From 2747e5bef21ef9361900d61218e594b85ed22b20 Mon Sep 17 00:00:00 2001 From: Ronald Portier Date: Wed, 24 Oct 2012 16:01:19 +0200 Subject: [PATCH 2/3] [FIX] Correct both age based as count based vacuum cleaning. lp bug: https://launchpad.net/bugs/1009014 fixed bzr revid: ronald@therp.nl-20121024140119-hv72ffxmf1gvf2sj --- openerp/osv/orm.py | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/openerp/osv/orm.py b/openerp/osv/orm.py index 6916f94730a..ca2a4fb6268 100644 --- a/openerp/osv/orm.py +++ b/openerp/osv/orm.py @@ -5065,7 +5065,7 @@ class BaseModel(object): def _transient_clean_rows_older_than(self, cr, seconds): 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, 600) + seconds = max(seconds, 300) now_str = "(now() at time zone 'UTC')" cr.execute( "SELECT id FROM " + self._table + " WHERE" @@ -5077,13 +5077,13 @@ class BaseModel(object): def _transient_clean_old_rows(self, cr, max_count): # Check how many rows we have in the table - cr.execute( - "SELECT count(*) as row_count FROM %s", (self._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, 600) + self._transient_clean_rows_older_than(cr, 300) def _transient_vacuum(self, cr, uid, force=False): """Clean the transient records. @@ -5093,12 +5093,20 @@ class BaseModel(object): 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 + _transient_check_time = 20 # arbitrary limit on vacuum executions self._transient_check_count += 1 if ((not force) - and (self._transient_check_count >= _transient_check_time)): + and (self._transient_check_count < _transient_check_time)): return True # no vacuum cleaning this time self._transient_check_count = 0 From 8ead65d82a47afde1d162597244a9a2089ee473c Mon Sep 17 00:00:00 2001 From: Raphael Collet Date: Fri, 21 Dec 2012 16:29:14 +0100 Subject: [PATCH 3/3] [IMP] orm: fix vacuum of transient model bzr revid: rco@openerp.com-20121221152914-qs1s9h1zwnyzft1m --- openerp/osv/orm.py | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/openerp/osv/orm.py b/openerp/osv/orm.py index ca2a4fb6268..dfb4fdd0fa7 100644 --- a/openerp/osv/orm.py +++ b/openerp/osv/orm.py @@ -5064,24 +5064,20 @@ class BaseModel(object): def _transient_clean_rows_older_than(self, cr, seconds): assert self._transient, "Model %s is not transient, it cannot be vacuumed!" % self._name - '''Never delete rows used in last 5 minutes''' + # Never delete rows used in last 5 minutes seconds = max(seconds, 300) - now_str = "(now() at time zone 'UTC')" - cr.execute( - "SELECT id FROM " + self._table + " WHERE" - " COALESCE(write_date, create_date, " + now_str + ")::timestamp" - "< (" + now_str + " - interval %s)", ("%s seconds" % seconds,)) + query = ("SELECT id FROM " + self._table + " WHERE" + " COALESCE(write_date, create_date, (now() at time zone 'UTC'))::timestamp" + " < ((now() at time zone 'UTC') - interval %s)") + cr.execute(query, ("%s seconds" % seconds,)) ids = [x[0] for x in cr.fetchall()] - if ids: - self.unlink(cr, SUPERUSER_ID, 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) + cr.execute("SELECT count(*) AS row_count FROM " + self._table) res = cr.fetchall() - row_count = res[0][0] - if row_count <= max_count: + if res[0][0] <= max_count: return # max not reached, nothing to do self._transient_clean_rows_older_than(cr, 300) @@ -5103,10 +5099,9 @@ class BaseModel(object): - 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 + _transient_check_time = 20 # arbitrary limit on vacuum executions self._transient_check_count += 1 - if ((not force) - and (self._transient_check_count < _transient_check_time)): + if not force and (self._transient_check_count < _transient_check_time): return True # no vacuum cleaning this time self._transient_check_count = 0