PATCH: Reducing lock strength of trigger and foreign key DDL

From: Andreas Karlsson <andreas(at)proxel(dot)se>
To: Noah Misch <noah(at)leadboat(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: PATCH: Reducing lock strength of trigger and foreign key DDL
Date: 2014-12-13 20:45:54
Message-ID: 548CA582.5010801@proxel.se
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

I have attached a patch with the current status of my work on reducing
the lock level of trigger and foreign key related DDL.

This commit reduces the lock level of the following commands from ACCESS
EXCLUSIVE to SHARE ROW EXCLUSIVE, plus that it does the same for the
referred table of the foreign key.

ADD TRIGGER
ALTER TRIGGER
ALTER TABLE ... ADD FOREIGN KEY
ALTER TABLE ... DISABLE TRIGGER
ALTER TABLE ... ENABLE TRIGGER

The patch does currently not reducing the lock level of the following
two commands, but I think it would be really nice to fix those too and
here I would like some feedback and ideas.

DROP TRIGGER
ALTER TABLE ... DROP CONSTRAINT -- For foreign keys

Foreign keys and triggers are fixed at the same time because foreign
keys are implemented with two triggers, one at each of the involved tables.

The idea behind the patch is that since we started using MVCC snapshots
we no longer need to hold an exclusive lock on the table when adding a
trigger. Theoretically we only need to lock out writers of the rows of
the table (SHARE ROW EXCLUSIVE/SHARE), but as noted by Robert and Noah
just reducing the lock level will break pg_dump since
pg_get_triggerdef() does not use the current snapshot when reading the
catalog.

I have fixed pg_get_triggerdef() to use the snapshot like how
e5550d5fec66aa74caad1f79b79826ec64898688 fixed pg_get_constraintdef(),
and this fixes the code for the ALTER TRIGGER case (ADD TRIGGER was
already safe). But to fix it for the DROP TRIGGER case we need to also
make the dumping of the WHEN clause (which is dumped by get_rule_expr())
use the current snapshot.

get_rule_expr() relies heavily on the catcache which to me does not look
like it could easily be (and probably not even should be) made to use
the current snapshot. Refactoring ruleutils.c to rely less no the
catcache seems like a reasonable thing to do if we want to reduce
weirdness of how it ignores MVCC but it is quite a bit of work and I
fear it could give us performance regressions.

Do you have any ideas for how to fix get_rule_expr()? Is this patch
worthwhile even without reducing the lock levels of the drop commands?

Andreas

Attachment Content-Type Size
add-fk-lock-strength-v2.patch text/x-patch 7.5 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message David Rowley 2014-12-13 22:10:44 Re: pgbench -f and vacuum
Previous Message Simon Riggs 2014-12-13 20:45:22 Re: [REVIEW] Re: Compression of full-page-writes