diff options
Diffstat (limited to 'src')
| -rw-r--r-- | src/backend/catalog/index.c | 25 | ||||
| -rw-r--r-- | src/backend/catalog/pg_enum.c | 27 | ||||
| -rw-r--r-- | src/backend/commands/cluster.c | 10 | ||||
| -rw-r--r-- | src/backend/utils/time/tqual.c | 223 | ||||
| -rw-r--r-- | src/include/utils/tqual.h | 4 |
5 files changed, 16 insertions, 273 deletions
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index 7e4d7c0578c..b73ee4f2d19 100644 --- a/src/backend/catalog/index.c +++ b/src/backend/catalog/index.c @@ -1258,10 +1258,8 @@ index_constraint_create(Relation heapRelation, /* * If needed, mark the index as primary and/or deferred in pg_index. * - * Note: since this is a transactional update, it's unsafe against - * concurrent SnapshotNow scans of pg_index. When making an existing - * index into a constraint, caller must have a table lock that prevents - * concurrent table updates; if it's less than a full exclusive lock, + * Note: When making an existing index into a constraint, caller must + * have a table lock that prevents concurrent table updates; otherwise, * there is a risk that concurrent readers of the table will miss seeing * this index at all. */ @@ -2989,17 +2987,18 @@ validate_index_heapscan(Relation heapRelation, * index_set_state_flags - adjust pg_index state flags * * This is used during CREATE/DROP INDEX CONCURRENTLY to adjust the pg_index - * flags that denote the index's state. We must use an in-place update of - * the pg_index tuple, because we do not have exclusive lock on the parent - * table and so other sessions might concurrently be doing SnapshotNow scans - * of pg_index to identify the table's indexes. A transactional update would - * risk somebody not seeing the index at all. Because the update is not + * flags that denote the index's state. Because the update is not * transactional and will not roll back on error, this must only be used as * the last step in a transaction that has not made any transactional catalog * updates! * * Note that heap_inplace_update does send a cache inval message for the * tuple, so other sessions will hear about the update as soon as we commit. + * + * NB: In releases prior to PostgreSQL 9.4, the use of a non-transactional + * update here would have been unsafe; now that MVCC rules apply even for + * system catalog scans, we could potentially use a transactional update here + * instead. */ void index_set_state_flags(Oid indexId, IndexStateFlagsAction action) @@ -3207,14 +3206,6 @@ reindex_index(Oid indexId, bool skip_constraint_checks) * be conservative.) In this case advancing the usability horizon is * appropriate. * - * Note that if we have to update the tuple, there is a risk of concurrent - * transactions not seeing it during their SnapshotNow scans of pg_index. - * While not especially desirable, this is safe because no such - * transaction could be trying to update the table (since we have - * ShareLock on it). The worst case is that someone might transiently - * fail to use the index for a query --- but it was probably unusable - * before anyway, if we are updating the tuple. - * * Another reason for avoiding unnecessary updates here is that while * reindexing pg_index itself, we must not try to update tuples in it. * pg_index's indexes should always have these flags in their clean state, diff --git a/src/backend/catalog/pg_enum.c b/src/backend/catalog/pg_enum.c index a7ef8cda596..88711e985e9 100644 --- a/src/backend/catalog/pg_enum.c +++ b/src/backend/catalog/pg_enum.c @@ -462,30 +462,9 @@ restart: * Renumber existing enum elements to have sort positions 1..n. * * We avoid doing this unless absolutely necessary; in most installations - * it will never happen. The reason is that updating existing pg_enum - * entries creates hazards for other backends that are concurrently reading - * pg_enum with SnapshotNow semantics. A concurrent SnapshotNow scan could - * see both old and new versions of an updated row as valid, or neither of - * them, if the commit happens between scanning the two versions. It's - * also quite likely for a concurrent scan to see an inconsistent set of - * rows (some members updated, some not). - * - * We can avoid these risks by reading pg_enum with an MVCC snapshot - * instead of SnapshotNow, but that forecloses use of the syscaches. - * We therefore make the following choices: - * - * 1. Any code that is interested in the enumsortorder values MUST read - * pg_enum with an MVCC snapshot, or else acquire lock on the enum type - * to prevent concurrent execution of AddEnumLabel(). The risk of - * seeing inconsistent values of enumsortorder is too high otherwise. - * - * 2. Code that is not examining enumsortorder can use a syscache - * (for example, enum_in and enum_out do so). The worst that can happen - * is a transient failure to find any valid value of the row. This is - * judged acceptable in view of the infrequency of use of RenumberEnumType. - * - * XXX. Now that we have MVCC catalog scans, the above reasoning is no longer - * correct. Should we revisit any decisions here? + * it will never happen. Before we had MVCC catalog scans, this function + * posed various concurrency hazards. It no longer does, but it's still + * extra work, so we don't do it unless it's needed. */ static void RenumberEnumType(Relation pg_enum, HeapTuple *existing, int nelems) diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c index 4519c00e223..051b806aa72 100644 --- a/src/backend/commands/cluster.c +++ b/src/backend/commands/cluster.c @@ -476,16 +476,6 @@ check_index_is_clusterable(Relation OldHeap, Oid indexOid, bool recheck, LOCKMOD * mark_index_clustered: mark the specified index as the one clustered on * * With indexOid == InvalidOid, will mark all indexes of rel not-clustered. - * - * Note: we do transactional updates of the pg_index rows, which are unsafe - * against concurrent SnapshotNow scans of pg_index. Therefore this is unsafe - * to execute with less than full exclusive lock on the parent table; - * otherwise concurrent executions of RelationGetIndexList could miss indexes. - * - * XXX: Now that we have MVCC catalog access, SnapshotNow scans of pg_index - * shouldn't be common enough to worry about. The above comment needs - * to be updated, and it may be possible to simplify the logic here in other - * ways also. */ void mark_index_clustered(Relation rel, Oid indexOid, bool is_internal) diff --git a/src/backend/utils/time/tqual.c b/src/backend/utils/time/tqual.c index da2ce1825e0..38bb704a4d8 100644 --- a/src/backend/utils/time/tqual.c +++ b/src/backend/utils/time/tqual.c @@ -30,10 +30,8 @@ * * HeapTupleSatisfiesMVCC() * visible to supplied snapshot, excludes current command - * HeapTupleSatisfiesNow() - * visible to instant snapshot, excludes current command * HeapTupleSatisfiesUpdate() - * like HeapTupleSatisfiesNow(), but with user-supplied command + * visible to instant snapshot, with user-supplied command * counter and more complex result * HeapTupleSatisfiesSelf() * visible to instant snapshot and current command @@ -68,7 +66,6 @@ /* Static variables representing various special snapshot semantics */ -SnapshotData SnapshotNowData = {HeapTupleSatisfiesNow}; SnapshotData SnapshotSelfData = {HeapTupleSatisfiesSelf}; SnapshotData SnapshotAnyData = {HeapTupleSatisfiesAny}; SnapshotData SnapshotToastData = {HeapTupleSatisfiesToast}; @@ -324,212 +321,6 @@ HeapTupleSatisfiesSelf(HeapTuple htup, Snapshot snapshot, Buffer buffer) } /* - * HeapTupleSatisfiesNow - * True iff heap tuple is valid "now". - * - * Here, we consider the effects of: - * all committed transactions (as of the current instant) - * previous commands of this transaction - * - * Note we do _not_ include changes made by the current command. This - * solves the "Halloween problem" wherein an UPDATE might try to re-update - * its own output tuples, http://en.wikipedia.org/wiki/Halloween_Problem. - * - * Note: - * Assumes heap tuple is valid. - * - * The satisfaction of "now" requires the following: - * - * ((Xmin == my-transaction && inserted by the current transaction - * Cmin < my-command && before this command, and - * (Xmax is null || the row has not been deleted, or - * (Xmax == my-transaction && it was deleted by the current transaction - * Cmax >= my-command))) but not before this command, - * || or - * (Xmin is committed && the row was inserted by a committed transaction, and - * (Xmax is null || the row has not been deleted, or - * (Xmax == my-transaction && the row is being deleted by this transaction - * Cmax >= my-command) || but it's not deleted "yet", or - * (Xmax != my-transaction && the row was deleted by another transaction - * Xmax is not committed)))) that has not been committed - * - */ -bool -HeapTupleSatisfiesNow(HeapTuple htup, Snapshot snapshot, Buffer buffer) -{ - HeapTupleHeader tuple = htup->t_data; - Assert(ItemPointerIsValid(&htup->t_self)); - Assert(htup->t_tableOid != InvalidOid); - - if (!(tuple->t_infomask & HEAP_XMIN_COMMITTED)) - { - if (tuple->t_infomask & HEAP_XMIN_INVALID) - return false; - - /* Used by pre-9.0 binary upgrades */ - if (tuple->t_infomask & HEAP_MOVED_OFF) - { - TransactionId xvac = HeapTupleHeaderGetXvac(tuple); - - if (TransactionIdIsCurrentTransactionId(xvac)) - return false; - if (!TransactionIdIsInProgress(xvac)) - { - if (TransactionIdDidCommit(xvac)) - { - SetHintBits(tuple, buffer, HEAP_XMIN_INVALID, - InvalidTransactionId); - return false; - } - SetHintBits(tuple, buffer, HEAP_XMIN_COMMITTED, - InvalidTransactionId); - } - } - /* Used by pre-9.0 binary upgrades */ - else if (tuple->t_infomask & HEAP_MOVED_IN) - { - TransactionId xvac = HeapTupleHeaderGetXvac(tuple); - - if (!TransactionIdIsCurrentTransactionId(xvac)) - { - if (TransactionIdIsInProgress(xvac)) - return false; - if (TransactionIdDidCommit(xvac)) - SetHintBits(tuple, buffer, HEAP_XMIN_COMMITTED, - InvalidTransactionId); - else - { - SetHintBits(tuple, buffer, HEAP_XMIN_INVALID, - InvalidTransactionId); - return false; - } - } - } - else if (TransactionIdIsCurrentTransactionId(HeapTupleHeaderGetXmin(tuple))) - { - if (HeapTupleHeaderGetCmin(tuple) >= GetCurrentCommandId(false)) - return false; /* inserted after scan started */ - - if (tuple->t_infomask & HEAP_XMAX_INVALID) /* xid invalid */ - return true; - - if (HEAP_XMAX_IS_LOCKED_ONLY(tuple->t_infomask)) /* not deleter */ - return true; - - if (tuple->t_infomask & HEAP_XMAX_IS_MULTI) - { - TransactionId xmax; - - xmax = HeapTupleGetUpdateXid(tuple); - if (!TransactionIdIsValid(xmax)) - return true; - - /* updating subtransaction must have aborted */ - if (!TransactionIdIsCurrentTransactionId(xmax)) - return true; - else - return false; - } - - if (!TransactionIdIsCurrentTransactionId(HeapTupleHeaderGetRawXmax(tuple))) - { - /* deleting subtransaction must have aborted */ - SetHintBits(tuple, buffer, HEAP_XMAX_INVALID, - InvalidTransactionId); - return true; - } - - if (HeapTupleHeaderGetCmax(tuple) >= GetCurrentCommandId(false)) - return true; /* deleted after scan started */ - else - return false; /* deleted before scan started */ - } - else if (TransactionIdIsInProgress(HeapTupleHeaderGetXmin(tuple))) - return false; - else if (TransactionIdDidCommit(HeapTupleHeaderGetXmin(tuple))) - SetHintBits(tuple, buffer, HEAP_XMIN_COMMITTED, - HeapTupleHeaderGetXmin(tuple)); - else - { - /* it must have aborted or crashed */ - SetHintBits(tuple, buffer, HEAP_XMIN_INVALID, - InvalidTransactionId); - return false; - } - } - - /* by here, the inserting transaction has committed */ - - if (tuple->t_infomask & HEAP_XMAX_INVALID) /* xid invalid or aborted */ - return true; - - if (tuple->t_infomask & HEAP_XMAX_COMMITTED) - { - if (HEAP_XMAX_IS_LOCKED_ONLY(tuple->t_infomask)) - return true; - return false; - } - - if (tuple->t_infomask & HEAP_XMAX_IS_MULTI) - { - TransactionId xmax; - - if (HEAP_XMAX_IS_LOCKED_ONLY(tuple->t_infomask)) - return true; - - xmax = HeapTupleGetUpdateXid(tuple); - if (!TransactionIdIsValid(xmax)) - return true; - if (TransactionIdIsCurrentTransactionId(xmax)) - { - if (HeapTupleHeaderGetCmax(tuple) >= GetCurrentCommandId(false)) - return true; /* deleted after scan started */ - else - return false; /* deleted before scan started */ - } - if (TransactionIdIsInProgress(xmax)) - return true; - if (TransactionIdDidCommit(xmax)) - return false; - return true; - } - - if (TransactionIdIsCurrentTransactionId(HeapTupleHeaderGetRawXmax(tuple))) - { - if (HEAP_XMAX_IS_LOCKED_ONLY(tuple->t_infomask)) - return true; - if (HeapTupleHeaderGetCmax(tuple) >= GetCurrentCommandId(false)) - return true; /* deleted after scan started */ - else - return false; /* deleted before scan started */ - } - - if (TransactionIdIsInProgress(HeapTupleHeaderGetRawXmax(tuple))) - return true; - - if (!TransactionIdDidCommit(HeapTupleHeaderGetRawXmax(tuple))) - { - /* it must have aborted or crashed */ - SetHintBits(tuple, buffer, HEAP_XMAX_INVALID, - InvalidTransactionId); - return true; - } - - /* xmax transaction committed */ - - if (HEAP_XMAX_IS_LOCKED_ONLY(tuple->t_infomask)) - { - SetHintBits(tuple, buffer, HEAP_XMAX_INVALID, - InvalidTransactionId); - return true; - } - - SetHintBits(tuple, buffer, HEAP_XMAX_COMMITTED, - HeapTupleHeaderGetRawXmax(tuple)); - return false; -} - -/* * HeapTupleSatisfiesAny * Dummy "satisfies" routine: any tuple satisfies SnapshotAny. */ @@ -614,10 +405,10 @@ HeapTupleSatisfiesToast(HeapTuple htup, Snapshot snapshot, /* * HeapTupleSatisfiesUpdate * - * Same logic as HeapTupleSatisfiesNow, but returns a more detailed result - * code, since UPDATE needs to know more than "is it visible?". Also, - * tuples of my own xact are tested against the passed CommandId not - * CurrentCommandId. + * This function returns a more detailed result code than most of the + * functions in this file, since UPDATE needs to know more than "is it + * visible?". It also allows for user-supplied CommandId rather than + * relying on CurrentCommandId. * * The possible return codes are: * @@ -1051,10 +842,6 @@ HeapTupleSatisfiesDirty(HeapTuple htup, Snapshot snapshot, * transactions started after the snapshot was taken * changes made by the current command * - * This is the same as HeapTupleSatisfiesNow, except that transactions that - * were in progress or as yet unstarted when the snapshot was taken will - * be treated as uncommitted, even if they have committed by now. - * * (Notice, however, that the tuple status hint bits will be updated on the * basis of the true state of the transaction, even if we then pretend we * can't see it.) diff --git a/src/include/utils/tqual.h b/src/include/utils/tqual.h index 800e366f30b..19f56e4b4d3 100644 --- a/src/include/utils/tqual.h +++ b/src/include/utils/tqual.h @@ -19,12 +19,10 @@ /* Static variables representing various special snapshot semantics */ -extern PGDLLIMPORT SnapshotData SnapshotNowData; extern PGDLLIMPORT SnapshotData SnapshotSelfData; extern PGDLLIMPORT SnapshotData SnapshotAnyData; extern PGDLLIMPORT SnapshotData SnapshotToastData; -#define SnapshotNow (&SnapshotNowData) #define SnapshotSelf (&SnapshotSelfData) #define SnapshotAny (&SnapshotAnyData) #define SnapshotToast (&SnapshotToastData) @@ -67,8 +65,6 @@ typedef enum /* These are the "satisfies" test routines for the various snapshot types */ extern bool HeapTupleSatisfiesMVCC(HeapTuple htup, Snapshot snapshot, Buffer buffer); -extern bool HeapTupleSatisfiesNow(HeapTuple htup, - Snapshot snapshot, Buffer buffer); extern bool HeapTupleSatisfiesSelf(HeapTuple htup, Snapshot snapshot, Buffer buffer); extern bool HeapTupleSatisfiesAny(HeapTuple htup, |