Reducing some DDL Locks to ShareLock - Mailing list pgsql-hackers
From | Simon Riggs |
---|---|
Subject | Reducing some DDL Locks to ShareLock |
Date | |
Msg-id | 1722552592.7.1224882093461.JavaMail.root@spotone Whole thread Raw |
List | pgsql-hackers |
It seems possible to change some DDL commands/subcommands to use a ShareLock rather than an AccessExclusiveLock. Enclosed patch implements this reduction for CREATE TRIGGER, CREATE RULE and ALTER TABLE. We can relax locking as long as we can verify that the changes effect DML statements *only*. I've marked the commands/subcommands this can be applied to with an "s" (for ShareLock) and noted others that must remain "x" (for AccessExclusiveLock). s CREATE RULE (only non-ON SELECT rules) s CREATE TRIGGER ALTER TABLE x RENAME [ COLUMN ] column TO new_column x RENAME TO new_name x SET SCHEMA new_schema x ADD [ COLUMN ] column type [ column_constraint [ ... ] ] x DROP [ COLUMN ] column [ RESTRICT | CASCADE ] x ALTER [ COLUMN ] column TYPE type [ USING expression ] x ALTER [ COLUMN ] column SET DEFAULT expression x ALTER [ COLUMN ] column DROP DEFAULT s ALTER [ COLUMN ] column SET NOT NULL x ALTER [ COLUMN ] column DROP NOT NULL s ALTER [ COLUMN ] column SET STATISTICS integer s ALTER [ COLUMN ] column SET STORAGE { PLAIN | EXTERNAL | EXTENDED | MAIN } s ADD table_constraint x DROP CONSTRAINT constraint_name [ RESTRICT | CASCADE ] s DISABLE TRIGGER [ trigger_name | ALL | USER ] s ENABLE TRIGGER [ trigger_name | ALL | USER ] s ENABLE REPLICA TRIGGER trigger_name s ENABLE ALWAYS TRIGGER trigger_name x DISABLE RULE rewrite_rule_name x ENABLE RULE rewrite_rule_name x ENABLE REPLICA RULE rewrite_rule_name x ENABLE ALWAYS RULE rewrite_rule_name s CLUSTER ON index_name s SET WITHOUT CLUSTER x SET WITHOUT OIDS s SET ( storage_parameter = value [, ... ] ) s RESET ( storage_parameter [, ... ] ) x INHERIT parent_table x NO INHERIT parent_table x OWNER TO new_owner x SET TABLESPACE new_tablespace If an ALTER TABLE has more than one sub-command we take the highest lock to cover the execution of all sub-commands, since we want to avoid lock upgrades and deadlocks. e.g. ALTER TABLE foo ADD PRIMARY KEY (col1); will take ShareLock whereas ALTER TABLE foo ADD PRIMARY KEY (col1), INHERIT bar; will require an AccessExclusiveLock I've checked definitions of these subcommands here http://developer.postgresql.org/pgdocs/postgres/sql-altertable.html As noted, Rules can be ON SELECT, so we can't enable/disable them without an AccessExclusiveLock. Other than that, many of the subcommands don't change the actual table data, they just change settings for future DML changes to the table. This will * allow ALTER TABLE ADD FOREIGN KEY to run in parallel with pg_restore * allow trigger-based replication systems to add triggers more easily * reduce the number of AccessExclusiveLocks that have to be dealt with in Hot Standby mode. Couple of points of note: * Patch implements this by changing pg_class.reltriggers so it only has two values, 0 or 1. Do we want to change this to a boolean, or leave as an int2 for backwards compatibility? Code works either way. Enclosed patch keeps it as an int2 for now, easily changed on request. * transformIndexStatement() and DefineIndex() both take the wrong level of locks in existing code, when called from ALTER TABLE. Not sure whether this should be fixed in conjunction with these changes or not. Doesn't seem to make any difference, since the "wrong level" is taken after the initial lock is taken, so we never actually notice. * ATRewriteTables() uses the wrong lock level in existing code when we call validateForeignKeyConstraint(). It uses RowShareLock(), which is not sufficient to stop DML when checking using the SELECT in RI_Initial_Check(). Again, at present we take an AccessExclusiveLock first, so not a problem now. Changed to ShareLock in patch. Patch passes make check on an assert build, plus manual observation of lock order and locks held. I've also introduced two new lock functions LockHeldByMe() and RelationLockedByMe() to allow Assert() checking of lock levels held. These are added in key points in ALTER TABLE code to ensure no mistakes are made about required lock levels. doc/src/sgml/mvcc.sgml | 17 !! src/backend/commands/tablecmds.c | 278 ++++++!!!!!!!!!!!!!!!!!!!!! src/backend/commands/trigger.c | 88 -!!!!!!!!!! src/backend/parser/parse_utilcmd.c | 21 !! src/backend/rewrite/rewriteDefine.c | 10 ! src/backend/storage/lmgr/lmgr.c | 12 + src/backend/storage/lmgr/lock.c | 30 +++ src/backend/utils/adt/ri_triggers.c | 2 src/include/commands/tablecmds.h | 2 src/include/storage/lmgr.h | 2 src/include/storage/lock.h | 1 11 files changed, 107 insertions(+), 11 deletions(-), 345 mods(!) Patch is overall fairly straightforward: * Assess required lock level * Pass down decision through APIs to allow recursing to inherited tables * Change comments at various places (or not) * Trigger code needs to work around no knowing how many triggers are present, so relcache now handled similarly to Rules Patch correctly handles lock levels in cases such a reindex of indexes following ALTER TYPE of an indexed column. It might appear from the code that this had been missed, but tracing code shows it is correctly set. There is still scope for an "ALTER TABLE CONCURRENTLY" command as has been suggested. This proposal neither blocks that nor is blocked by it. Not every application can be changed to accept new syntax, nor is it desirable for every application to issue these commands only in top-level transactions. There are probably even more tweaks that we could do than the above list, but the above seems fairly straightforward for now. We can always do more later. -- Simon Riggs www.2ndQuadrant.com PostgreSQL Training, Services and Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Attachment
pgsql-hackers by date: