launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:comma-separated-standby-dbs into launchpad:master.

Commit message:
Accept a comma-separated list in database.rw_main_slave

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/403277

We currently have a rather complex arrangement of production config files whose purpose is to distribute load between two standby databases (although in fact both happen to point to the same actual database in pgbouncer at the moment), and it would be useful to be able to simplify this.

Accept a comma-separated list of connection strings, and select randomly between them.  This should be good enough at distributing load if and when we ever have multiple standby databases again, and will allow us to disentangle the production configs a little bit.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:comma-separated-standby-dbs into launchpad:master.

diff --git a/lib/lp/services/config/__init__.py b/lib/lp/services/config/__init__.py
index 9c4e3c1..64e47b7 100644
--- a/lib/lp/services/config/__init__.py
+++ b/lib/lp/services/config/__init__.py
@@ -20,6 +20,7 @@ except ImportError:
     import importlib_resources as resources
 import logging
 import os
+import random
 import sys
 
 from lazr.config import ImplicitTypeSchema
@@ -32,6 +33,10 @@ from six.moves.urllib.parse import (
 import ZConfig
 
 from lp.services.osutils import open_for_writing
+from lp.services.propertycache import (
+    cachedproperty,
+    get_property_cache,
+    )
 
 
 __all__ = [
@@ -500,9 +505,9 @@ class DatabaseConfig:
     def main_master(self):
         return self.rw_main_master
 
-    @property
+    @cachedproperty
     def main_slave(self):
-        return self.rw_main_slave
+        return random.choice(self.rw_main_slave.split(','))
 
     def override(self, **kwargs):
         """Override one or more config attributes.
@@ -517,9 +522,12 @@ class DatabaseConfig:
                     delattr(self.overrides, attr)
             else:
                 setattr(self.overrides, attr, value)
+                if attr == 'rw_main_slave':
+                    del get_property_cache(self).main_slave
 
     def reset(self):
         self.overrides = DatabaseConfigOverrides()
+        del get_property_cache(self).main_slave
 
     def _getConfigSections(self):
         """Returns a list of sections to search for database configuration.
diff --git a/lib/lp/services/config/tests/test_database_config.py b/lib/lp/services/config/tests/test_database_config.py
index 76780f4..d4261b6 100644
--- a/lib/lp/services/config/tests/test_database_config.py
+++ b/lib/lp/services/config/tests/test_database_config.py
@@ -4,6 +4,7 @@
 __metaclass__ = type
 
 from lp.services.config import DatabaseConfig
+from lp.services.propertycache import get_property_cache
 from lp.testing import TestCase
 from lp.testing.layers import DatabaseLayer
 
@@ -41,3 +42,19 @@ class TestDatabaseConfig(TestCase):
         self.assertEqual('not_launchpad', dbc.dbuser)
         dbc.reset()
         self.assertEqual('launchpad_main', dbc.dbuser)
+
+    def test_main_slave(self):
+        # If rw_main_slave is a comma-separated list, then the main_slave
+        # property selects among them randomly, and caches the result.
+        dbc = DatabaseConfig()
+        original_standby = dbc.main_slave
+        standbys = [
+            'dbname=launchpad_standby1 port=5433',
+            'dbname=launchpad_standby2 port=5433',
+            ]
+        dbc.override(rw_main_slave=','.join(standbys))
+        selected_standby = dbc.main_slave
+        self.assertIn(selected_standby, standbys)
+        self.assertEqual(selected_standby, get_property_cache(dbc).main_slave)
+        dbc.reset()
+        self.assertEqual(original_standby, dbc.main_slave)