launchpad-reviewers team mailing list archive

[Stuart Bishop <stuart.bishop@xxxxxxxxxxxxx>]

Stuart Bishop has proposed merging lp:~stub/launchpad/db-deploy into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #752841 in Launchpad itself: "db deploys require manual  checks and fiddling during schema process"
  https://bugs.launchpad.net/launchpad/+bug/752841
  Bug #777559 in Launchpad itself: "DB deploy preflight check must ignore SSO database connections"
  https://bugs.launchpad.net/launchpad/+bug/777559

For more details, see:
https://code.launchpad.net/~stub/launchpad/db-deploy/+merge/60491

Connections to the SSO database should not block production rollouts.

The SSO database is not subscribed to the main Launchpad replications set. This is the only set of tables that is locked during a database update, so connections to the SSO database do not affect rollouts (this is by design).
-- 
https://code.launchpad.net/~stub/launchpad/db-deploy/+merge/60491
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stub/launchpad/db-deploy into lp:launchpad.

=== modified file 'database/schema/preflight.py'
--- database/schema/preflight.py	2011-04-27 09:13:48 +0000
+++ database/schema/preflight.py	2011-05-10 11:33:17 +0000
@@ -26,7 +26,7 @@
 
 
 # Ignore connections by these users.
-SYSTEM_USERS = frozenset(['postgres', 'slony', 'nagios'])
+SYSTEM_USERS = frozenset(['postgres', 'slony', 'nagios', 'lagmon'])
 
 # How lagged the cluster can be before failing the preflight check.
 MAX_LAG = timedelta(seconds=45)
@@ -37,14 +37,32 @@
         self.log = log
         self.is_replicated = replication.helpers.slony_installed(master_con)
         if self.is_replicated:
-            self.nodes = replication.helpers.get_all_cluster_nodes(master_con)
+            self.nodes = set(
+                replication.helpers.get_all_cluster_nodes(master_con))
             for node in self.nodes:
                 node.con = psycopg2.connect(node.connection_string)
                 node.con.set_isolation_level(ISOLATION_LEVEL_AUTOCOMMIT)
         else:
             node = replication.helpers.Node(None, None, None, True)
             node.con = master_con
-            self.nodes = [node]
+            self.nodes = set([node])
+
+        # Create a list of nodes subscribed to the replicated sets we
+        # are modifying.
+        cur = master_con.cursor()
+        cur.execute("""
+            WITH subscriptions AS (
+                SELECT *
+                FROM _sl.sl_subscribe
+                WHERE sub_set = 1 AND sub_active IS TRUE)
+            SELECT sub_provider FROM subscriptions
+            UNION
+            SELECT sub_receiver FROM subscriptions
+            """)
+        lpmain_node_ids = set(row[0] for row in cur.fetchall())
+        self.lpmain_nodes = set(
+            node for node in self.nodes
+            if node.node_id in lpmain_node_ids)
 
     def check_is_superuser(self):
         """Return True if all the node connections are as superusers."""
@@ -65,12 +83,16 @@
         return success
 
     def check_open_connections(self):
-        """Return False if any nodes have connections from non-system users.
+        """False if any lpmain nodes have connections from non-system users.
+
+        We only check on subscribed nodes, as there will be active systems
+        connected to other nodes in the replication cluster (such as the
+        SSO servers).
 
         System users are defined by SYSTEM_USERS.
         """
         success = True
-        for node in self.nodes:
+        for node in self.lpmain_nodes:
             cur = node.con.cursor()
             cur.execute("""
                 SELECT datname, usename, COUNT(*) AS num_connections