Commit f9900df5 authored by Alvaro Herrera's avatar Alvaro Herrera

Avoid spurious wait in concurrent reindex

This is like commit c98763bf, but for REINDEX CONCURRENTLY.  To wit:
this flags indicates that the current process is safe to ignore for the
purposes of waiting for other snapshots, when doing CREATE INDEX
CONCURRENTLY and REINDEX CONCURRENTLY.  This helps two processes doing
either of those things not deadlock, and also avoids spurious waits.

Author: Álvaro Herrera <alvherre@alvh.no-ip.org>
Reviewed-by: default avatarDmitry Dolgov <9erthalion6@gmail.com>
Reviewed-by: default avatarHamid Akhtar <hamid.akhtar@gmail.com>
Reviewed-by: default avatarMasahiko Sawada <sawada.mshk@gmail.com>
Discussion: https://postgr.es/m/20201130195439.GA24598@alvherre.pgsql
parent 2ad78a87
...@@ -385,7 +385,7 @@ CompareOpclassOptions(Datum *opts1, Datum *opts2, int natts) ...@@ -385,7 +385,7 @@ CompareOpclassOptions(Datum *opts1, Datum *opts2, int natts)
* lazy VACUUMs, because they won't be fazed by missing index entries * lazy VACUUMs, because they won't be fazed by missing index entries
* either. (Manual ANALYZEs, however, can't be excluded because they * either. (Manual ANALYZEs, however, can't be excluded because they
* might be within transactions that are going to do arbitrary operations * might be within transactions that are going to do arbitrary operations
* later.) Processes running CREATE INDEX CONCURRENTLY * later.) Processes running CREATE INDEX CONCURRENTLY or REINDEX CONCURRENTLY
* on indexes that are neither expressional nor partial are also safe to * on indexes that are neither expressional nor partial are also safe to
* ignore, since we know that those processes won't examine any data * ignore, since we know that those processes won't examine any data
* outside the table they're indexing. * outside the table they're indexing.
...@@ -3066,6 +3066,7 @@ ReindexRelationConcurrently(Oid relationOid, int options) ...@@ -3066,6 +3066,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
Oid indexId; Oid indexId;
Oid tableId; Oid tableId;
Oid amId; Oid amId;
bool safe; /* for set_indexsafe_procflags */
} ReindexIndexInfo; } ReindexIndexInfo;
List *heapRelationIds = NIL; List *heapRelationIds = NIL;
List *indexIds = NIL; List *indexIds = NIL;
...@@ -3377,6 +3378,9 @@ ReindexRelationConcurrently(Oid relationOid, int options) ...@@ -3377,6 +3378,9 @@ ReindexRelationConcurrently(Oid relationOid, int options)
heapRel = table_open(indexRel->rd_index->indrelid, heapRel = table_open(indexRel->rd_index->indrelid,
ShareUpdateExclusiveLock); ShareUpdateExclusiveLock);
/* determine safety of this index for set_indexsafe_procflags */
idx->safe = (indexRel->rd_indexprs == NIL &&
indexRel->rd_indpred == NIL);
idx->tableId = RelationGetRelid(heapRel); idx->tableId = RelationGetRelid(heapRel);
idx->amId = indexRel->rd_rel->relam; idx->amId = indexRel->rd_rel->relam;
...@@ -3418,6 +3422,7 @@ ReindexRelationConcurrently(Oid relationOid, int options) ...@@ -3418,6 +3422,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
newidx = palloc(sizeof(ReindexIndexInfo)); newidx = palloc(sizeof(ReindexIndexInfo));
newidx->indexId = newIndexId; newidx->indexId = newIndexId;
newidx->safe = idx->safe;
newidx->tableId = idx->tableId; newidx->tableId = idx->tableId;
newidx->amId = idx->amId; newidx->amId = idx->amId;
...@@ -3485,6 +3490,11 @@ ReindexRelationConcurrently(Oid relationOid, int options) ...@@ -3485,6 +3490,11 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand(); CommitTransactionCommand();
StartTransactionCommand(); StartTransactionCommand();
/*
* Because we don't take a snapshot in this transaction, there's no need
* to set the PROC_IN_SAFE_IC flag here.
*/
/* /*
* Phase 2 of REINDEX CONCURRENTLY * Phase 2 of REINDEX CONCURRENTLY
* *
...@@ -3514,6 +3524,10 @@ ReindexRelationConcurrently(Oid relationOid, int options) ...@@ -3514,6 +3524,10 @@ ReindexRelationConcurrently(Oid relationOid, int options)
*/ */
CHECK_FOR_INTERRUPTS(); CHECK_FOR_INTERRUPTS();
/* Tell concurrent indexing to ignore us, if index qualifies */
if (newidx->safe)
set_indexsafe_procflags();
/* Set ActiveSnapshot since functions in the indexes may need it */ /* Set ActiveSnapshot since functions in the indexes may need it */
PushActiveSnapshot(GetTransactionSnapshot()); PushActiveSnapshot(GetTransactionSnapshot());
...@@ -3534,8 +3548,14 @@ ReindexRelationConcurrently(Oid relationOid, int options) ...@@ -3534,8 +3548,14 @@ ReindexRelationConcurrently(Oid relationOid, int options)
PopActiveSnapshot(); PopActiveSnapshot();
CommitTransactionCommand(); CommitTransactionCommand();
} }
StartTransactionCommand(); StartTransactionCommand();
/*
* Because we don't take a snapshot or Xid in this transaction, there's no
* need to set the PROC_IN_SAFE_IC flag here.
*/
/* /*
* Phase 3 of REINDEX CONCURRENTLY * Phase 3 of REINDEX CONCURRENTLY
* *
...@@ -3564,6 +3584,10 @@ ReindexRelationConcurrently(Oid relationOid, int options) ...@@ -3564,6 +3584,10 @@ ReindexRelationConcurrently(Oid relationOid, int options)
*/ */
CHECK_FOR_INTERRUPTS(); CHECK_FOR_INTERRUPTS();
/* Tell concurrent indexing to ignore us, if index qualifies */
if (newidx->safe)
set_indexsafe_procflags();
/* /*
* Take the "reference snapshot" that will be used by validate_index() * Take the "reference snapshot" that will be used by validate_index()
* to filter candidate tuples. * to filter candidate tuples.
...@@ -3607,6 +3631,9 @@ ReindexRelationConcurrently(Oid relationOid, int options) ...@@ -3607,6 +3631,9 @@ ReindexRelationConcurrently(Oid relationOid, int options)
* interesting tuples. But since it might not contain tuples deleted * interesting tuples. But since it might not contain tuples deleted
* just before the reference snap was taken, we have to wait out any * just before the reference snap was taken, we have to wait out any
* transactions that might have older snapshots. * transactions that might have older snapshots.
*
* Because we don't take a snapshot or Xid in this transaction,
* there's no need to set the PROC_IN_SAFE_IC flag here.
*/ */
pgstat_progress_update_param(PROGRESS_CREATEIDX_PHASE, pgstat_progress_update_param(PROGRESS_CREATEIDX_PHASE,
PROGRESS_CREATEIDX_PHASE_WAIT_3); PROGRESS_CREATEIDX_PHASE_WAIT_3);
...@@ -3628,6 +3655,13 @@ ReindexRelationConcurrently(Oid relationOid, int options) ...@@ -3628,6 +3655,13 @@ ReindexRelationConcurrently(Oid relationOid, int options)
StartTransactionCommand(); StartTransactionCommand();
/*
* Because this transaction only does catalog manipulations and doesn't do
* any index operations, we can set the PROC_IN_SAFE_IC flag here
* unconditionally.
*/
set_indexsafe_procflags();
forboth(lc, indexIds, lc2, newIndexIds) forboth(lc, indexIds, lc2, newIndexIds)
{ {
ReindexIndexInfo *oldidx = lfirst(lc); ReindexIndexInfo *oldidx = lfirst(lc);
...@@ -3675,6 +3709,12 @@ ReindexRelationConcurrently(Oid relationOid, int options) ...@@ -3675,6 +3709,12 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand(); CommitTransactionCommand();
StartTransactionCommand(); StartTransactionCommand();
/*
* While we could set PROC_IN_SAFE_IC if all indexes qualified, there's no
* real need for that, because we only acquire an Xid after the wait is
* done, and that lasts for a very short period.
*/
/* /*
* Phase 5 of REINDEX CONCURRENTLY * Phase 5 of REINDEX CONCURRENTLY
* *
...@@ -3705,6 +3745,12 @@ ReindexRelationConcurrently(Oid relationOid, int options) ...@@ -3705,6 +3745,12 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand(); CommitTransactionCommand();
StartTransactionCommand(); StartTransactionCommand();
/*
* While we could set PROC_IN_SAFE_IC if all indexes qualified, there's no
* real need for that, because we only acquire an Xid after the wait is
* done, and that lasts for a very short period.
*/
/* /*
* Phase 6 of REINDEX CONCURRENTLY * Phase 6 of REINDEX CONCURRENTLY
* *
......
...@@ -54,6 +54,7 @@ struct XidCache ...@@ -54,6 +54,7 @@ struct XidCache
#define PROC_IS_AUTOVACUUM 0x01 /* is it an autovac worker? */ #define PROC_IS_AUTOVACUUM 0x01 /* is it an autovac worker? */
#define PROC_IN_VACUUM 0x02 /* currently running lazy vacuum */ #define PROC_IN_VACUUM 0x02 /* currently running lazy vacuum */
#define PROC_IN_SAFE_IC 0x04 /* currently running CREATE INDEX #define PROC_IN_SAFE_IC 0x04 /* currently running CREATE INDEX
* CONCURRENTLY or REINDEX
* CONCURRENTLY on non-expressional, * CONCURRENTLY on non-expressional,
* non-partial index */ * non-partial index */
#define PROC_VACUUM_FOR_WRAPAROUND 0x08 /* set by autovac only */ #define PROC_VACUUM_FOR_WRAPAROUND 0x08 /* set by autovac only */
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment