Hi Stuart,

Just a few notes below. The main one I'm interested in is whether we should include the BuildFarmJob.status index in the db patch.

Thanks!

> === modified file 'database/replication/helpers.py'
> --- database/replication/helpers.py	2010-04-29 12:38:05 +0000
> +++ database/replication/helpers.py	2010-06-10 13:40:07 +0000
> @@ -420,7 +424,8 @@
>              """ % sqlvalues(namespace, tablename))
>          for namespace, tablename in cur.fetchall():
>              key = (namespace, tablename)
> -            if key not in tables and key not in pending_tables:
> +            if (key not in tables and key not in pending_tables
> +                and '%s.%s' % (namespace, tablename) not in IGNORED_TABLES):
>                  pending_tables.add(key)

Just out of interest, this kind of change (or these helpers) *could* be tested couldn't they?

>  
>      # Generate the set of sequences that are linked to any of our set of
> @@ -441,8 +446,9 @@
>                  ) AS whatever
>              WHERE seq IS NOT NULL;
>              """ % sqlvalues(fqn(namespace, tablename), namespace, tablename))
> -        for row in cur.fetchall():
> -            sequences.add(row[0])
> +        for sequence, in cur.fetchall():

Nice, but would it be clearer to do:

           for sequence, whateverelse in cur.fetchall():

using something appropriate?

?
> +            if sequence not in IGNORED_SEQUENCES:
> +                sequences.add(sequence)
>  
>      # We can't easily convert the sequence name to (namespace, name) tuples,
>      # so we might as well convert the tables to dot notation for consistancy.
> 
> === modified file 'database/schema/fti.py'
> --- database/schema/fti.py	2010-05-19 18:07:56 +0000
> +++ database/schema/fti.py	2010-06-10 13:40:07 +0000
> @@ -319,18 +319,15 @@
>              cmd += ' -h %s' % lp.dbhost
>          if options.dbuser:
>              cmd += ' -U %s' % options.dbuser
> -        p = popen2.Popen4(cmd)
> -        c = p.tochild
> -        print >> c, "SET client_min_messages=ERROR;"
> -        print >> c, "CREATE SCHEMA ts2;"
> -        print >> c, open(tsearch2_sql_path).read().replace(
> -                'public;','ts2, public;'
> -                )
> -        p.tochild.close()
> -        rv = p.wait()
> -        if rv != 0:
> +        p = subprocess.Popen(
> +            cmd.split(' '), stdin=subprocess.PIPE,
> +            stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
> +        out, err = p.communicate(
> +            "SET client_min_messages=ERROR; CREATE SCHEMA ts2;"
> +            + open(tsearch2_sql_path).read().replace('public;','ts2, public;'))
> +        if p.returncode != 0:
>              log.fatal('Error executing %s:', cmd)
> -            log.debug(p.fromchild.read())
> +            log.debug(out)

Is err not useful here?

>              sys.exit(rv)
>  
>      # Create ftq helper and its sibling _ftq.
> 
> === added file 'database/schema/patch-2207-62-0.sql'
> --- database/schema/patch-2207-62-0.sql	1970-01-01 00:00:00 +0000
> +++ database/schema/patch-2207-62-0.sql	2010-06-10 13:40:07 +0000
> @@ -0,0 +1,14 @@
> +SET client_min_messages=ERROR;
> +
> +-- Bug #49717
> +ALTER TABLE SourcePackageRelease ALTER component SET NOT NULL;
> +
> +-- We are taking OAuthNonce out of replication, so we make the foreign
> +-- key reference ON DELETE CASCADE so things don't explode when we
> +-- shuffle the lpmain master around.
> +ALTER TABLE OAuthNonce DROP CONSTRAINT oauthnonce__access_token__fk;
> +ALTER TABLE OAuthNonce ADD CONSTRAINT oauthnonce__access_token__fk
> +    FOREIGN KEY (access_token) REFERENCES OAuthAccessToken
> +    ON DELETE CASCADE;
> +
> +INSERT INTO LaunchpadDatabaseRevision VALUES (2207, 62, 0);

I think you added a missing index to BuildFarmJob.status last Friday too. Although it may not have helped the direct issue there, I think it should still be included. Do you?

https://irclogs.canonical.com/2010/06/04/%23launchpad-code.html#t10:37