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_3i54fdq3ibmogmknc2tmevdsmvojbsxg...@mail.gmail.com),
I have noticed the following code in
AlterTableGetLockLevel@tablecmds.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
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 970abd4..0bcdd76 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -2807,7 +2807,8 @@ AlterTableInternal(Oid relid, List *cmds, bool recurse)
* Function is called before and after parsing, so it must give same
* answer each time it is called. Some subcommands are transformed
* into other subcommand types, so the transform must never be made to a
- * lower lock level than previously assigned. All transforms are noted below.
+ * lock level different than previously assigned. All transforms are noted
+ * below.
*
* Since this is called before we lock the table we cannot use table metadata
* to influence the type of lock we acquire.
@@ -2837,6 +2838,7 @@ AlterTableGetLockLevel(List *cmds)
*/
ListCell *lcmd;
LOCKMODE lockmode = ShareUpdateExclusiveLock;
+ bool is_init = false;
foreach(lcmd, cmds)
{
@@ -3057,10 +3059,22 @@ AlterTableGetLockLevel(List *cmds)
}
/*
- * Take the greatest lockmode from any subcommand
+ * At the first command evaluated, take the lock corresponding to
+ * it. When looking at the next commands, check if the lock taken
+ * is different than the first one taken and enforce it to
+ * AccessExclusiveLock, which is the only lock super-seeding all
+ * the others.
*/
- if (cmd_lockmode > lockmode)
+ if (!is_init)
+ {
lockmode = cmd_lockmode;
+ is_init = true;
+ }
+ else
+ {
+ if (cmd_lockmode != lockmode)
+ lockmode = AccessExclusiveLock;
+ }
}
return lockmode;
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
