Colin Watson has proposed merging
~cjwatson/launchpad:twisted-start-feature-controller-earlier into
launchpad:master.
Commit message:
Start feature controller earlier in Twisted services
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/447085
After migrating buildd-manager to a charmed deployment, it quite frequently
seems to start up in a bad state, where all its database operations fail with
errors like "permission denied for relation builder".
I can reproduce this on staging, but only sometimes, and unreliably: it seems
to be a race condition of some kind. My best guess is that it has something to
do with the different database connection arrangements for charmed deployments,
where we switch to the desired database role after connecting. As such, I
guessed that setting up the feature controller earlier in the service startup
sequence might help, since that makes a database connection, and it would make
some sense that it might matter whether this happens before or after we start a
bunch of threads; and indeed when I try that I don't seem to be able to
reproduce the problem on staging any more.
This isn't entirely satisfactory, and I'd like to understand the problem
better, but perhaps this will do for now.
--
Your team Launchpad code reviewers is requested to review the proposed merge of
~cjwatson/launchpad:twisted-start-feature-controller-earlier into
launchpad:master.
diff --git a/daemons/buildd-manager.tac b/daemons/buildd-manager.tac
index 234c976..876ecae 100644
--- a/daemons/buildd-manager.tac
+++ b/daemons/buildd-manager.tac
@@ -23,6 +23,11 @@ dbconfig.override(dbuser="buildd_manager", isolation_level="read_committed")
# Should be removed from callsites verified to not need it.
set_immediate_mail_delivery(True)
+# Allow use of feature flags. Do this before setting up the Twisted
+# application, in order to ensure that we switch to the correct database
+# role before starting any threads.
+setup_feature_controller("buildd-manager")
+
# ampoule uses five file descriptors per subprocess (i.e.
# 5 * config.builddmaster.download_connections); we also need at least three
# per active builder for resuming virtualized builders or making XML-RPC
@@ -46,6 +51,3 @@ readyservice.ReadyService().setServiceParent(application)
# Service for scanning buildd workers.
service = BuilddManager()
service.setServiceParent(application)
-
-# Allow use of feature flags.
-setup_feature_controller("buildd-manager")
diff --git a/daemons/librarian.tac b/daemons/librarian.tac
index 062ef54..4522b9c 100644
--- a/daemons/librarian.tac
+++ b/daemons/librarian.tac
@@ -45,6 +45,11 @@ execute_zcml_for_scripts(
scriptzcmlfilename="librarian.zcml", setup_interaction=False
)
+# Allow use of feature flags. Do this before setting up the Twisted
+# application, in order to ensure that we switch to the correct database
+# role before starting any threads.
+setup_feature_controller("librarian")
+
if os.environ.get("LP_TEST_INSTANCE"):
# Running in ephemeral mode: get the root dir from the environment and
# dynamically allocate ports.
@@ -121,9 +126,6 @@ logfile = options.get("logfile")
observer = set_up_oops_reporting("librarian", "librarian", logfile)
application.addComponent(observer, ignoreClass=1)
-# Allow use of feature flags.
-setup_feature_controller("librarian")
-
# Setup a signal handler to dump the process' memory upon 'kill -44'.
def sigdumpmem_handler(signum, frame):
diff --git a/daemons/numbercruncher.tac b/daemons/numbercruncher.tac
index 8efe410..f396f32 100644
--- a/daemons/numbercruncher.tac
+++ b/daemons/numbercruncher.tac
@@ -16,6 +16,11 @@ from lp.services.twistedsupport.loggingsupport import RotatableFileLogObserver
execute_zcml_for_scripts()
+# Allow use of feature flags. Do this before setting up the Twisted
+# application, in order to ensure that we switch to the correct database
+# role before starting any threads.
+setup_feature_controller("number-cruncher")
+
options = ServerOptions()
options.parseOptions()
@@ -31,6 +36,3 @@ readyservice.ReadyService().setServiceParent(application)
# Service for updating statsd receivers.
service = NumberCruncher()
service.setServiceParent(application)
-
-# Allow use of feature flags.
-setup_feature_controller("number-cruncher")
_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
