launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #03882
[Merge] lp:~stub/launchpad/bug-794802-journal into lp:launchpad/db-devel
Stuart Bishop has proposed merging lp:~stub/launchpad/bug-794802-journal into lp:launchpad/db-devel with lp:~stub/launchpad/bug-794802 as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~stub/launchpad/bug-794802-journal/+merge/64051
= Summary =
Move to a journaled system where the journaled entries are rolled up out of band.
As of this writing, test_bugsummary.py is passing but bugsummary.txt seems to have triggered an error. This looks like it might be legitimate.
== Proposed fix ==
== Pre-implementation notes ==
== Implementation details ==
== Tests ==
== Demo and Q/A ==
= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
database/schema/patch-2208-63-1.sql
database/schema/patch-2208-63-4.sql
database/schema/patch-2208-75-0.sql
database/schema/patch-2208-63-2.sql
lib/lp/bugs/doc/bugsummary.txt
database/schema/security.cfg
database/schema/patch-2208-63-3.sql
lib/lp/bugs/model/bugsummary.py
lib/lp/registry/model/person.py
== Schema ==
./lib/lp/bugs/doc/bugsummary.txt
89: narrative has trailing whitespace.
254: 'ProductSeries' imported but unused
--
https://code.launchpad.net/~stub/launchpad/bug-794802-journal/+merge/64051
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stub/launchpad/bug-794802-journal into lp:launchpad/db-devel.
=== added file 'database/schema/patch-2208-63-3.sql'
--- database/schema/patch-2208-63-3.sql 1970-01-01 00:00:00 +0000
+++ database/schema/patch-2208-63-3.sql 2011-06-09 16:45:00 +0000
@@ -0,0 +1,55 @@
+-- Copyright 2011 Canonical Ltd. This software is licensed under the
+-- GNU Affero General Public License version 3 (see the file LICENSE).
+
+SET client_min_messages=ERROR;
+
+
+-- Create a journal for BugSummary updates.
+-- This is a separate DB patch as the table needs to be created and
+-- added to replication before triggers are created, and we want to
+-- do this live. We discussed not replicating this table, but this
+-- would break our ability to failover to a new master.
+
+CREATE TABLE BugSummaryJournal (
+ id serial PRIMARY KEY,
+ count INTEGER NOT NULL default 0,
+ product INTEGER REFERENCES Product ON DELETE CASCADE,
+ productseries INTEGER REFERENCES ProductSeries ON DELETE CASCADE,
+ distribution INTEGER REFERENCES Distribution ON DELETE CASCADE,
+ distroseries INTEGER REFERENCES DistroSeries ON DELETE CASCADE,
+ sourcepackagename INTEGER REFERENCES SourcePackageName ON DELETE CASCADE,
+ viewed_by INTEGER REFERENCES Person ON DELETE CASCADE,
+ tag TEXT,
+ status INTEGER NOT NULL,
+ milestone INTEGER REFERENCES Milestone ON DELETE CASCADE);
+
+-- Fat index for fast lookups
+CREATE INDEX bugsummaryjournal__full__idx ON BugSummaryJournal (
+ status, product, productseries, distribution, distroseries,
+ sourcepackagename, viewed_by, milestone, tag);
+
+-- Indexes for fast deletions.
+CREATE INDEX bugsummaryjournal__viewed_by__idx
+ ON BugSummaryJournal(viewed_by) WHERE viewed_by IS NOT NULL;
+CREATE INDEX bugsummaryjournal__milestone__idx
+ ON BugSummaryJournal(milestone) WHERE milestone IS NOT NULL;
+
+
+-- Combined view so we don't have to manually collate rows from both tables.
+-- Note that we flip the sign of the id column of BugSummaryJournal to avoid
+-- clashes. This is enough to keep Storm happy as it never needs to update
+-- this table, and there are no other suitable primary keys.
+-- We don't SUM() rows here to ensure PostgreSQL has the most hope of
+-- generating good query plans when we query this view.
+CREATE OR REPLACE VIEW CombinedBugSummary AS (
+ SELECT
+ id, count, product, productseries, distribution, distroseries,
+ sourcepackagename, viewed_by, tag, status, milestone
+ FROM BugSummary
+ UNION ALL
+ SELECT
+ -id as id, count, product, productseries, distribution, distroseries,
+ sourcepackagename, viewed_by, tag, status, milestone
+ FROM BugSummaryJournal);
+
+INSERT INTO LaunchpadDatabaseRevision VALUES (2208, 63, 3);
=== added file 'database/schema/patch-2208-63-4.sql'
--- database/schema/patch-2208-63-4.sql 1970-01-01 00:00:00 +0000
+++ database/schema/patch-2208-63-4.sql 2011-06-09 16:45:00 +0000
@@ -0,0 +1,280 @@
+-- Copyright 2011 Canonical Ltd. This software is licensed under the
+-- GNU Affero General Public License version 3 (see the file LICENSE).
+
+SET client_min_messages=ERROR;
+
+CREATE OR REPLACE FUNCTION bugsummary_journal_ins(d bugsummary)
+RETURNS VOID
+LANGUAGE plpgsql AS
+$$
+BEGIN
+ IF d.count <> 0 THEN
+ INSERT INTO BugSummaryJournal (
+ count, product, productseries, distribution,
+ distroseries, sourcepackagename, viewed_by, tag,
+ status, milestone)
+ VALUES (
+ d.count, d.product, d.productseries, d.distribution,
+ d.distroseries, d.sourcepackagename, d.viewed_by, d.tag,
+ d.status, d.milestone);
+ END IF;
+END;
+$$;
+
+COMMENT ON FUNCTION bugsummary_journal_ins(bugsummary) IS
+'Add an entry into BugSummaryJournal';
+
+
+CREATE OR REPLACE FUNCTION bugsummary_rollup_journal() RETURNS VOID
+LANGUAGE plpgsql VOLATILE
+SECURITY DEFINER SET search_path TO public AS
+$$
+DECLARE
+ d bugsummary%ROWTYPE;
+ max_id integer;
+BEGIN
+ -- Lock so we don't content with other invokations of this
+ -- function. We can happily lock the BugSummary table for writes
+ -- as this function is the only thing that updates that table.
+ -- BugSummaryJournal remains unlocked so nothing should be blocked.
+ LOCK TABLE BugSummary IN ROW EXCLUSIVE MODE;
+
+ SELECT MAX(id) INTO max_id FROM BugSummaryJournal;
+
+ FOR d IN
+ SELECT
+ NULL as id,
+ SUM(count),
+ product,
+ productseries,
+ distribution,
+ distroseries,
+ sourcepackagename,
+ viewed_by,
+ tag,
+ status,
+ milestone
+ FROM BugSummaryJournal
+ WHERE id <= max_id
+ GROUP BY
+ product, productseries, distribution, distroseries,
+ sourcepackagename, viewed_by, tag, status, milestone
+ HAVING sum(count) <> 0
+ LOOP
+ IF d.count < 0 THEN
+ PERFORM bug_summary_dec(d);
+ ELSIF d.count > 0 THEN
+ PERFORM bug_summary_inc(d);
+ END IF;
+ END LOOP;
+
+ DELETE FROM BugSummaryJournal WHERE id <= max_id;
+END;
+$$;
+
+COMMENT ON FUNCTION bugsummary_rollup_journal() IS
+'Collate and migrate rows from BugSummaryJournal to BugSummary';
+
+
+CREATE OR REPLACE FUNCTION unsummarise_bug(BUG_ROW bug) RETURNS VOID
+LANGUAGE plpgsql VOLATILE AS
+$$
+DECLARE
+ d bugsummary%ROWTYPE;
+BEGIN
+ FOR d IN SELECT * FROM bugsummary_locations(BUG_ROW) LOOP
+ d.count = -1;
+ PERFORM bugsummary_journal_ins(d);
+ END LOOP;
+END;
+$$;
+
+
+CREATE OR REPLACE FUNCTION summarise_bug(BUG_ROW bug) RETURNS VOID
+LANGUAGE plpgsql VOLATILE AS
+$$
+DECLARE
+ d bugsummary%ROWTYPE;
+BEGIN
+ FOR d IN SELECT * FROM bugsummary_locations(BUG_ROW) LOOP
+ d.count = 1;
+ PERFORM bugsummary_journal_ins(d);
+ END LOOP;
+END;
+$$;
+
+CREATE OR REPLACE FUNCTION bugsubscription_maintain_bug_summary()
+RETURNS TRIGGER LANGUAGE plpgsql VOLATILE
+SECURITY DEFINER SET search_path TO public AS
+$$
+BEGIN
+ -- This trigger only works if we are inserting, updating or deleting
+ -- a single row per statement.
+ IF TG_OP = 'INSERT' THEN
+ IF NOT (bug_row(NEW.bug)).private THEN
+ -- Public subscriptions are not aggregated.
+ RETURN NEW;
+ END IF;
+ IF TG_WHEN = 'BEFORE' THEN
+ PERFORM unsummarise_bug(bug_row(NEW.bug));
+ ELSE
+ PERFORM summarise_bug(bug_row(NEW.bug));
+ END IF;
+ RETURN NEW;
+ ELSIF TG_OP = 'DELETE' THEN
+ IF NOT (bug_row(OLD.bug)).private THEN
+ -- Public subscriptions are not aggregated.
+ RETURN OLD;
+ END IF;
+ IF TG_WHEN = 'BEFORE' THEN
+ PERFORM unsummarise_bug(bug_row(OLD.bug));
+ ELSE
+ PERFORM summarise_bug(bug_row(OLD.bug));
+ END IF;
+ RETURN OLD;
+ ELSE
+ IF (OLD.person IS DISTINCT FROM NEW.person
+ OR OLD.bug IS DISTINCT FROM NEW.bug) THEN
+ IF TG_WHEN = 'BEFORE' THEN
+ IF (bug_row(OLD.bug)).private THEN
+ -- Public subscriptions are not aggregated.
+ PERFORM unsummarise_bug(bug_row(OLD.bug));
+ END IF;
+ IF OLD.bug <> NEW.bug AND (bug_row(NEW.bug)).private THEN
+ -- Public subscriptions are not aggregated.
+ PERFORM unsummarise_bug(bug_row(NEW.bug));
+ END IF;
+ ELSE
+ IF (bug_row(OLD.bug)).private THEN
+ -- Public subscriptions are not aggregated.
+ PERFORM summarise_bug(bug_row(OLD.bug));
+ END IF;
+ IF OLD.bug <> NEW.bug AND (bug_row(NEW.bug)).private THEN
+ -- Public subscriptions are not aggregated.
+ PERFORM summarise_bug(bug_row(NEW.bug));
+ END IF;
+ END IF;
+ END IF;
+ RETURN NEW;
+ END IF;
+END;
+$$;
+
+
+CREATE OR REPLACE FUNCTION bugtag_maintain_bug_summary() RETURNS TRIGGER
+LANGUAGE plpgsql VOLATILE SECURITY DEFINER SET search_path TO public AS
+$$
+BEGIN
+ IF TG_OP = 'INSERT' THEN
+ IF TG_WHEN = 'BEFORE' THEN
+ PERFORM unsummarise_bug(bug_row(NEW.bug));
+ ELSE
+ PERFORM summarise_bug(bug_row(NEW.bug));
+ END IF;
+ RETURN NEW;
+ ELSIF TG_OP = 'DELETE' THEN
+ IF TG_WHEN = 'BEFORE' THEN
+ PERFORM unsummarise_bug(bug_row(OLD.bug));
+ ELSE
+ PERFORM summarise_bug(bug_row(OLD.bug));
+ END IF;
+ RETURN OLD;
+ ELSE
+ IF TG_WHEN = 'BEFORE' THEN
+ PERFORM unsummarise_bug(bug_row(OLD.bug));
+ IF OLD.bug <> NEW.bug THEN
+ PERFORM unsummarise_bug(bug_row(NEW.bug));
+ END IF;
+ ELSE
+ PERFORM summarise_bug(bug_row(OLD.bug));
+ IF OLD.bug <> NEW.bug THEN
+ PERFORM summarise_bug(bug_row(NEW.bug));
+ END IF;
+ END IF;
+ RETURN NEW;
+ END IF;
+END;
+$$;
+
+-- fixed to use the journal
+CREATE OR REPLACE FUNCTION bug_maintain_bug_summary() RETURNS TRIGGER
+LANGUAGE plpgsql VOLATILE SECURITY DEFINER SET search_path TO public AS
+$$
+BEGIN
+ -- There is no INSERT logic, as a bug will not have any summary
+ -- information until BugTask rows have been attached.
+ IF TG_OP = 'UPDATE' THEN
+ IF OLD.duplicateof IS DISTINCT FROM NEW.duplicateof
+ OR OLD.private IS DISTINCT FROM NEW.private THEN
+ PERFORM unsummarise_bug(OLD);
+ PERFORM summarise_bug(NEW);
+ END IF;
+
+ ELSIF TG_OP = 'DELETE' THEN
+ PERFORM unsummarise_bug(OLD);
+ END IF;
+
+ RETURN NULL; -- Ignored - this is an AFTER trigger
+END;
+$$;
+
+-- fixed to use the journal
+CREATE OR REPLACE FUNCTION bugtask_maintain_bug_summary() RETURNS TRIGGER
+LANGUAGE plpgsql VOLATILE SECURITY DEFINER SET search_path TO public AS
+$$
+BEGIN
+ -- This trigger only works if we are inserting, updating or deleting
+ -- a single row per statement.
+
+ -- Unlike bug_maintain_bug_summary, this trigger does not have access
+ -- to the old bug when invoked as an AFTER trigger. To work around this
+ -- we install this trigger as both a BEFORE and an AFTER trigger.
+ IF TG_OP = 'INSERT' THEN
+ IF TG_WHEN = 'BEFORE' THEN
+ PERFORM unsummarise_bug(bug_row(NEW.bug));
+ ELSE
+ PERFORM summarise_bug(bug_row(NEW.bug));
+ END IF;
+ RETURN NEW;
+
+ ELSIF TG_OP = 'DELETE' THEN
+ IF TG_WHEN = 'BEFORE' THEN
+ PERFORM unsummarise_bug(bug_row(OLD.bug));
+ ELSE
+ PERFORM summarise_bug(bug_row(OLD.bug));
+ END IF;
+ RETURN OLD;
+
+ ELSE
+ IF (OLD.product IS DISTINCT FROM NEW.product
+ OR OLD.productseries IS DISTINCT FROM NEW.productseries
+ OR OLD.distribution IS DISTINCT FROM NEW.distribution
+ OR OLD.distroseries IS DISTINCT FROM NEW.distroseries
+ OR OLD.sourcepackagename IS DISTINCT FROM NEW.sourcepackagename
+ OR OLD.status IS DISTINCT FROM NEW.status
+ OR OLD.milestone IS DISTINCT FROM NEW.milestone) THEN
+ IF TG_WHEN = 'BEFORE' THEN
+ PERFORM unsummarise_bug(bug_row(OLD.bug));
+ IF OLD.bug <> NEW.bug THEN
+ PERFORM unsummarise_bug(bug_row(NEW.bug));
+ END IF;
+ ELSE
+ PERFORM summarise_bug(bug_row(OLD.bug));
+ IF OLD.bug <> NEW.bug THEN
+ PERFORM summarise_bug(bug_row(NEW.bug));
+ END IF;
+ END IF;
+ END IF;
+ RETURN NEW;
+ END IF;
+END;
+$$;
+
+-- No longer needed - we have a persistent and replicated journal.
+DROP FUNCTION ensure_bugsummary_temp_journal();
+DROP FUNCTION bug_summary_flush_temp_journal();
+DROP FUNCTION bug_summary_temp_journal_dec(d bugsummary);
+DROP FUNCTION bug_summary_temp_journal_inc(d bugsummary);
+
+
+INSERT INTO LaunchpadDatabaseRevision VALUES (2208, 63, 4);
=== added file 'database/schema/patch-2208-75-0.sql'
--- database/schema/patch-2208-75-0.sql 1970-01-01 00:00:00 +0000
+++ database/schema/patch-2208-75-0.sql 2011-06-09 16:45:00 +0000
@@ -0,0 +1,11 @@
+-- Copyright 2011 Canonical Ltd. This software is licensed under the
+-- GNU Affero General Public License version 3 (see the file LICENSE).
+
+SET client_min_messages=ERROR;
+
+-- Cleanup of BugSummary deferred until next downtime window.
+
+ALTER TABLE BugSummary ADD CONSTRAINT bugsummary__viewed_by__fk
+ FOREIGN KEY (viewed_by) REFERENCES Person;
+
+INSERT INTO LaunchpadDatabaseRevision VALUES (2208, 75, 0);
=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg 2011-06-09 16:44:56 +0000
+++ database/schema/security.cfg 2011-06-09 16:45:00 +0000
@@ -89,11 +89,7 @@
public.bugsummary_tags(bug) =
public.unsummarise_bug(bug) =
public.bugsummary_viewers(bug) =
-public.bug_summary_temp_journal_inc(bugsummary) =
-public.ensure_bugsummary_temp_journal() =
-public.bug_summary_flush_temp_journal() =
-public.bug_summary_temp_journal_dec(bugsummary) =
-public.bug_summary_temp_journal_clean_row(bugsummary) =
+public.bugsummary_journal_ins(bugsummary) =
[ro]
groups=read
@@ -142,6 +138,8 @@
public.bugnotificationrecipient = SELECT, INSERT, UPDATE, DELETE
public.bugnotificationrecipientarchive = SELECT, UPDATE
public.bugsummary = SELECT
+public.bugsummaryjournal = SELECT
+public.bugsummary_rollup_journal() = EXECUTE
public.bugtag = SELECT, INSERT, DELETE
public.bugtrackercomponent = SELECT, INSERT, UPDATE
public.bugtrackercomponentgroup = SELECT, INSERT, UPDATE
@@ -156,6 +154,7 @@
public.codeimportresult = SELECT, INSERT, UPDATE, DELETE
public.codereviewmessage = SELECT, INSERT, DELETE
public.codereviewvote = SELECT, INSERT, UPDATE, DELETE
+public.combinedbugsummary = SELECT
public.commercialsubscription = SELECT, INSERT, UPDATE, DELETE
public.continent = SELECT
public.customlanguagecode = SELECT, INSERT, UPDATE, DELETE
@@ -2145,6 +2144,7 @@
public.bugsubscriptionfilterimportance = SELECT
public.bugsubscriptionfilterstatus = SELECT
public.bugsubscriptionfiltertag = SELECT
+public.bugsummary_rollup_journal() = EXECUTE
public.bugtag = SELECT
public.bugwatch = SELECT, UPDATE
public.bugwatchactivity = SELECT, DELETE
=== modified file 'lib/lp/bugs/doc/bugsummary.txt'
--- lib/lp/bugs/doc/bugsummary.txt 2011-06-03 10:44:36 +0000
+++ lib/lp/bugs/doc/bugsummary.txt 2011-06-09 16:45:00 +0000
@@ -44,6 +44,11 @@
... # bugs created in previous transactions.
... store.flush()
... store.invalidate()
+ ...
+ ... # And rollup the BugSummaryJournal into BugSummary
+ ... # so all the records are in one place.
+ ... store.execute("SELECT bugsummary_rollup_journal()")
+ ...
... # Make sure our results are in a consistent order.
... ordered_results = bugsummary_resultset.order_by(
... BugSummary.product_id, BugSummary.productseries_id,
=== modified file 'lib/lp/bugs/model/bugsummary.py'
--- lib/lp/bugs/model/bugsummary.py 2011-05-25 14:14:22 +0000
+++ lib/lp/bugs/model/bugsummary.py 2011-06-09 16:45:00 +0000
@@ -31,7 +31,7 @@
implements(IBugSummary)
- __storm_table__ = 'bugsummary'
+ __storm_table__ = 'combinedbugsummary'
id = Int(primary=True)
count = Int()
=== modified file 'lib/lp/registry/model/person.py'
--- lib/lp/registry/model/person.py 2011-06-03 10:38:25 +0000
+++ lib/lp/registry/model/person.py 2011-06-09 16:45:00 +0000
@@ -3958,6 +3958,9 @@
('votecast', 'person'),
('vote', 'person'),
('translationrelicensingagreement', 'person'),
+ # These are ON DELETE CASCADE and maintained by triggers.
+ ('bugsummary', 'viewed_by'),
+ ('bugsummaryjournal', 'viewed_by'),
]
references = list(postgresql.listReferences(cur, 'person', 'id'))
