tablecmds.c and lock hierarchy

From: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
To: PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org>
Subject: tablecmds.c and lock hierarchy
Date: 2015-08-04 04:56:56
Message-ID: CAB7nPqQPMRkN1O6wHEAaqzDTXhqK6-Fpk9vH7=Tx-iXgKXizXA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi all,

As mentioned in the thread related to lowering locks of autovacuum
reloptions in ALTER TABLE SET
(http://www.postgresql.org/message-id/CAFcNs+oX7jVENC_3i54fDQ3ibmOGmknc2tMevdSmvojbSXGbGg@mail.gmail.com),
I have noticed the following code in
AlterTableGetLockLevel(at)tablecmds(dot)c:
/*
* Take the greatest lockmode from any subcommand
*/
if (cmd_lockmode > lockmode)
lockmode = cmd_lockmode;

The thing is that, as mentioned by Alvaro and Andres on this thread,
we have no guarantee that the different relation locks compared have a
monotone hierarchy and we may finish by taking a lock that does not
behave as you would like to. We are now lucky enough that ALTER TABLE
only uses ShareUpdateExclusiveLock, ShareRowExclusiveLock and
AccessExclusiveLock that actually have a hierarchy so this is not a
problem yet.
However it may become a problem if we add in the future more lock
modes and that are used by ALTER TABLE.

One idea mentioned in the ALTER TABLE SET thread was to enforce the
lock to AccessExclusiveLock should two locks of any subcommands
differ. Another idea was to select all the locks of the subcommands
found to be present, and open the different relations with all of
them. Tracking all those locks is going to require some heavy
rewriting, like for example the use of a LOCKMASK to track the locks
that need to be taken, and more extended routines for relation_open.
Other possibilities may be to add a comment on top of
AlterTableGetLockLevel so as we do not use a lock in ALTER TABLE
commands that may result in a choice conflict with the other ones, to
simply do nothing because committers are very careful people usually,
or to track all the locks taken in AlterTableGetLockLevel and then run
DoLockModesConflict and ERROR if there are incompatible locks.

I am attaching a patch that implements the first approach mentioned to
give a short idea of how things could be improved.
Thoughts?
--
Michael

Attachment Content-Type Size
20150804_altertable_locks.patch application/x-patch 1.6 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jeff Janes 2015-08-04 05:03:00 FSM versus GIN pending list bloat
Previous Message David Rowley 2015-08-04 04:55:00 cost_agg() with AGG_HASHED does not account for startup costs