Merge lp:~edwin-grubbs/launchpad/bug-535430-needspackaging-timeout-part1 into lp:launchpad/db-devel

Proposed by Edwin Grubbs
Status: Merged
Merged at revision: 9449
Proposed branch: lp:~edwin-grubbs/launchpad/bug-535430-needspackaging-timeout-part1
Merge into: lp:launchpad/db-devel
Diff against target: 45 lines (+29/-1)
2 files modified
database/schema/comments.sql (+5/-1)
database/schema/patch-2207-56-0.sql (+24/-0)
To merge this branch: bzr merge lp:~edwin-grubbs/launchpad/bug-535430-needspackaging-timeout-part1
Reviewer Review Type Date Requested Status
Stuart Bishop (community) db Approve
Björn Tillenius (community) db Approve
Review via email: mp+25359@code.launchpad.net

Description of the change

This branch adds the following columns the DistributionSourcePackage table:
    total_bug_heat INTEGER
    bug_count INTEGER
    po_message_count INTEGER
    section INTEGER REFERENCES Section(id)

This table will be used to cache data so the $distroseries/+needs-packaging
page won't timeout. A DistributionSourcePackage row is only needed for
packages with a SourcePackagePublishingHistory record.

The section will be updated by the trigger, since it only needs to be
changed when a new SourcePackagePublishingHistory record is added. The
other three columns will be populated by a cronjob.

The total_bug_heat differs from the existing max_bug_heat column in that
the max_bug_heat column is the highest heat value for a single bug related
to the package, and the total_bug_heat is the sum of all the bugs related
to the package.

To post a comment you must log in.
Revision history for this message
Edwin Grubbs (edwin-grubbs) wrote :

After talking with Julian, I have removed the trigger. Updating the DistributionSourcePackage table when a new SourcePackagePublishingHistory record is added will instead be handled in lp.soyuz.model.publishing.PublishingSet.newSourcePublication().

Revision history for this message
Björn Tillenius (bjornt) wrote :

Ok, removing the trigger makes it easier to keep things sane. How do you intend to keep the other values up-to-date? Maybe it makes sense to rename them, so that it's clear that they are caches only, and might not be correct? Unless you find a way of making sure they are up to date, of course. Also, for the bug values, will it include private bugs as well?

The comments should be added to comments.sql.

review: Needs Information
Revision history for this message
Edwin Grubbs (edwin-grubbs) wrote :

> Ok, removing the trigger makes it easier to keep things sane. How do you
> intend to keep the other values up-to-date?

The other values will be kept up-to-date with a cronjob.

> Maybe it makes sense to rename
> them, so that it's clear that they are caches only, and might not be correct?
> Unless you find a way of making sure they are up to date, of course.

Although I was originally thinking of updating the values with a cronjob, the max_bug_heat is currently updated with a hook in addTask(), so it would be easy to add that. I assume that I could do something similar for po_message_count.

> Also, for
> the bug values, will it include private bugs as well?

Yes.

> The comments should be added to comments.sql.

The comments have been moved.

Revision history for this message
Björn Tillenius (bjornt) wrote :

< BjornT> EdwinGrubbs: well, i guess the db patch is fine. you should.
          have a talk with deryck, as to whether include private bugs in.
          the count, since that might not be desired. also talk to him.
          about how to keep the bug heat values up-to-date
< BjornT> EdwinGrubbs: doing it in addTask() only won't be enough

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

I assume NULL values mean 'has not been calculated yet'. This should be documented in comments.sql.

If we need to retrieve or order rows by bug_count or po_message_count we will need an index on those columns.

Otherwise all looks fine. patch-2207-56-0.sql

review: Approve (db)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'database/schema/comments.sql'
2--- database/schema/comments.sql 2010-06-03 22:21:24 +0000
3+++ database/schema/comments.sql 2010-06-04 17:54:29 +0000
4@@ -472,7 +472,11 @@
5
6 COMMENT ON TABLE DistributionSourcePackage IS 'Representing a sourcepackage in a distribution across all distribution series.';
7 COMMENT ON COLUMN DistributionSourcePackage.bug_reporting_guidelines IS 'Guidelines to the end user for reporting bugs on a particular a source package in a distribution.';
8-COMMENT ON COLUMN DistributionSourcePackage.max_bug_heat IS 'The highest heat value across bugs for this source package.';
9+COMMENT ON COLUMN DistributionSourcePackage.max_bug_heat IS 'The highest heat value across bugs for this source package. NULL means it has not yet been calculated.';
10+COMMENT ON COLUMN DistributionSourcePackage.total_bug_heat IS 'Sum of bug heat matching the package distribution and sourcepackagename. NULL means it has not yet been calculated.';
11+COMMENT ON COLUMN DistributionSourcePackage.bug_count IS 'Number of bugs matching the package distribution and sourcepackagename. NULL means it has not yet been calculated.';
12+COMMENT ON COLUMN DistributionSourcePackage.po_message_count IS 'Number of translations matching the package distribution and sourcepackagename. NULL means it has not yet been calculated.';
13+COMMENT ON COLUMN DistributionSourcePackage.section IS 'Cached section matching the latest SourcePackagePublishingHistory record by distribution and sourcepackagename whose archive purpose is PRIMARY and whose distroseries releasestatus is CURRENT.';
14 COMMENT ON COLUMN DistributionSourcePackage.bug_reported_acknowledgement IS 'A message of acknowledgement to display to a bug reporter after they\'ve reported a new bug.';
15
16 -- DistributionSourcePackageCache
17
18=== added file 'database/schema/patch-2207-56-0.sql'
19--- database/schema/patch-2207-56-0.sql 1970-01-01 00:00:00 +0000
20+++ database/schema/patch-2207-56-0.sql 2010-06-04 17:54:29 +0000
21@@ -0,0 +1,24 @@
22+-- Copyright 2009 Canonical Ltd. This software is licensed under the
23+-- GNU Affero General Public License version 3 (see the file LICENSE).
24+
25+SET client_min_messages=ERROR;
26+
27+INSERT INTO LaunchpadDatabaseRevision VALUES (2207, 56, 0);
28+
29+/*
30+Existing Schema:
31+
32+CREATE TABLE distributionsourcepackage (
33+ id integer NOT NULL,
34+ distribution integer NOT NULL,
35+ sourcepackagename integer NOT NULL,
36+ bug_reporting_guidelines text,
37+ max_bug_heat integer
38+);
39+*/
40+
41+ALTER TABLE DistributionSourcePackage ADD COLUMN total_bug_heat INTEGER;
42+ALTER TABLE DistributionSourcePackage ADD COLUMN bug_count INTEGER;
43+ALTER TABLE DistributionSourcePackage ADD COLUMN po_message_count INTEGER;
44+ALTER TABLE DistributionSourcePackage
45+ ADD COLUMN section INTEGER NOT NULL REFERENCES section(id);

Subscribers

People subscribed via source and target branches

to status/vote changes: