Merge lp:~therve/storm/postgres-repeatable-read into lp:storm

[Thomas Herve]

The branch adds support for the 2 others serialization level in the URI, and it switches to repeatable read by default for Postgres 9. See http://wiki.postgresql.org/wiki/Serializable#PostgreSQL_Implementation for the reason why it makes sense. To have this support, psycopg2 2.4 needs to be used. Older versions will fallback to serializable.

The tests is fairly ugly, but I didn't any better idea...

[Jamu Kakar (jkakar)]

This looks good to me, +1! One thought is that you could have some
surprised if you manually specify 'isolation=repeatable-read' in your
connection string and then subsequently connect to a version of
PostgreSQL you don't expect. I'm not really sure what we can do about
that though, so probably just need to be clear in our documentation
about what each isolation level means for different versions of
PostgreSQL.

review: Approve

[Stuart Bishop (stub)]

This can be simpler by always using repeatable read, no matter the PostgreSQL version. As the SQL standards defined the 4 levels, when an application requests an isolation the database needs to provide *at least* that much protection. PostgreSQL <9.0 supports setting the isolation level to 'repeatable read' and does the right thing by internally using an actual isolation protection of 'serializable'. It even reports the isolation level as 'repeatable read'. http://www.postgresql.org/docs/8.4/static/transaction-iso.html for details.

psycopg2 < 2.4 does seem to be broken in that it decided to bump up the isolation level itself, rather than leave it to PostgreSQL to make the decision. I'd suggest dropping support for < 2.4, except that Launchpad is still on 2.2 and keeping support makes my life easier :-)

[lp:~therve/storm/postgres-repeatable-read]

443. By Thomas Herve on 2012-03-30

Remove the dynamic setting, and use REPEATABLE READ everywhere

[Thomas Herve (therve)]

Thanks for the insights, I made the changes accordingly. If you use psycogp2 < 2.4 it keeps the same behavior everywhere. Starting 2.4 it uses REPEATABLE READ everywhere, which is equivalent with serializable on Postgres < 9, but different starting with 9.

[lp:~therve/storm/postgres-repeatable-read]

444. By Thomas Herve on 2012-03-30

Add NEWS statement

[Stuart Bishop (stub)]

review: Approve

=== modified file 'NEWS'
--- NEWS	2012-03-27 10:45:17 +0000
+++ NEWS	2012-03-30 08:45:23 +0000
@@ -33,6 +33,11 @@
 - Add a new Distinct expression for pre-pending the 'DISTINCT' token to
   SQL expressions that support it (like columns).
 
+- Switch to REPEATABLE READ as isolation level for Postgres. If you use
+  psycopg2 < 2.4, it doesn't change anything. For psycopg2 2.4 and newer, it
+  will keep the same behavior on Postgres < 9 as it's equivalent to
+  SERIALIZABLE, but it will be less strict on Postgres >= 9.
+
 Bug fixes
 ---------
 

=== modified file 'storm/databases/postgres.py'
--- storm/databases/postgres.py	2012-03-10 15:16:18 +0000
+++ storm/databases/postgres.py	2012-03-30 08:45:23 +0000
@@ -345,12 +345,16 @@
                 "'psycopg2' >= %s not found. Found %s."
                 % (REQUIRED_PSYCOPG2_VERSION, PSYCOPG2_VERSION))
         self._dsn = make_dsn(uri)
-        isolation = uri.options.get("isolation", "serializable")
+        isolation = uri.options.get("isolation", "repeatable-read")
         isolation_mapping = {
             "autocommit": psycopg2.extensions.ISOLATION_LEVEL_AUTOCOMMIT,
             "serializable": psycopg2.extensions.ISOLATION_LEVEL_SERIALIZABLE,
-            "read-committed": psycopg2.extensions.ISOLATION_LEVEL_READ_COMMITTED,
-        }
+            "read-committed":
+                psycopg2.extensions.ISOLATION_LEVEL_READ_COMMITTED,
+            "repeatable-read":
+                psycopg2.extensions.ISOLATION_LEVEL_REPEATABLE_READ,
+            "read-uncommitted":
+                psycopg2.extensions.ISOLATION_LEVEL_READ_UNCOMMITTED}
         try:
             self._isolation = isolation_mapping[isolation]
         except KeyError:

=== modified file 'tests/databases/postgres.py'
--- tests/databases/postgres.py	2012-03-08 14:01:31 +0000
+++ tests/databases/postgres.py	2012-03-30 08:45:23 +0000
@@ -553,8 +553,17 @@
         connection.rollback()
 
     def test_default_isolation(self):
+        """
+        The default isolation level is REPEATABLE READ, but it's only supported
+        by psycopg2 2.4 and newer. Before, SERIALIZABLE is used instead.
+        """
         result = self.connection.execute("SHOW TRANSACTION ISOLATION LEVEL")
-        self.assertEquals(result.get_one()[0], u"serializable")
+        import psycopg2
+        psycopg2_version = psycopg2.__version__.split(None, 1)[0]
+        if psycopg2_version < "2.4":
+            self.assertEquals(result.get_one()[0], u"serializable")
+        else:
+            self.assertEquals(result.get_one()[0], u"repeatable read")
 
     def test_unknown_serialization(self):
         self.assertRaises(ValueError, create_database,
@@ -696,13 +705,13 @@
             "SELECT current_database()")
 
     if has_psycopg:
-        import psycopg2._psycopg
-        psycopg2_version = psycopg2._psycopg.__version__.split(None, 1)[0]
+        import psycopg2
+        psycopg2_version = psycopg2.__version__.split(None, 1)[0]
         if psycopg2_version in ("2.4", "2.4.1", "2.4.2"):
             test_pgbouncer_stopped.skip = (
                 "Fails with pyscopg2 %s; see https://bugs.launchpad"
                 ".net/storm/+bug/900702.") % psycopg2_version
- 
+
 
 if has_fixtures:
     # Upgrade to full test case class with fixtures.