Launchpad itself

Merge lp:~stub/launchpad/pending-db-changes into lp:launchpad/db-devel

  1. pending-db-changes
  2. Merge into db-devel
Proposed by Stuart Bishop
Status: Merged
Merged at revision: 9445
Proposed branch: lp:~stub/launchpad/pending-db-changes
Merge into: lp:launchpad/db-devel
Diff against target: 127 lines (+35/-17)
4 files modified
database/replication/helpers.py (+10/-4)
database/schema/fti.py (+10/-13)
database/schema/patch-2207-62-0.sql (+14/-0)
database/schema/security.cfg (+1/-0)
To merge this branch: bzr merge lp:~stub/launchpad/pending-db-changes
Reviewer Review Type Date Requested Status
Michael Nelson (community) code Approve
Review via email: mp+27263@code.launchpad.net

Commit message

SourcePackageRelease.component NOT NULL, omit OAuthNonce from replication per current production setup, Py 2.6 deprecation fix, nagios perm

Description of the change

Collection of minor DB tweaks.

We have stopped replicating the OAuthNonce table, and tweaks to our replication maintenance scripts to stop it being added back.

fti.py was spitting a deprecation warning under Python 2.6 - fix that.

The Bug #49717 DB patch I though had been landed had not been landed. Extend this with a tweak for the OAuthNonce table, so things still cope now it is no longer replicated.

A DB permission for our nagios checks.

To post a comment you must log in.
Revision history for this message
Michael Nelson (michael.nelson) wrote :
Download full text (3.9 KiB)

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 SourcePackageRele...

Read more...

review: Approve (code)
Revision history for this message
Stuart Bishop (stub) wrote :

On Thu, Jun 10, 2010 at 8:53 PM, Michael Nelson
<email address hidden> wrote:

>> === 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?

Bits could. I tend to consider staging the test environment for most
of this though, and haven't worried about the few bits that are
testable. Its all a bit ugly, but now we know it is all working I
guess we can start cleaning things up.

>>      # 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?

That would give an index exception, as there is only a single column
being returned.

>> +        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?

No. It is None because I specified stderr=subprocess.STDOUT to munge
stdout and stderr together.

> 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?

That has already landed as patch-2207-60-1.sql. Its in a different db
patch as it has already been applied to production.

--
Stuart Bishop <email address hidden>
http://www.stuartbishop.net/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'database/replication/helpers.py'
--- database/replication/helpers.py 2010-04-29 12:38:05 +0000
+++ database/replication/helpers.py 2010-06-10 12:51:38 +0000
@@ -44,7 +44,6 @@
44 ('public', 'nameblacklist'),44 ('public', 'nameblacklist'),
45 ('public', 'openidconsumerassociation'),45 ('public', 'openidconsumerassociation'),
46 ('public', 'openidconsumernonce'),46 ('public', 'openidconsumernonce'),
47 ('public', 'oauthnonce'),
48 ('public', 'codeimportmachine'),47 ('public', 'codeimportmachine'),
49 ('public', 'scriptactivity'),48 ('public', 'scriptactivity'),
50 ('public', 'standardshipitrequest'),49 ('public', 'standardshipitrequest'),
@@ -71,6 +70,8 @@
71 # Database statistics70 # Database statistics
72 'public.databasetablestats',71 'public.databasetablestats',
73 'public.databasecpustats',72 'public.databasecpustats',
73 # Don't replicate OAuthNonce - too busy and no real gain.
74 'public.oauthnonce',
74 # Ubuntu SSO database. These tables where created manually by ISD75 # Ubuntu SSO database. These tables where created manually by ISD
75 # and the Launchpad scripts should not mess with them. Eventually76 # and the Launchpad scripts should not mess with them. Eventually
76 # these tables will be in a totally separate database.77 # these tables will be in a totally separate database.
@@ -353,6 +354,9 @@
353354
354 A replication set must contain all tables linked by foreign key355 A replication set must contain all tables linked by foreign key
355 reference to the given table, and sequences used to generate keys.356 reference to the given table, and sequences used to generate keys.
357 Tables and sequences can be added to the IGNORED_TABLES and
358 IGNORED_SEQUENCES lists for cases where we known can safely ignore
359 this restriction.
356360
357 :param seeds: [(namespace, tablename), ...]361 :param seeds: [(namespace, tablename), ...]
358362
@@ -420,7 +424,8 @@
420 """ % sqlvalues(namespace, tablename))424 """ % sqlvalues(namespace, tablename))
421 for namespace, tablename in cur.fetchall():425 for namespace, tablename in cur.fetchall():
422 key = (namespace, tablename)426 key = (namespace, tablename)
423 if key not in tables and key not in pending_tables:427 if (key not in tables and key not in pending_tables
428 and '%s.%s' % (namespace, tablename) not in IGNORED_TABLES):
424 pending_tables.add(key)429 pending_tables.add(key)
425430
426 # Generate the set of sequences that are linked to any of our set of431 # Generate the set of sequences that are linked to any of our set of
@@ -441,8 +446,9 @@
441 ) AS whatever446 ) AS whatever
442 WHERE seq IS NOT NULL;447 WHERE seq IS NOT NULL;
443 """ % sqlvalues(fqn(namespace, tablename), namespace, tablename))448 """ % sqlvalues(fqn(namespace, tablename), namespace, tablename))
444 for row in cur.fetchall():449 for sequence, in cur.fetchall():
445 sequences.add(row[0])450 if sequence not in IGNORED_SEQUENCES:
451 sequences.add(sequence)
446452
447 # We can't easily convert the sequence name to (namespace, name) tuples,453 # We can't easily convert the sequence name to (namespace, name) tuples,
448 # so we might as well convert the tables to dot notation for consistancy.454 # so we might as well convert the tables to dot notation for consistancy.
449455
=== modified file 'database/schema/fti.py'
--- database/schema/fti.py 2010-05-19 18:07:56 +0000
+++ database/schema/fti.py 2010-06-10 12:51:38 +0000
@@ -14,10 +14,10 @@
14import _pythonpath14import _pythonpath
1515
16from distutils.version import LooseVersion16from distutils.version import LooseVersion
17import sys
18import os.path17import os.path
19from optparse import OptionParser18from optparse import OptionParser
20import popen219import subprocess
20import sys
21from tempfile import NamedTemporaryFile21from tempfile import NamedTemporaryFile
22from textwrap import dedent22from textwrap import dedent
23import time23import time
@@ -319,18 +319,15 @@
319 cmd += ' -h %s' % lp.dbhost319 cmd += ' -h %s' % lp.dbhost
320 if options.dbuser:320 if options.dbuser:
321 cmd += ' -U %s' % options.dbuser321 cmd += ' -U %s' % options.dbuser
322 p = popen2.Popen4(cmd)322 p = subprocess.Popen(
323 c = p.tochild323 cmd.split(' '), stdin=subprocess.PIPE,
324 print >> c, "SET client_min_messages=ERROR;"324 stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
325 print >> c, "CREATE SCHEMA ts2;"325 out, err = p.communicate(
326 print >> c, open(tsearch2_sql_path).read().replace(326 "SET client_min_messages=ERROR; CREATE SCHEMA ts2;"
327 'public;','ts2, public;'327 + open(tsearch2_sql_path).read().replace('public;','ts2, public;'))
328 )328 if p.returncode != 0:
329 p.tochild.close()
330 rv = p.wait()
331 if rv != 0:
332 log.fatal('Error executing %s:', cmd)329 log.fatal('Error executing %s:', cmd)
333 log.debug(p.fromchild.read())330 log.debug(out)
334 sys.exit(rv)331 sys.exit(rv)
335332
336 # Create ftq helper and its sibling _ftq.333 # Create ftq helper and its sibling _ftq.
337334
=== 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 12:51:38 +0000
@@ -0,0 +1,14 @@
1SET client_min_messages=ERROR;
2
3-- Bug #49717
4ALTER TABLE SourcePackageRelease ALTER component SET NOT NULL;
5
6-- We are taking OAuthNonce out of replication, so we make the foreign
7-- key reference ON DELETE CASCADE so things don't explode when we
8-- shuffle the lpmain master around.
9ALTER TABLE OAuthNonce DROP CONSTRAINT oauthnonce__access_token__fk;
10ALTER TABLE OAuthNonce ADD CONSTRAINT oauthnonce__access_token__fk
11 FOREIGN KEY (access_token) REFERENCES OAuthAccessToken
12 ON DELETE CASCADE;
13
14INSERT INTO LaunchpadDatabaseRevision VALUES (2207, 62, 0);
015
=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg 2010-06-07 10:24:03 +0000
+++ database/schema/security.cfg 2010-06-10 12:51:38 +0000
@@ -1826,6 +1826,7 @@
1826type=user1826type=user
1827public.archive = SELECT1827public.archive = SELECT
1828public.buildfarmjob = SELECT1828public.buildfarmjob = SELECT
1829public.databasereplicationlag = SELECT
1829public.packagebuild = SELECT1830public.packagebuild = SELECT
1830public.binarypackagebuild = SELECT1831public.binarypackagebuild = SELECT
1831public.buildqueue = SELECT1832public.buildqueue = SELECT

Subscribers

People subscribed via source and target branches

to status/vote changes: