From: Simon Riggs on
On Fri, 2010-07-16 at 20:45 -0400, Tom Lane wrote:
> Andres Freund <andres(a)anarazel.de> writes:
> > Just to help me: The primary reasons for using SnapshotNow is speed and in
> > some cases correctness (referential integrity). Right? Any other reasons?
>
> Well, the main point for system catalog accesses is that you *must* have
> an up-to-date view of the table schemas. As an example, if someone just
> added an index to an existing table, it would not do for an INSERT to
> fail to update that index --- no matter whether it's from a serializable
> transaction or not. So the DDL-executing transaction must hold a lock
> that would block any operation that had better be able to see what it
> did, and once another transaction has acquired the lock that lets it go
> ahead with another operation, it had better see the results of the DDL
> transaction.
>
> However that argument mostly applies to what the executor does. A plan
> could still be usable despite having been made against a now-obsolete
> version of the table schema.
>
> In the case at hand, I think most constraint-adding situations would
> require at least ShareLock, because they had better block execution of
> INSERT/UPDATE/DELETE operations that could fail to honor the constraint
> if they didn't see it in the catalogs. But AFAICS, addition of a
> constraint need not block SELECT, and it need not invalidate existing
> plans.
>
> CREATE INDEX uses ShareLock because it's okay to run multiple CREATE
> INDEXes in parallel (thanks to some rather dodgy coding of the catalog
> updates). For other cases of constraint additions, it might not be
> practical to run two constraint additions in parallel. In that case we
> could use ShareRowExclusive instead, which is self-exclusive but is not
> any stronger than Share from the perspective of DML commands. Since
> it's not, I'm unconvinced that it's worth taking any great pains to try
> to make constraint additions run in parallel.

The patch follows all of the above exactly.

--
Simon Riggs www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Training and Services


--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

From: Simon Riggs on
On Sun, 2010-07-18 at 17:28 +0200, Andres Freund wrote:

> Unfortunately the same issue exists with constraint exclusion - and we
> can hardly disable that for serializable transactions...

Then I think the fix is to look at the xmin values on all of the tables
used during planning and ensure that we only use constraint-based
optimisations in a serializable transaction when our top xmin is later
than the last DDL change (via its xmin).

--
Simon Riggs www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Training and Services


--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

From: Simon Riggs on
On Sun, 2010-07-18 at 22:47 -0400, Robert Haas wrote:
> But it seems
> that it's far from clear what to do about it, and it's not the job of
> this patch to fix it anyway.

Agreed.

> Regarding the actual patch, it looks mostly good. Questions:
>
> 1. Why in rewriteSupport.c are we adding a call to
> heap_inplace_update() in some situations? Doesn't seem like this is
> something we should need or want to be monkeying with.

Hmm, yes, that looks like a hangover. Will change. No others similar.

> 2. Instead of AlterTableGreatestLockLevel(), how about
> AlterTableGetLockLevel()? Yeah, it's going to be the highest lock
> level required by any subcommand, but it seems mildly overspecified.
> I don't feel strongly about this one, though, if someone has a strong
> contrary opinion...

I felt it indicated the process it's using. Happy to change.

--
Simon Riggs www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Training and Services


--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

From: Peter Eisentraut on
On tor, 2010-07-15 at 10:24 +0100, Simon Riggs wrote:
> Patch to reduce lock levels for
> ALTER TABLE
> CREATE TRIGGER
> CREATE RULE

Tried this out, but $subject is still the case. The problem is that
ATRewriteCatalogs() calls AlterTableCreateToastTable() based on what it
thinks the subcommands are, and AlterTableCreateToastTable() takes an
AccessExclusiveLock.

This could possibly be addressed by moving AT_SetStatistics to
AT_PASS_MISC in order to avoid the TOAST table call.

In a related matter, assigning ShareUpdateExclusiveLock to AT_SetStorage
doesn't work either, because the TOAST table call needs to be done in
that case.

Perhaps this logic needs to be refactored a bit more so that there
aren't any inconsistencies between the lock modes and the "passes",
which could lead to false expectations and deadlocks.


--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

From: Simon Riggs on
On Wed, 2010-07-28 at 15:24 +0300, Peter Eisentraut wrote:
> On tor, 2010-07-15 at 10:24 +0100, Simon Riggs wrote:
> > Patch to reduce lock levels for
> > ALTER TABLE
> > CREATE TRIGGER
> > CREATE RULE
>
> Tried this out, but $subject is still the case. The problem is that
> ATRewriteCatalogs() calls AlterTableCreateToastTable() based on what it
> thinks the subcommands are, and AlterTableCreateToastTable() takes an
> AccessExclusiveLock.
>
> This could possibly be addressed by moving AT_SetStatistics to
> AT_PASS_MISC in order to avoid the TOAST table call.
>
> In a related matter, assigning ShareUpdateExclusiveLock to AT_SetStorage
> doesn't work either, because the TOAST table call needs to be done in
> that case.
>
> Perhaps this logic needs to be refactored a bit more so that there
> aren't any inconsistencies between the lock modes and the "passes",
> which could lead to false expectations and deadlocks.

Thanks for your comments. Will reconsider and update.

--
Simon Riggs www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Training and Services


--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers