New rdbms configuration option 'isolation_level'

See discussion in http://issues.roundup-tracker.org/issue2550806

Author: schlatterbeck

CHANGES.txt

@@ -32,6 +32,14 @@ Features:
   value as an ID when evaluating form values -- not as a key. Specifying
   try_id_parsing='no' for these Link/Multilink will skip the ID step,
   default is 'yes'.
+- New configuration option 'isolation_level' in rdbms section. Currently
+  supported for Postgres and mysql, sets the transaction isolation level.
+  Wrong history entries for concurrent database updates observed in
+  issue2550806 can be prevented by setting this to 'repeatable read' if
+  you want to pay the performance penalty. We test this behaviour in the
+  regression tests for Postgres but not currently for mysql.
+  See http://www.postgresql.org/docs/9.1/static/transaction-iso.html
+  (Ralf Schlatterbeck)
 
 Fixed:
 

roundup/backends/back_mysql.py

@@ -40,6 +40,13 @@
 from MySQLdb.constants import ER
 import logging
 
+isolation_levels = \
+    { 'read uncommitted': 'READ UNCOMMITTED'
+    , 'read committed': 'READ COMMITTED'
+    , 'repeatable read': 'REPEATABLE READ'
+    , 'serializable': 'SERIALIZABLE'
+    }
+
 def connection_dict(config, dbnamestr=None):
     d = rdbms_common.connection_dict(config, dbnamestr)
     if d.has_key('password'):
@@ -145,6 +152,8 @@ def sql_open_connection(self):
             raise DatabaseError, message
         cursor = conn.cursor()
         cursor.execute("SET AUTOCOMMIT=0")
+        lvl = isolation_levels [self.config.RDBMS_ISOLATION_LEVEL]
+        cursor.execute("SET SESSION TRANSACTION ISOLATION LEVEL %s" % lvl)
         cursor.execute("START TRANSACTION")
         return (conn, cursor)
 

roundup/backends/back_postgresql.py

@@ -8,20 +8,44 @@
 __docformat__ = 'restructuredtext'
 
 import os, shutil, time
+ISOLATION_LEVEL_READ_UNCOMMITTED = None
+ISOLATION_LEVEL_READ_COMMITTED = None
+ISOLATION_LEVEL_REPEATABLE_READ = None
+ISOLATION_LEVEL_SERIALIZABLE = None
 try:
     import psycopg
     from psycopg import QuotedString
     from psycopg import ProgrammingError
+    TransactionRollbackError = ProgrammingError
+    try:
+        from psycopg.extensions import ISOLATION_LEVEL_READ_UNCOMMITTED
+        from psycopg.extensions import ISOLATION_LEVEL_READ_COMMITTED
+        from psycopg.extensions import ISOLATION_LEVEL_REPEATABLE_READ
+        from psycopg.extensions import ISOLATION_LEVEL_SERIALIZABLE
+    except ImportError:
+        pass
 except:
     from psycopg2 import psycopg1 as psycopg
     from psycopg2.extensions import QuotedString
+    from psycopg2.extensions import ISOLATION_LEVEL_READ_UNCOMMITTED
+    from psycopg2.extensions import ISOLATION_LEVEL_READ_COMMITTED
+    from psycopg2.extensions import ISOLATION_LEVEL_REPEATABLE_READ
+    from psycopg2.extensions import ISOLATION_LEVEL_SERIALIZABLE
     from psycopg2.psycopg1 import ProgrammingError
+    from psycopg2.extensions import TransactionRollbackError
 import logging
 
 from roundup import hyperdb, date
 from roundup.backends import rdbms_common
 from roundup.backends import sessions_rdbms
 
+isolation_levels = \
+    { 'read uncommitted': ISOLATION_LEVEL_READ_COMMITTED
+    , 'read committed': ISOLATION_LEVEL_READ_COMMITTED
+    , 'repeatable read': ISOLATION_LEVEL_REPEATABLE_READ
+    , 'serializable': ISOLATION_LEVEL_SERIALIZABLE
+    }
+
 def connection_dict(config, dbnamestr=None):
     ''' read_default_group is MySQL-specific, ignore it '''
     d = rdbms_common.connection_dict(config, dbnamestr)
@@ -145,6 +169,9 @@ def sql_open_connection(self):
             raise hyperdb.DatabaseError(message)
 
         cursor = conn.cursor()
+        if ISOLATION_LEVEL_REPEATABLE_READ is not None:
+            lvl = isolation_levels [self.config.RDBMS_ISOLATION_LEVEL]
+            conn.set_isolation_level(lvl)
 
         return (conn, cursor)
 

roundup/configuration.py

@@ -293,6 +293,20 @@ def str2value(self, value):
         else:
             raise OptionValueError(self, value, self.class_description)
 
+class IsolationOption(Option):
+    """Database isolation levels"""
+
+    allowed = ['read uncommitted', 'read committed', 'repeatable read',
+        'serializable']
+    class_description = "Allowed values: %s" % ', '.join ("'%s'" % a
+        for a in allowed)
+
+    def str2value(self, value):
+        _val = value.lower()
+        if _val in self.allowed:
+            return _val
+        raise OptionValueError(self, value, self.class_description)
+
 class MailAddressOption(Option):
 
     """Email address
@@ -650,6 +664,10 @@ def str2value(self, value):
             "  or use template0 as template.\n"
             "then set this option to the template name given in the\n"
             "error message."),
+        (IsolationOption, 'isolation_level', 'read committed',
+            "Database isolation level, currently supported for\n"
+            "PostgreSQL and mysql. See, e.g.,\n"
+            "http://www.postgresql.org/docs/9.1/static/transaction-iso.html"),
     ), "Settings in this section are used"
         " by RDBMS backends only"
     ),

test/test_postgresql.py

@@ -21,6 +21,7 @@
 
 from db_test_base import DBTest, ROTest, config, SchemaTest, ClassicInitTest
 from db_test_base import ConcurrentDBTest, HTMLItemTest, FilterCacheTest
+from db_test_base import ClassicInitBase, setupTracker
 
 from roundup.backends import get_backend, have_backend
 
@@ -66,6 +67,54 @@ def tearDown(self):
         ConcurrentDBTest.tearDown(self)
         postgresqlOpener.tearDown(self)
 
+class postgresqlJournalTest(postgresqlOpener, ClassicInitBase):
+    backend = 'postgresql'
+    def setUp(self):
+        postgresqlOpener.setUp(self)
+        ClassicInitBase.setUp(self)
+        self.tracker = setupTracker(self.dirname, self.backend)
+        db = self.tracker.open('admin')
+        self.id = db.issue.create(title='initial value')
+        db.commit()
+        db.close()
+
+    def tearDown(self):
+        self.db1.close()
+        self.db2.close()
+        ClassicInitBase.tearDown(self)
+        postgresqlOpener.tearDown(self)
+
+    def _test_journal(self, expected_journal):
+        id  = self.id
+        db1 = self.db1 = self.tracker.open('admin')
+        db2 = self.db2 = self.tracker.open('admin')
+
+        t1  = db1.issue.get(id, 'title')
+        t2  = db2.issue.get(id, 'title')
+
+        db1.issue.set (id, title='t1')
+        db1.commit()
+        db1.close()
+
+        db2.issue.set (id, title='t2')
+        db2.commit()
+        db2.close()
+        self.db = self.tracker.open('admin')
+        journal = self.db.getjournal('issue', id)
+        for n, line in enumerate(journal):
+            self.assertEqual(line[4], expected_journal[n])
+
+    def testConcurrentReadCommitted(self):
+        expected_journal = [
+            {}, {'title': 'initial value'}, {'title': 'initial value'}
+        ]
+        self._test_journal(expected_journal)
+
+    def testConcurrentRepeatableRead(self):
+        self.tracker.config.RDBMS_ISOLATION_LEVEL='repeatable read'
+        exc = self.module.TransactionRollbackError
+        self.assertRaises(exc, self._test_journal, [])
+
 class postgresqlHTMLItemTest(postgresqlOpener, HTMLItemTest):
     backend = 'postgresql'
     def setUp(self):
@@ -132,6 +181,7 @@ def test_suite():
     suite.addTest(unittest.makeSuite(postgresqlClassicInitTest))
     suite.addTest(unittest.makeSuite(postgresqlSessionTest))
     suite.addTest(unittest.makeSuite(postgresqlConcurrencyTest))
+    suite.addTest(unittest.makeSuite(postgresqlJournalTest))
     suite.addTest(unittest.makeSuite(postgresqlHTMLItemTest))
     suite.addTest(unittest.makeSuite(postgresqlFilterCacheTest))
     return suite