Merge lp:~stub/charms/precise/postgresql/bug-1278731-hot-standby-allowed-units into lp:charms/postgresql

[Stuart Bishop]

Fix for Bug #1278731.

It was possible for hot standby units to have their db relation hooks run before the master, in which case they would copy obsolete connection details from the master. Clients following the db relation protocol would rightfully ignore the obsolete credentials (in this case, an invalid database name) and wait for the server unit to catch up (which it never did).

Now the hot standby explicitly overrides the database name in this case, and blocks waiting for the master to create the database and for it to replicate.

[Stuart Bishop (stub)]

It seems that with this fix, along with a little more sleep()ing, the replication tests are no longer flaky with the local provider. We might need to increase the sleeps further for other providers.

[Marco Ceppi (marcoceppi)]

LGTM, +1

review: Approve

=== modified file 'hooks/hooks.py'
--- hooks/hooks.py	2014-02-11 09:59:00 +0000
+++ hooks/hooks.py	2014-02-11 09:59:01 +0000
@@ -248,7 +248,8 @@
 
 def run(command, exit_on_error=True, quiet=False):
     '''Run a command and return the output.'''
-    log("Running {!r}".format(command), DEBUG)
+    if not quiet:
+        log("Running {!r}".format(command), DEBUG)
     p = subprocess.Popen(
         command, stdin=subprocess.PIPE, stdout=subprocess.PIPE,
         shell=isinstance(command, basestring))
@@ -1372,8 +1373,8 @@
 
     # Update the peer relation, notifying any hot standby units
     # to republish connection details to the client relation.
-    local_state['client_relations'] = ' '.join(
-        hookenv.relation_ids('db') + hookenv.relation_ids('db-admin'))
+    local_state['client_relations'] = ' '.join(sorted(
+        hookenv.relation_ids('db') + hookenv.relation_ids('db-admin')))
     log("Client relations {}".format(local_state['client_relations']))
     local_state.publish()
 
@@ -1861,6 +1862,13 @@
         connection_settings['host'] = hookenv.unit_private_ip()
         connection_settings['port'] = get_service_port()
         connection_settings['state'] = local_state['state']
+        requested_db = hookenv.relation_get('database')
+        # A hot standby might have seen a database name change before
+        # the master, so override. This is no problem because we block
+        # until this database has been created on the master and
+        # replicated through to this unit.
+        if requested_db:
+            connection_settings['database'] = requested_db
 
         # Block until users and database has replicated, so we know the
         # connection details we publish are actually valid. This will
@@ -1877,7 +1885,8 @@
                 connection_settings['database']))
             time.sleep(10)
 
-        log("Connection settings {!r}".format(connection_settings), DEBUG)
+        log("Relation {} connection settings {!r}".format(
+            client_relation, connection_settings), DEBUG)
         hookenv.relation_set(
             client_relation, relation_settings=connection_settings)
 

=== modified file 'test.py'
--- test.py	2014-02-11 09:59:00 +0000
+++ test.py	2014-02-11 09:59:01 +0000
@@ -101,10 +101,23 @@
             # Due to Bug #1191079, we need to send the whole remote command
             # as a single argument.
             ' '.join(psql_cmd + psql_args)]
-        out = run(self, cmd, input=sql)
-        result = [line.split(',') for line in out.splitlines()]
-        self.addDetail('sql', text_content(repr((sql, result))))
-        return result
+        # The hooks are complex, and it is common that relations haven't
+        # yet been setup despite sleep()ing after juju reports
+        # everything is up. Increasing the sleep() in jujufixture
+        # further is just getting crazy, so instead here we loop a few
+        # times with a shorter wait.
+        attempts = 0
+        while True:
+            attempts += 1
+            try:
+                out = run(self, cmd, input=sql)
+                result = [line.split(',') for line in out.splitlines()]
+                self.addDetail('sql', text_content(repr((sql, result))))
+                return result
+            except subprocess.CalledProcessError:
+                if attempts >= 3:
+                    raise
+            time.sleep(30)
 
     def pg_ctlcluster(self, unit, command):
         cmd = [