Merge lp:~allenap/launchpad/oneiric-librarian-bug-871596 into lp:launchpad

[Gavin Panella]

It seems that disconnection errors behave slightly differently on Oneiric... possibly. This branch intercepts pgcode 57P01 - admin_shutdown - and allows transactions to be retried instead of just breaking at the first opportunity. Doing this lets the example test command given in the bug to complete without failure.

[Stuart Bishop (stub)]

Rather than fix this in every system, we should just fix it once in Storm. Storm catches the PostgreSQL exceptions, puts its stores in a state so they will attempt to reconnect, and raises DisconnectionError. In particular, I don't think the proposed patch works as the Storm store is being left in an inconsistent state (the next request will likely fail, and the subsequent one might reconnect).

I think this might also be the issue Garry saw with making the appserver return 503 errors when the database was unavailable.

review: Disapprove

Unmerged revisions

14139. By Gavin Panella on 2011-10-12

Use pgcode instead of string matching.

14138. By Gavin Panella on 2011-10-12

Treat unexpected disconnections from the database as a possible transient error.

=== modified file 'lib/lp/services/database/__init__.py'
--- lib/lp/services/database/__init__.py	2010-09-24 15:21:05 +0000
+++ lib/lp/services/database/__init__.py	2011-10-12 20:06:13 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """The lp.services.database package."""
@@ -9,14 +9,16 @@
     'write_transaction',
     ]
 
+from psycopg2 import OperationalError
 from psycopg2.extensions import TransactionRollbackError
-from storm.exceptions import DisconnectionError, IntegrityError
+from storm.exceptions import (
+    DisconnectionError,
+    IntegrityError,
+    )
 import transaction
 from twisted.python.util import mergeFunctionMetadata
 
-from canonical.database.sqlbase import (
-    reset_store,
-    )
+from canonical.database.sqlbase import reset_store
 
 
 RETRY_ATTEMPTS = 3
@@ -35,9 +37,18 @@
             try:
                 return func(*args, **kwargs)
             except (DisconnectionError, IntegrityError,
-                    TransactionRollbackError), exc:
+                    TransactionRollbackError):
                 if attempt >= RETRY_ATTEMPTS:
-                    raise # tried too many times
+                    raise  # tried too many times
+            except OperationalError, error:
+                # 57P01 is admin_shutdown:
+                #   FATAL: terminating connection due to administrator command
+                #   SSL connection has been closed unexpectedly
+                if error.pgcode == "57P01":
+                    if attempt >= RETRY_ATTEMPTS:
+                        raise  # tried too many times
+                else:
+                    raise
     return mergeFunctionMetadata(func, retry_transaction_decorator)
 
 
@@ -77,4 +88,3 @@
         return ret
     return retry_transaction(mergeFunctionMetadata(
         func, write_transaction_decorator))
-