pgsql: Improve table locking behavior in the face of current DDL.

From: Robert Haas <rhaas(at)postgresql(dot)org>
To: pgsql-committers(at)postgresql(dot)org
Subject: pgsql: Improve table locking behavior in the face of current DDL.
Date: 2011-11-30 15:27:34
Message-ID: E1RVm4M-0002jO-EX@gemulon.postgresql.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers

Improve table locking behavior in the face of current DDL.

In the previous coding, callers were faced with an awkward choice:
look up the name, do permissions checks, and then lock the table; or
look up the name, lock the table, and then do permissions checks.
The first choice was wrong because the results of the name lookup
and permissions checks might be out-of-date by the time the table
lock was acquired, while the second allowed a user with no privileges
to interfere with access to a table by users who do have privileges
(e.g. if a malicious backend queues up for an AccessExclusiveLock on
a table on which AccessShareLock is already held, further attempts
to access the table will be blocked until the AccessExclusiveLock
is obtained and the malicious backend's transaction rolls back).

To fix, allow callers of RangeVarGetRelid() to pass a callback which
gets executed after performing the name lookup but before acquiring
the relation lock. If the name lookup is retried (because
invalidation messages are received), the callback will be re-executed
as well, so we get the best of both worlds. RangeVarGetRelid() is
renamed to RangeVarGetRelidExtended(); callers not wishing to supply
a callback can continue to invoke it as RangeVarGetRelid(), which is
now a macro. Since the only one caller that uses nowait = true now
passes a callback anyway, the RangeVarGetRelid() macro defaults nowait
as well. The callback can also be used for supplemental locking - for
example, REINDEX INDEX needs to acquire the table lock before the index
lock to reduce deadlock possibilities.

There's a lot more work to be done here to fix all the cases where this
can be a problem, but this commit provides the general infrastructure
and fixes the following specific cases: REINDEX INDEX, REINDEX TABLE,
LOCK TABLE, and and DROP TABLE/INDEX/SEQUENCE/VIEW/FOREIGN TABLE.

Per discussion with Noah Misch and Alvaro Herrera.

Branch
------
master

Details
-------
http://git.postgresql.org/pg/commitdiff/2ad36c4e44c8b513f6155656e1b7a8d26715bb94

Modified Files
--------------
src/backend/access/heap/heapam.c | 4 +-
src/backend/catalog/aclchk.c | 2 +-
src/backend/catalog/index.c | 13 ++-
src/backend/catalog/namespace.c | 21 ++++-
src/backend/commands/alter.c | 2 +-
src/backend/commands/indexcmds.c | 136 ++++++++++++++++++++--------
src/backend/commands/lockcmds.c | 172 ++++++++++++++++++++---------------
src/backend/commands/sequence.c | 4 +-
src/backend/commands/tablecmds.c | 141 +++++++++++++++++-----------
src/backend/commands/trigger.c | 3 +-
src/backend/commands/vacuum.c | 2 +-
src/backend/parser/parse_relation.c | 2 +-
src/backend/parser/parse_type.c | 2 +-
src/backend/rewrite/rewriteDefine.c | 3 +-
src/backend/tcop/utility.c | 2 +-
src/backend/utils/adt/acl.c | 2 +-
src/backend/utils/adt/regproc.c | 5 +-
src/backend/utils/adt/ruleutils.c | 6 +-
src/include/catalog/index.h | 1 +
src/include/catalog/namespace.h | 11 ++-
src/pl/plpgsql/src/pl_comp.c | 4 +-
21 files changed, 339 insertions(+), 199 deletions(-)

Browse pgsql-committers by date

  From Date Subject
Next Message Tom Lane 2011-11-30 16:50:38 pgsql: Update time zone data files to tzdata release 2011n.
Previous Message User Fxjr 2011-11-30 14:19:08 npgsql - Npgsql2: Removed assembly info creation from build process.