launchpad-dev team mailing list archive

[Guilherme Salgado <salgado@xxxxxxxxxxxxx>]

On Wed, 2010-01-06 at 12:43 -0500, Francis J. Lacoste wrote:
> On January 6, 2010, Guilherme Salgado wrote:
> > On Wed, 2010-01-06 at 20:26 +0700, Stuart Bishop wrote:
> > > On Wed, Jan 6, 2010 at 8:20 PM, Guilherme Salgado <salgado@xxxxxxxxxxxxx> 
> wrote:
> > > >> > After realizing that I came up with another approach, which relies
> > > >> > only on the presence/absence of the read-only.txt file to figure out
> > > >> > the mode we're on.  On this approach,
> > > >> > config.database.main_master/slave are gone and we use
> > > >> > dbconfig.main_master/slave instead, which are properties in
> > > >> > DatabaseConfig that return the appropriate value according to the
> > > >> > mode we're on.
> > > >>
> > > >> Does this mean we're checking for the presence of this text file
> > > >> before every database operation? That sounds quite IO intensive.
> > > >
> > > > ISTM that the presence of the file would be checked only a couple times
> > > > (once for each of the properties in DatabaseConfig that look for that
> > > > file) for each handler thread, as a consequence of storm creating the
> > > > DB connections when they're first used.
> > > >
> > > > If that's correct, then we'll have to find a way to reset the stores in
> > > > all threads when we switch modes -- something I didn't realize before.
> > >
> > > You can look for this file at the start of the request when installing
> > > the database policy and change the database settings for just the
> > > current thread. That is just one stat() per request rather than per
> > > database operation. We certainly wouldn't want the appserver to be
> > > changing from normal to read-only mode half way through handling a
> > > request.
> > 
> > I guess that'd work, but since we have other places[1] which check
> > whether or not we're in read only mode on every request, I think we
> > should cache that information somewhere so that we don't issue more than
> > one stat() call (on read-only.txt) for each request.  Since we'll be
> > checking that file at the beginning of every request, we could cache it
> > in request.annotations, maybe?
> > 
> > [1] LaunchpadDatabase.connection_factory
> >     LaunchpadSecurityPolicy.checkPermission()
> >     LaunchpadDatabasePolicyFactory and WebServiceDatabasePolicyFactory
> > 
> 
> Yes, I think request.annotations is the right place to save this information.
> 

As I explained earlier, to make the read-only mode switching
transparent, I turned the main_master/slave config variables into
properties of DatabaseConfig, which return either rw_main_* or
ro_main_*, depending on the mode we're on. 

After our discussion we decided to cache the mode in the request so that
we only look for the file once for each request (and, more importantly,
it doesn't change mid-request).  One small problem with that, though, is
that DatabaseConfig.main_master is called in some contexts which don't
have a request, so we need to cope with that.  It doesn't sound like a
show-stopper to me, but I'd like to know what you think.

Here's the utility we discussed, which deals with a missing request
nicely: http://paste.ubuntu.com/353092/

Cheers,

-- 
Guilherme Salgado <salgado@xxxxxxxxxxxxx>

Attachment: signature.asc
Description: This is a digitally signed message part