launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #04807
[Merge] lp:~stub/launchpad/pgbouncer-fixture into lp:launchpad
Stuart Bishop has proposed merging lp:~stub/launchpad/pgbouncer-fixture into lp:launchpad with lp:~stub/launchpad/disco as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~stub/launchpad/pgbouncer-fixture/+merge/73283
= Summary =
We need to test disconnection handling for fastdowntime deployments.
== Proposed fix ==
Add a pgbouncer fixture. When the fixture is installed, database connections are made via the pgbouncer proxy. pgbouncer can be started and stopped at will, duplicating what happens on staging and production during fastdowntime deployments.
== Pre-implementation notes ==
== Implementation details ==
The pgbouncer debian package needs to be added to our developer dependencies before this branch can land.
== Tests ==
== Demo and Q/A ==
= Launchpad lint =
There is old lint in pgsql.py I have not fixed.
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/testing/pgsql.py
lib/lp/testing/tests/test_fixture.py
versions.cfg
setup.py
lib/lp/testing/fixture.py
configs/testrunner/launchpad-lazr.conf
./lib/lp/testing/pgsql.py
90: Line exceeds 78 characters.
270: Line exceeds 78 characters.
281: Line exceeds 78 characters.
282: Line exceeds 78 characters.
346: Line exceeds 78 characters.
350: Line exceeds 78 characters.
47: E261 at least two spaces before inline comment
90: E501 line too long (80 characters)
133: E302 expected 2 blank lines, found 0
136: E302 expected 2 blank lines, found 1
142: E302 expected 2 blank lines, found 1
151: E261 at least two spaces before inline comment
329: E225 missing whitespace around operator
341: E225 missing whitespace around operator
346: E501 line too long (88 characters)
350: E501 line too long (83 characters)
351: E225 missing whitespace around operator
365: E261 at least two spaces before inline comment
./configs/testrunner/launchpad-lazr.conf
140: Line exceeds 78 characters.
--
https://code.launchpad.net/~stub/launchpad/pgbouncer-fixture/+merge/73283
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stub/launchpad/pgbouncer-fixture into lp:launchpad.
=== modified file 'configs/testrunner/launchpad-lazr.conf'
--- configs/testrunner/launchpad-lazr.conf 2011-08-14 23:11:45 +0000
+++ configs/testrunner/launchpad-lazr.conf 2011-08-29 19:48:27 +0000
@@ -45,12 +45,12 @@
error_dir: /var/tmp/codehosting.test
[database]
-rw_main_master: dbname=launchpad_ftest
-rw_main_slave: dbname=launchpad_ftest
+rw_main_master: dbname=launchpad_ftest host=localhost
+rw_main_slave: dbname=launchpad_ftest host=localhost
# Use our _template databases here just so that we have different values from
# the rw_* configs.
-ro_main_master: dbname=launchpad_ftest_template
-ro_main_slave: dbname=launchpad_ftest_template
+ro_main_master: dbname=launchpad_ftest_template host=localhost
+ro_main_slave: dbname=launchpad_ftest_template host=localhost
randomise_select_results: true
[error_reports]
=== modified file 'lib/lp/testing/fixture.py'
--- lib/lp/testing/fixture.py 2011-07-05 15:05:21 +0000
+++ lib/lp/testing/fixture.py 2011-08-29 19:48:27 +0000
@@ -11,9 +11,15 @@
'ZopeViewReplacementFixture',
]
+from ConfigParser import SafeConfigParser
+import os.path
from textwrap import dedent
-from fixtures import Fixture
+from fixtures import (
+ EnvironmentVariableFixture,
+ Fixture,
+ )
+import pgbouncer.fixture
import rabbitfixture.server
from zope.component import (
getGlobalSiteManager,
@@ -27,6 +33,8 @@
undefineChecker,
)
+from canonical.config import config
+
class RabbitServer(rabbitfixture.server.RabbitServer):
"""A RabbitMQ server fixture with Launchpad-specific config.
@@ -46,6 +54,55 @@
""" % self.config.port)
+class PGBouncerFixture(pgbouncer.fixture.PGBouncerFixture):
+ def __init__(self):
+ super(PGBouncerFixture, self).__init__()
+
+ # Known databases
+ from canonical.testing.layers import DatabaseLayer
+ dbnames = [
+ DatabaseLayer._db_fixture.dbname,
+ DatabaseLayer._db_template_fixture.dbname,
+ 'session_ftest',
+ 'launchpad_empty',
+ ]
+ for dbname in dbnames:
+ self.databases[dbname] = 'dbname=%s port=5432 host=localhost' % (
+ dbname,)
+
+ # Known users, pulled from security.cfg
+ security_cfg_path = os.path.join(
+ config.root, 'database', 'schema', 'security.cfg')
+ security_cfg_config = SafeConfigParser({})
+ security_cfg_config.read([security_cfg_path])
+ for section_name in security_cfg_config.sections():
+ self.users[section_name] = 'trusted'
+ self.users[os.environ['USER']] = 'trusted'
+
+ def setUp(self):
+ super(PGBouncerFixture, self).setUp()
+
+ from canonical.testing.layers import reconnect_stores
+
+ # Abuse the PGPORT environment variable to get things connecting
+ # via pgbouncer. Otherwise, we would need to temporarily
+ # overwrite the database connection strings in the config.
+ pgport_fixture = EnvironmentVariableFixture('PGPORT', str(self.port))
+ pghost_fixture = EnvironmentVariableFixture('PGHOST', 'localhost')
+
+ # reconnect_store cleanup added first so it is run last, after
+ # the environment variables have been reset.
+ self.addCleanup(reconnect_stores)
+ self.addCleanup(pgport_fixture.cleanUp)
+ self.addCleanup(pghost_fixture.cleanUp)
+
+ pgport_fixture.setUp()
+ pghost_fixture.setUp()
+
+ # Reset database connections so they go through pgbouncer.
+ reconnect_stores()
+
+
class ZopeAdapterFixture(Fixture):
"""A fixture to register and unregister an adapter."""
=== modified file 'lib/lp/testing/pgsql.py'
--- lib/lp/testing/pgsql.py 2011-07-01 07:11:27 +0000
+++ lib/lp/testing/pgsql.py 2011-08-29 19:48:27 +0000
@@ -41,7 +41,10 @@
def close(self):
if self in PgTestSetup.connections:
PgTestSetup.connections.remove(self)
- self.real_connection.close()
+ try:
+ self.real_connection.close()
+ except psycopg2.InterfaceError:
+ pass # Already closed, killed etc. Ignore.
def rollback(self, InterfaceError=psycopg2.InterfaceError):
# In our test suites, rollback ends up being called twice in some
=== modified file 'lib/lp/testing/tests/test_fixture.py'
--- lib/lp/testing/tests/test_fixture.py 2011-07-05 15:05:21 +0000
+++ lib/lp/testing/tests/test_fixture.py 2011-08-29 19:48:27 +0000
@@ -8,6 +8,7 @@
from textwrap import dedent
from fixtures import EnvironmentVariableFixture
+from storm.exceptions import DisconnectionError
from zope.component import (
adapts,
queryAdapter,
@@ -17,9 +18,12 @@
Interface,
)
-from canonical.testing.layers import BaseLayer
+from canonical.launchpad.interfaces.lpstorm import IMasterStore
+from canonical.testing.layers import BaseLayer, LaunchpadZopelessLayer
+from lp.registry.model.person import Person
from lp.testing import TestCase
from lp.testing.fixture import (
+ PGBouncerFixture,
RabbitServer,
ZopeAdapterFixture,
)
@@ -89,3 +93,59 @@
self.assertIsInstance(adapter, FooToBar)
# The adapter is no longer registered.
self.assertIs(None, queryAdapter(context, IBar))
+
+
+class TestPGBouncerFixture(TestCase):
+ layer = LaunchpadZopelessLayer
+
+ def is_connected(self):
+ # First rollback any existing transaction to ensure we attempt
+ # to reconnect. We currently rollback the store explicitely
+ # rather than call transaction.abort() due to Bug #819282.
+ store = IMasterStore(Person)
+ store.rollback()
+
+ try:
+ store.find(Person).first()
+ return True
+ except DisconnectionError:
+ return False
+
+ def test_stop_and_start(self):
+ # Database is working.
+ assert self.is_connected()
+
+ # And database with the fixture is working too.
+ pgbouncer = PGBouncerFixture()
+ with PGBouncerFixture() as pgbouncer:
+ assert self.is_connected()
+
+ # pgbouncer is transparant. To confirm we are connecting via
+ # pgbouncer, we need to shut it down and confirm our
+ # connections are dropped.
+ pgbouncer.stop()
+ assert not self.is_connected()
+
+ # If we restart it, things should be back to normal.
+ pgbouncer.start()
+ assert self.is_connected()
+
+ # Database is still working.
+ assert self.is_connected()
+
+ def test_stop_no_start(self):
+ # Database is working.
+ assert self.is_connected()
+
+ # And database with the fixture is working too.
+ with PGBouncerFixture() as pgbouncer:
+ assert self.is_connected()
+
+ # pgbouncer is transparant. To confirm we are connecting via
+ # pgbouncer, we need to shut it down and confirm our
+ # connections are dropped.
+ pgbouncer.stop()
+ assert not self.is_connected()
+
+ # Database is still working.
+ assert self.is_connected()
=== modified file 'setup.py'
--- setup.py 2011-08-22 18:13:58 +0000
+++ setup.py 2011-08-29 19:48:27 +0000
@@ -60,6 +60,7 @@
'oops_datedir_repo',
'oops_wsgi',
'paramiko',
+ 'pgbouncer',
'psycopg2',
'python-memcached',
'pyasn1',
=== modified file 'versions.cfg'
--- versions.cfg 2011-08-25 10:11:02 +0000
+++ versions.cfg 2011-08-29 19:48:27 +0000
@@ -55,6 +55,7 @@
paramiko = 1.7.4
Paste = 1.7.2
PasteDeploy = 1.3.3
+pgbouncer = 0.0.2
plone.recipe.command = 1.1
psycopg2 = 2.2.2
pyasn1 = 0.0.9a
