From 1c0e678678a661f8437ed238a5c515b81547aaf4 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Fri, 4 May 2012 17:43:35 -0400 Subject: [PATCH] Overdue code review for transaction-level advisory locks patch. Commit 62c7bd31c8878dd45c9b9b2429ab7a12103f3590 had assorted problems, most visibly that it broke PREPARE TRANSACTION in the presence of session-level advisory locks (which should be ignored by PREPARE), as per a recent complaint from Stephen Rees. More abstractly, the patch made the LockMethodData.transactional flag not merely useless but outright dangerous, because in point of fact that flag no longer tells you anything at all about whether a lock is held transactionally. This fix therefore removes that flag altogether. We now rely entirely on the convention already in use in lock.c that transactional lock holds must be owned by some ResourceOwner, while session holds are never so owned. Setting the locallock struct's owner link to NULL thus denotes a session hold, and there is no redundant marker for that. PREPARE TRANSACTION now works again when there are session-level advisory locks, and it is also able to transfer transactional advisory locks to the prepared transaction, but for implementation reasons it throws an error if we hold both types of lock on a single lockable object. Perhaps it will be worth improving that someday. Assorted other minor cleanup and documentation editing, as well. Back-patch to 9.1, except that in the 9.1 branch I did not remove the LockMethodData.transactional flag for fear of causing an ABI break for any external code that might be examining those structs. --- doc/src/sgml/func.sgml | 67 ++++++----- doc/src/sgml/mvcc.sgml | 80 +++++++------ src/backend/storage/lmgr/README | 16 +-- src/backend/storage/lmgr/lock.c | 198 ++++++++++++++++++++------------ src/backend/storage/lmgr/proc.c | 10 +- src/include/storage/lock.h | 4 +- 6 files changed, 219 insertions(+), 156 deletions(-) diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml index e330909cb19..4eb91311e6b 100644 --- a/doc/src/sgml/func.sgml +++ b/doc/src/sgml/func.sgml @@ -14767,7 +14767,7 @@ SELECT (pg_stat_file('filename')).modification; pg_advisory_xact_lock_shared(key1 int, key2 int) void - Obtain shared advisory lock for the current transaction + Obtain shared transaction level advisory lock @@ -14836,11 +14836,10 @@ SELECT (pg_stat_file('filename')).modification; pg_advisory_lock locks an application-defined resource, which can be identified either by a single 64-bit key value or two 32-bit key values (note that these two key spaces do not overlap). - The key type is specified in pg_locks.objid. If - another session already holds a lock on the same resource, the - function will wait until the resource becomes available. The lock + If another session already holds a lock on the same resource identifier, + this function will wait until the resource becomes available. The lock is exclusive. Multiple lock requests stack, so that if the same resource - is locked three times it must be also unlocked three times to be + is locked three times it must then be unlocked three times to be released for other sessions' use. @@ -14874,6 +14873,35 @@ SELECT (pg_stat_file('filename')).modification; a shared rather than an exclusive lock. + + pg_advisory_unlock + + + pg_advisory_unlock will release a previously-acquired + exclusive session level advisory lock. It + returns true if the lock is successfully released. + If the lock was not held, it will return false, + and in addition, an SQL warning will be reported by the server. + + + + pg_advisory_unlock_shared + + + pg_advisory_unlock_shared works the same as + pg_advisory_unlock, + except it releases a shared session level advisory lock. + + + + pg_advisory_unlock_all + + + pg_advisory_unlock_all will release all session level advisory + locks held by the current session. (This function is implicitly invoked + at session end, even if the client disconnects ungracefully.) + + pg_advisory_xact_lock @@ -14912,35 +14940,6 @@ SELECT (pg_stat_file('filename')).modification; cannot be released explicitly. - - pg_advisory_unlock - - - pg_advisory_unlock will release a previously-acquired - exclusive session level advisory lock. It - returns true if the lock is successfully released. - If the lock was not held, it will return false, - and in addition, an SQL warning will be raised by the server. - - - - pg_advisory_unlock_shared - - - pg_advisory_unlock_shared works the same as - pg_advisory_unlock, - except it releases a shared session level advisory lock. - - - - pg_advisory_unlock_all - - - pg_advisory_unlock_all will release all session level advisory - locks held by the current session. (This function is implicitly invoked - at session end, even if the client disconnects ungracefully.) - - diff --git a/doc/src/sgml/mvcc.sgml b/doc/src/sgml/mvcc.sgml index b311d240109..8bc3ffbc6ad 100644 --- a/doc/src/sgml/mvcc.sgml +++ b/doc/src/sgml/mvcc.sgml @@ -1207,6 +1207,10 @@ UPDATE accounts SET balance = balance - 100.00 WHERE acctnum = 22222; Advisory Locks + + advisory lock + + lock advisory @@ -1218,35 +1222,51 @@ UPDATE accounts SET balance = balance - 100.00 WHERE acctnum = 22222; called advisory locks, because the system does not enforce their use — it is up to the application to use them correctly. Advisory locks can be useful for locking strategies - that are an awkward fit for the MVCC model. - - - There are two different types of advisory locks in - PostgreSQL: session level and transaction level. - Once acquired, a session level advisory lock is held until explicitly - released or the session ends. Unlike standard locks, session level - advisory locks do not honor transaction semantics: a lock acquired during - a transaction that is later rolled back will still be held following the - rollback, and likewise an unlock is effective even if the calling - transaction fails later. The same session level lock can be acquired - multiple times by its owning process: for each lock request there must be - a corresponding unlock request before the lock is actually released. (If a - session already holds a given lock, additional requests will always succeed, - even if other sessions are awaiting the lock.) Transaction level locks on - the other hand behave more like regular locks; they are automatically - released at the end of the transaction, and can not be explicitly unlocked. - Session and transaction level locks share the same lock space, which means - that a transaction level lock will prevent another session from obtaining - a session level lock on that same resource and vice versa. - Like all locks in PostgreSQL, a complete list of - advisory locks currently held by any session can be found in the - pg_locks - system view. + that are an awkward fit for the MVCC model. + For example, a common use of advisory locks is to emulate pessimistic + locking strategies typical of so called flat file data + management systems. + While a flag stored in a table could be used for the same purpose, + advisory locks are faster, avoid table bloat, and are automatically + cleaned up by the server at the end of the session. - Advisory locks are allocated out of a shared memory pool whose size - is defined by the configuration variables + There are two ways to acquire an advisory lock in + PostgreSQL: at session level or at + transaction level. + Once acquired at session level, an advisory lock is held until + explicitly released or the session ends. Unlike standard lock requests, + session-level advisory lock requests do not honor transaction semantics: + a lock acquired during a transaction that is later rolled back will still + be held following the rollback, and likewise an unlock is effective even + if the calling transaction fails later. A lock can be acquired multiple + times by its owning process; for each completed lock request there must + be a corresponding unlock request before the lock is actually released. + Transaction-level lock requests, on the other hand, behave more like + regular lock requests: they are automatically released at the end of the + transaction, and there is no explicit unlock operation. This behavior + is often more convenient than the session-level behavior for short-term + usage of an advisory lock. + Session-level and transaction-level lock requests for the same advisory + lock identifier will block each other in the expected way. + If a session already holds a given advisory lock, additional requests by + it will always succeed, even if other sessions are awaiting the lock; this + statement is true regardless of whether the existing lock hold and new + request are at session level or transaction level. + + + + Like all locks in + PostgreSQL, a complete list of advisory locks + currently held by any session can be found in the pg_locks system + view. + + + + Both advisory locks and regular locks are stored in a shared memory + pool whose size is defined by the configuration variables and . Care must be taken not to exhaust this @@ -1257,13 +1277,7 @@ UPDATE accounts SET balance = balance - 100.00 WHERE acctnum = 22222; - A common use of advisory locks is to emulate pessimistic locking - strategies typical of so called flat file data management - systems. - While a flag stored in a table could be used for the same purpose, - advisory locks are faster, avoid MVCC bloat, and can be automatically - cleaned up by the server at the end of the session. - In certain cases using this advisory locking method, especially in queries + In certain cases using advisory locking methods, especially in queries involving explicit ordering and LIMIT clauses, care must be taken to control the locks acquired because of the order in which SQL expressions are evaluated. For example: diff --git a/src/backend/storage/lmgr/README b/src/backend/storage/lmgr/README index 87fd312e314..81ad489a5a1 100644 --- a/src/backend/storage/lmgr/README +++ b/src/backend/storage/lmgr/README @@ -501,8 +501,8 @@ The caller can then send a cancellation signal. This implements the principle that autovacuum has a low locking priority (eg it must not block DDL on the table). -User Locks ----------- +User Locks (Advisory Locks) +--------------------------- User locks are handled totally on the application side as long term cooperative locks which may extend beyond the normal transaction boundaries. @@ -516,12 +516,12 @@ level by someone. User locks and normal locks are completely orthogonal and they don't interfere with each other. -There are two types of user locks: session level and transaction level. -Session level user locks are not released at transaction end. They must -be released explicitly by the application --- but they are released -automatically when a backend terminates. On the other hand, transaction -level user locks are released automatically at the end of the transaction -as like as other normal locks. +User locks can be acquired either at session level or transaction level. +A session-level lock request is not automatically released at transaction +end, but must be explicitly released by the application. (However, any +remaining locks are always released at session end.) Transaction-level +user lock requests behave the same as normal lock requests, in that they +are released at transaction end and do not need explicit unlocking. Locking during Hot Standby -------------------------- diff --git a/src/backend/storage/lmgr/lock.c b/src/backend/storage/lmgr/lock.c index 7964415456d..6df7e8b239f 100644 --- a/src/backend/storage/lmgr/lock.c +++ b/src/backend/storage/lmgr/lock.c @@ -256,7 +256,7 @@ static uint32 proclock_hash(const void *key, Size keysize); static void RemoveLocalLock(LOCALLOCK *locallock); static void GrantLockLocal(LOCALLOCK *locallock, ResourceOwner owner); static void WaitOnLock(LOCALLOCK *locallock, ResourceOwner owner); -static void ReleaseLockForOwner(LOCALLOCK *locallock, ResourceOwner owner); +static void ReleaseLockIfHeld(LOCALLOCK *locallock, bool sessionLock); static bool UnGrantLock(LOCK *lock, LOCKMODE lockmode, PROCLOCK *proclock, LockMethod lockMethodTable); static void CleanUpLock(LOCK *lock, PROCLOCK *proclock, @@ -525,11 +525,11 @@ LockAcquireExtended(const LOCKTAG *locktag, lockMethodTable->lockModeNames[lockmode]); #endif - /* Session locks are never transactional, else check table */ - if (!sessionLock && lockMethodTable->transactional) - owner = CurrentResourceOwner; - else + /* Identify owner for lock */ + if (sessionLock) owner = NULL; + else + owner = CurrentResourceOwner; /* * Find or create a LOCALLOCK entry for this lock and lockmode @@ -1393,11 +1393,11 @@ LockRelease(const LOCKTAG *locktag, LOCKMODE lockmode, bool sessionLock) ResourceOwner owner; int i; - /* Session locks are never transactional, else check table */ - if (!sessionLock && lockMethodTable->transactional) - owner = CurrentResourceOwner; - else + /* Identify owner for lock */ + if (sessionLock) owner = NULL; + else + owner = CurrentResourceOwner; for (i = locallock->numLockOwners - 1; i >= 0; i--) { @@ -1478,31 +1478,6 @@ LockRelease(const LOCKTAG *locktag, LOCKMODE lockmode, bool sessionLock) return TRUE; } -/* - * LockReleaseSession -- Release all session locks of the specified lock method - * that are held by the current process. - */ -void -LockReleaseSession(LOCKMETHODID lockmethodid) -{ - HASH_SEQ_STATUS status; - LOCALLOCK *locallock; - - if (lockmethodid <= 0 || lockmethodid >= lengthof(LockMethods)) - elog(ERROR, "unrecognized lock method: %d", lockmethodid); - - hash_seq_init(&status, LockMethodLocalHash); - - while ((locallock = (LOCALLOCK *) hash_seq_search(&status)) != NULL) - { - /* Ignore items that are not of the specified lock method */ - if (LOCALLOCK_LOCKMETHOD(*locallock) != lockmethodid) - continue; - - ReleaseLockForOwner(locallock, NULL); - } -} - /* * LockReleaseAll -- Release all locks of the specified lock method that * are held by the current process. @@ -1690,6 +1665,31 @@ LockReleaseAll(LOCKMETHODID lockmethodid, bool allLocks) #endif } +/* + * LockReleaseSession -- Release all session locks of the specified lock method + * that are held by the current process. + */ +void +LockReleaseSession(LOCKMETHODID lockmethodid) +{ + HASH_SEQ_STATUS status; + LOCALLOCK *locallock; + + if (lockmethodid <= 0 || lockmethodid >= lengthof(LockMethods)) + elog(ERROR, "unrecognized lock method: %d", lockmethodid); + + hash_seq_init(&status, LockMethodLocalHash); + + while ((locallock = (LOCALLOCK *) hash_seq_search(&status)) != NULL) + { + /* Ignore items that are not of the specified lock method */ + if (LOCALLOCK_LOCKMETHOD(*locallock) != lockmethodid) + continue; + + ReleaseLockIfHeld(locallock, true); + } +} + /* * LockReleaseCurrentOwner * Release all locks belonging to CurrentResourceOwner @@ -1704,25 +1704,37 @@ LockReleaseCurrentOwner(void) while ((locallock = (LOCALLOCK *) hash_seq_search(&status)) != NULL) { - /* Ignore items that must be nontransactional */ - if (!LockMethods[LOCALLOCK_LOCKMETHOD(*locallock)]->transactional) - continue; - - ReleaseLockForOwner(locallock, CurrentResourceOwner); + ReleaseLockIfHeld(locallock, false); } } /* - * Subroutine to release a lock belonging to the 'owner' if found. - * 'owner' can be NULL to release a session lock. + * ReleaseLockIfHeld + * Release any session-level locks on this lockable object if sessionLock + * is true; else, release any locks held by CurrentResourceOwner. + * + * It is tempting to pass this a ResourceOwner pointer (or NULL for session + * locks), but without refactoring LockRelease() we cannot support releasing + * locks belonging to resource owners other than CurrentResourceOwner. + * If we were to refactor, it'd be a good idea to fix it so we don't have to + * do a hashtable lookup of the locallock, too. However, currently this + * function isn't used heavily enough to justify refactoring for its + * convenience. */ static void -ReleaseLockForOwner(LOCALLOCK *locallock, ResourceOwner owner) +ReleaseLockIfHeld(LOCALLOCK *locallock, bool sessionLock) { - int i; + ResourceOwner owner; LOCALLOCKOWNER *lockOwners; + int i; - /* Scan to see if there are any locks belonging to the owner */ + /* Identify owner for lock (must match LockRelease!) */ + if (sessionLock) + owner = NULL; + else + owner = CurrentResourceOwner; + + /* Scan to see if there are any locks belonging to the target owner */ lockOwners = locallock->lockOwners; for (i = locallock->numLockOwners - 1; i >= 0; i--) { @@ -1749,8 +1761,8 @@ ReleaseLockForOwner(LOCALLOCK *locallock, ResourceOwner owner) locallock->nLocks = 1; if (!LockRelease(&locallock->tag.lock, locallock->tag.mode, - owner == NULL)) - elog(WARNING, "ReleaseLockForOwner: failed??"); + sessionLock)) + elog(WARNING, "ReleaseLockIfHeld: failed??"); } break; } @@ -1780,10 +1792,6 @@ LockReassignCurrentOwner(void) int ic = -1; int ip = -1; - /* Ignore items that must be nontransactional */ - if (!LockMethods[LOCALLOCK_LOCKMETHOD(*locallock)]->transactional) - continue; - /* * Scan to see if there are any locks belonging to current owner or * its parent @@ -1944,13 +1952,13 @@ GetLockConflicts(const LOCKTAG *locktag, LOCKMODE lockmode) * Do the preparatory work for a PREPARE: make 2PC state file records * for all locks currently held. * - * Non-transactional locks are ignored, as are VXID locks. + * Session-level locks are ignored, as are VXID locks. * - * There are some special cases that we error out on: we can't be holding - * any session locks (should be OK since only VACUUM uses those) and we - * can't be holding any locks on temporary objects (since that would mess - * up the current backend if it tries to exit before the prepared xact is - * committed). + * There are some special cases that we error out on: we can't be holding any + * locks at both session and transaction level (since we must either keep or + * give away the PROCLOCK object), and we can't be holding any locks on + * temporary objects (since that would mess up the current backend if it tries + * to exit before the prepared xact is committed). */ void AtPrepare_Locks(void) @@ -1968,12 +1976,10 @@ AtPrepare_Locks(void) { TwoPhaseLockRecord record; LOCALLOCKOWNER *lockOwners = locallock->lockOwners; + bool haveSessionLock; + bool haveXactLock; int i; - /* Ignore nontransactional locks */ - if (!LockMethods[LOCALLOCK_LOCKMETHOD(*locallock)]->transactional) - continue; - /* * Ignore VXID locks. We don't want those to be held by prepared * transactions, since they aren't meaningful after a restart. @@ -1985,14 +1991,38 @@ AtPrepare_Locks(void) if (locallock->nLocks <= 0) continue; - /* Scan to verify there are no session locks */ + /* Scan to see whether we hold it at session or transaction level */ + haveSessionLock = haveXactLock = false; for (i = locallock->numLockOwners - 1; i >= 0; i--) { - /* elog not ereport since this should not happen */ if (lockOwners[i].owner == NULL) - elog(ERROR, "cannot PREPARE when session locks exist"); + haveSessionLock = true; + else + haveXactLock = true; } + /* Ignore it if we have only session lock */ + if (!haveXactLock) + continue; + + /* + * If we have both session- and transaction-level locks, fail. This + * should never happen with regular locks, since we only take those at + * session level in some special operations like VACUUM. It's + * possible to hit this with advisory locks, though. + * + * It would be nice if we could keep the session hold and give away + * the transactional hold to the prepared xact. However, that would + * require two PROCLOCK objects, and we cannot be sure that another + * PROCLOCK will be available when it comes time for PostPrepare_Locks + * to do the deed. So for now, we error out while we can still do so + * safely. + */ + if (haveSessionLock) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot PREPARE while holding both session-level and transaction-level locks on the same object"))); + /* * Create a 2PC record. */ @@ -2047,6 +2077,11 @@ PostPrepare_Locks(TransactionId xid) while ((locallock = (LOCALLOCK *) hash_seq_search(&status)) != NULL) { + LOCALLOCKOWNER *lockOwners = locallock->lockOwners; + bool haveSessionLock; + bool haveXactLock; + int i; + if (locallock->proclock == NULL || locallock->lock == NULL) { /* @@ -2058,15 +2093,29 @@ PostPrepare_Locks(TransactionId xid) continue; } - /* Ignore nontransactional locks */ - if (!LockMethods[LOCALLOCK_LOCKMETHOD(*locallock)]->transactional) - continue; - /* Ignore VXID locks */ if (locallock->tag.lock.locktag_type == LOCKTAG_VIRTUALTRANSACTION) continue; - /* We already checked there are no session locks */ + /* Scan to see whether we hold it at session or transaction level */ + haveSessionLock = haveXactLock = false; + for (i = locallock->numLockOwners - 1; i >= 0; i--) + { + if (lockOwners[i].owner == NULL) + haveSessionLock = true; + else + haveXactLock = true; + } + + /* Ignore it if we have only session lock */ + if (!haveXactLock) + continue; + + /* This can't happen, because we already checked it */ + if (haveSessionLock) + ereport(PANIC, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot PREPARE while holding both session-level and transaction-level locks on the same object"))); /* Mark the proclock to show we need to release this lockmode */ if (locallock->nLocks > 0) @@ -2107,10 +2156,6 @@ PostPrepare_Locks(TransactionId xid) lock = proclock->tag.myLock; - /* Ignore nontransactional locks */ - if (!LockMethods[LOCK_LOCKMETHOD(*lock)]->transactional) - goto next_item; - /* Ignore VXID locks */ if (lock->tag.locktag_type == LOCKTAG_VIRTUALTRANSACTION) goto next_item; @@ -2122,10 +2167,11 @@ PostPrepare_Locks(TransactionId xid) Assert(lock->nGranted <= lock->nRequested); Assert((proclock->holdMask & ~lock->grantMask) == 0); - /* - * Since there were no session locks, we should be releasing all - * locks - */ + /* Ignore it if nothing to release (must be a session lock) */ + if (proclock->releaseMask == 0) + goto next_item; + + /* Else we should be releasing all locks */ if (proclock->releaseMask != proclock->holdMask) elog(PANIC, "we seem to have dropped a bit somewhere"); diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c index 58d95bc0d6b..6ea2fed0b8e 100644 --- a/src/backend/storage/lmgr/proc.c +++ b/src/backend/storage/lmgr/proc.c @@ -642,9 +642,12 @@ LockWaitCancel(void) * ProcReleaseLocks() -- release locks associated with current transaction * at main transaction commit or abort * - * At main transaction commit, we release all locks except session locks. + * At main transaction commit, we release standard locks except session locks. * At main transaction abort, we release all locks including session locks. * + * Advisory locks are released only if they are transaction-level; + * session-level holds remain, whether this is a commit or not. + * * At subtransaction commit, we don't release any locks (so this func is not * needed at all); we will defer the releasing to the parent transaction. * At subtransaction abort, we release all locks held by the subtransaction; @@ -658,10 +661,9 @@ ProcReleaseLocks(bool isCommit) return; /* If waiting, get off wait queue (should only be needed after error) */ LockWaitCancel(); - /* Release locks */ + /* Release standard locks, including session-level if aborting */ LockReleaseAll(DEFAULT_LOCKMETHOD, !isCommit); - - /* Release transaction level advisory locks */ + /* Release transaction-level advisory locks */ LockReleaseAll(USER_LOCKMETHOD, false); } diff --git a/src/include/storage/lock.h b/src/include/storage/lock.h index 7ec961f4430..ace485854d7 100644 --- a/src/include/storage/lock.h +++ b/src/include/storage/lock.h @@ -104,6 +104,8 @@ typedef int LOCKMODE; * are defined in this lock method. Must be less than MAX_LOCKMODES. * * transactional -- TRUE if locks are released automatically at xact end. + * (Caution: this flag no longer means what you might think, and it + * will be removed altogether in 9.2.) * * conflictTab -- this is an array of bitmasks showing lock * mode conflicts. conflictTab[i] is a mask with the j-th bit @@ -484,8 +486,8 @@ extern LockAcquireResult LockAcquireExtended(const LOCKTAG *locktag, bool report_memory_error); extern bool LockRelease(const LOCKTAG *locktag, LOCKMODE lockmode, bool sessionLock); -extern void LockReleaseSession(LOCKMETHODID lockmethodid); extern void LockReleaseAll(LOCKMETHODID lockmethodid, bool allLocks); +extern void LockReleaseSession(LOCKMETHODID lockmethodid); extern void LockReleaseCurrentOwner(void); extern void LockReassignCurrentOwner(void); extern VirtualTransactionId *GetLockConflicts(const LOCKTAG *locktag,