launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/9.1-serializable-is-special into lp:launchpad with lp:~wgrant/launchpad/psycopg2-2.4.4 as a prerequisite.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #973159 in Launchpad itself: "Uses SERIALIZABLE isolation on PostgreSQL 9.1"
  https://bugs.launchpad.net/launchpad/+bug/973159

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/9.1-serializable-is-special/+merge/100913

Appservers currently ask psycopg2 for SERIALIZABLE isolation. In PostgreSQL 9.1 this uses SSI, which is far more expensive than 8.4's implementation. 9.1's REPEATABLE READ is equivalent to 8.4's SERIALIZABLE, and in 8.4 REPEATABLE READ is just an alias for SERIALIZABLE, so we should use REPEATABLE READ instead.

This branch adds support for REPEATABLE READ, and changes SERIALIZABLE defaults to REPEATABLE READ.
-- 
https://code.launchpad.net/~wgrant/launchpad/9.1-serializable-is-special/+merge/100913
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/9.1-serializable-is-special into lp:launchpad.

=== modified file 'lib/lp/registry/doc/convert-person-to-team.txt'
--- lib/lp/registry/doc/convert-person-to-team.txt	2011-12-30 06:14:56 +0000
+++ lib/lp/registry/doc/convert-person-to-team.txt	2012-04-05 06:30:43 +0000
@@ -25,7 +25,7 @@
     0
 
     # The script already committed its transaction but this test runs
-    # the LaunchpadFunctionalLayer which, in turn, uses the SERIALIZABLE
+    # the LaunchpadFunctionalLayer which, in turn, uses the REPEATABLE READ
     # isolation level, so we need to forcibly begin another transaction here.
     >>> import transaction; transaction.abort()
 

=== modified file 'lib/lp/services/config/schema-lazr.conf'
--- lib/lp/services/config/schema-lazr.conf	2012-03-27 02:20:56 +0000
+++ lib/lp/services/config/schema-lazr.conf	2012-04-05 06:30:43 +0000
@@ -542,7 +542,7 @@
 # will not be used.
 max_usable_lag: 120
 
-isolation_level: serializable
+isolation_level: repeatable_read
 
 # SQL statement timeout in milliseconds. If a statement
 # takes longer than this to execute, then it will be aborted.

=== modified file 'lib/lp/services/config/tests/test_database_config.py'
--- lib/lp/services/config/tests/test_database_config.py	2012-01-01 02:58:52 +0000
+++ lib/lp/services/config/tests/test_database_config.py	2012-04-05 06:30:43 +0000
@@ -21,7 +21,7 @@
         # dbuser and isolation_level can be overridden at runtime.
         dbc = DatabaseConfig()
         self.assertEqual('launchpad_main', dbc.dbuser)
-        self.assertEqual('serializable', dbc.isolation_level)
+        self.assertEqual('repeatable_read', dbc.isolation_level)
 
         # dbuser and isolation_level overrides both work.
         dbc.override(dbuser='not_launchpad', isolation_level='autocommit')
@@ -36,7 +36,7 @@
         # Overriding with None removes the override.
         dbc.override(dbuser=None, isolation_level=None)
         self.assertEqual('launchpad_main', dbc.dbuser)
-        self.assertEqual('serializable', dbc.isolation_level)
+        self.assertEqual('repeatable_read', dbc.isolation_level)
 
     def test_reset(self):
         # reset() removes any overrides.

=== modified file 'lib/lp/services/database/multitablecopy.py'
--- lib/lp/services/database/multitablecopy.py	2011-12-30 06:47:17 +0000
+++ lib/lp/services/database/multitablecopy.py	2012-04-05 06:30:43 +0000
@@ -158,7 +158,7 @@
 
     This stage will lock the rows that are being inserted in the source
     tables, if the database is so inclined (e.g. when using postgres with
-    SERIALIZABLE isolation level).  For that reason, the pouring is done in
+    REPEATABLE READ isolation level).  For that reason, the pouring is done in
     smaller, controlled batches.  If you give the object a database
     transaction to work with, that transaction will be committed and restarted
     between batches.

=== modified file 'lib/lp/services/database/sqlbase.py'
--- lib/lp/services/database/sqlbase.py	2012-03-27 13:38:55 +0000
+++ lib/lp/services/database/sqlbase.py	2012-04-05 06:30:43 +0000
@@ -14,6 +14,7 @@
     'ISOLATION_LEVEL_AUTOCOMMIT',
     'ISOLATION_LEVEL_DEFAULT',
     'ISOLATION_LEVEL_READ_COMMITTED',
+    'ISOLATION_LEVEL_REPEATABLE_READ',
     'ISOLATION_LEVEL_SERIALIZABLE',
     'quote',
     'quote_like',
@@ -33,6 +34,7 @@
 from psycopg2.extensions import (
     ISOLATION_LEVEL_AUTOCOMMIT,
     ISOLATION_LEVEL_READ_COMMITTED,
+    ISOLATION_LEVEL_REPEATABLE_READ,
     ISOLATION_LEVEL_SERIALIZABLE,
     )
 import pytz

=== modified file 'lib/lp/services/database/tests/script_isolation.py'
--- lib/lp/services/database/tests/script_isolation.py	2011-12-30 06:20:00 +0000
+++ lib/lp/services/database/tests/script_isolation.py	2012-04-05 06:30:43 +0000
@@ -44,6 +44,6 @@
 disconnect_stores()
 check()
 
-dbconfig.override(isolation_level='serializable')
+dbconfig.override(isolation_level='repeatable_read')
 disconnect_stores()
 check()

=== modified file 'lib/lp/services/database/tests/test_isolation_changes.py'
--- lib/lp/services/database/tests/test_isolation_changes.py	2011-12-30 06:20:00 +0000
+++ lib/lp/services/database/tests/test_isolation_changes.py	2012-04-05 06:30:43 +0000
@@ -74,6 +74,10 @@
         set_isolation_level('read_committed')
         self.failUnlessEqual(self.getCurrentIsolation(), 'read committed')
 
+    def test_repeatableRead(self):
+        set_isolation_level('repeatable_read')
+        self.failUnlessEqual(self.getCurrentIsolation(), 'repeatable read')
+
     def test_serializable(self):
         set_isolation_level('serializable')
         self.failUnlessEqual(self.getCurrentIsolation(), 'serializable')
@@ -113,8 +117,8 @@
         self.failUnlessEqual(script_output, dedent("""\
                 read committed
                 read committed
-                serializable
-                serializable
+                repeatable read
+                repeatable read
                 """))
 
     def test_connect(self):

=== modified file 'lib/lp/services/webapp/adapter.py'
--- lib/lp/services/webapp/adapter.py	2012-03-06 23:39:08 +0000
+++ lib/lp/services/webapp/adapter.py	2012-04-05 06:30:43 +0000
@@ -25,6 +25,7 @@
 from psycopg2.extensions import (
     ISOLATION_LEVEL_AUTOCOMMIT,
     ISOLATION_LEVEL_READ_COMMITTED,
+    ISOLATION_LEVEL_REPEATABLE_READ,
     ISOLATION_LEVEL_SERIALIZABLE,
     QueryCanceledError,
     )
@@ -467,6 +468,7 @@
 isolation_level_map = {
     'autocommit': ISOLATION_LEVEL_AUTOCOMMIT,
     'read_committed': ISOLATION_LEVEL_READ_COMMITTED,
+    'repeatable_read': ISOLATION_LEVEL_REPEATABLE_READ,
     'serializable': ISOLATION_LEVEL_SERIALIZABLE,
     }
 
@@ -543,7 +545,7 @@
         flags = _get_dirty_commit_flags()
 
         if dbconfig.isolation_level is None:
-            self._isolation = ISOLATION_LEVEL_SERIALIZABLE
+            self._isolation = ISOLATION_LEVEL_REPEATABLE_READ
         else:
             self._isolation = isolation_level_map[dbconfig.isolation_level]