From: Álvaro Herrera <alvherre@alvh.no-ip.org>
To: Masahiko Sawada <sawada.mshk@gmail.com>
Cc: Hamid Akhtar <hamid.akhtar@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: {CREATE INDEX, REINDEX} CONCURRENTLY improvements
Date: Tue, 12 Jan 2021 17:21:29 -0300
Message-ID: <20210112202129.GA7219@alvherre.pgsql> (raw)
In-Reply-To: <20210112201028.GA5902@alvherre.pgsql>
On 2021-Jan-12, Álvaro Herrera wrote:
> > For the 0001 patch, since ReindexIndexInfo is used only within
> > ReindexRelationConcurrently() I think it’s a function-local structure
> > type. So we can declare it within the function. What do you think?
>
> That's a good idea. Pushed with that change, thanks.
Here's the other patch, with comments fixed per reviews, and rebased to
account for the scope change.
--
Álvaro Herrera Valdivia, Chile
Tulio: oh, para qué servirá este boton, Juan Carlos?
Policarpo: No, aléjense, no toquen la consola!
Juan Carlos: Lo apretaré una y otra vez.
Attachments:v2-0001-set-PROC_IN_SAFE_IC-during-REINDEX-CONCURRENTLY.patch (text/x-diff, 6.2K) download
From f2476636a358b77aca4b7adb36ca8e383e78af9b Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Mon, 30 Nov 2020 16:01:46 -0300
Subject: [PATCH v2] set PROC_IN_SAFE_IC during REINDEX CONCURRENTLY
---
src/backend/commands/indexcmds.c | 56 +++++++++++++++++++++++++++++---
src/include/storage/proc.h | 1 +
2 files changed, 53 insertions(+), 4 deletions(-)
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.cindex 8c9c39a467..35c3d20eae 100644--- a/src/backend/commands/indexcmds.c+++ b/src/backend/commands/indexcmds.c@@ -385,7 +385,7 @@ CompareOpclassOptions(Datum *opts1, Datum *opts2, int natts)
* lazy VACUUMs, because they won't be fazed by missing index entries
* either. (Manual ANALYZEs, however, can't be excluded because they
* 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
* ignore, since we know that those processes won't examine any data
* outside the table they're indexing.
@@ -1566,9 +1566,11 @@ DefineIndex(Oid relationId,
CommitTransactionCommand();
StartTransactionCommand();
- /* Tell concurrent index builds to ignore us, if index qualifies */- if (safe_index)- set_indexsafe_procflags();+ /*+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because+ * it only acquires an Xid to do some catalog manipulations, after the+ * wait is over.+ */
/* We should now definitely not be advertising any xmin. */
Assert(MyProc->xmin == InvalidTransactionId);
@@ -3066,6 +3068,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
Oid indexId;
Oid tableId;
Oid amId;
+ bool safe; /* for set_indexsafe_procflags */
} ReindexIndexInfo;
List *heapRelationIds = NIL;
List *indexIds = NIL;
@@ -3377,6 +3380,9 @@ ReindexRelationConcurrently(Oid relationOid, int options)
heapRel = table_open(indexRel->rd_index->indrelid,
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->amId = indexRel->rd_rel->relam;
@@ -3418,6 +3424,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
newidx = palloc(sizeof(ReindexIndexInfo));
newidx->indexId = newIndexId;
+ newidx->safe = idx->safe;
newidx->tableId = idx->tableId;
newidx->amId = idx->amId;
@@ -3485,6 +3492,11 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
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
*
@@ -3514,6 +3526,10 @@ ReindexRelationConcurrently(Oid relationOid, int options)
*/
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 */
PushActiveSnapshot(GetTransactionSnapshot());
@@ -3534,8 +3550,14 @@ ReindexRelationConcurrently(Oid relationOid, int options)
PopActiveSnapshot();
CommitTransactionCommand();
}
+
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 3 of REINDEX CONCURRENTLY
*
@@ -3564,6 +3586,10 @@ ReindexRelationConcurrently(Oid relationOid, int options)
*/
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()
* to filter candidate tuples.
@@ -3607,6 +3633,9 @@ ReindexRelationConcurrently(Oid relationOid, int options)
* interesting tuples. But since it might not contain tuples deleted
* just before the reference snap was taken, we have to wait out any
* transactions that might have older snapshots.
+ *+ * Because we don't take a snapshot in this transaction, there's no+ * need to set the PROC_IN_SAFE_IC flag here.
*/
pgstat_progress_update_param(PROGRESS_CREATEIDX_PHASE,
PROGRESS_CREATEIDX_PHASE_WAIT_3);
@@ -3628,6 +3657,13 @@ ReindexRelationConcurrently(Oid relationOid, int options)
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)
{
ReindexIndexInfo *oldidx = lfirst(lc);
@@ -3675,6 +3711,12 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
StartTransactionCommand();
+ /*+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because+ * it only acquires an Xid to do some catalog manipulations, after the+ * wait is over.+ */+
/*
* Phase 5 of REINDEX CONCURRENTLY
*
@@ -3705,6 +3747,12 @@ ReindexRelationConcurrently(Oid relationOid, int options)
CommitTransactionCommand();
StartTransactionCommand();
+ /*+ * This transaction doesn't need to set the PROC_IN_SAFE_IC flag, because+ * it only acquires an Xid to do some catalog manipulations, after all the+ * waiting has been completed.+ */+
/*
* Phase 6 of REINDEX CONCURRENTLY
*
diff --git a/src/include/storage/proc.h b/src/include/storage/proc.hindex 0786fcf103..683ab64f76 100644--- a/src/include/storage/proc.h+++ b/src/include/storage/proc.h@@ -54,6 +54,7 @@ struct XidCache
#define PROC_IS_AUTOVACUUM 0x01 /* is it an autovac worker? */
#define PROC_IN_VACUUM 0x02 /* currently running lazy vacuum */
#define PROC_IN_SAFE_IC 0x04 /* currently running CREATE INDEX
+ * CONCURRENTLY or REINDEX
* CONCURRENTLY on non-expressional,
* non-partial index */
#define PROC_VACUUM_FOR_WRAPAROUND 0x08 /* set by autovac only */
--
2.20.1
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: alvherre@alvh.no-ip.org, sawada.mshk@gmail.com, hamid.akhtar@gmail.com, pgsql-hackers@lists.postgresql.org
Subject: Re: {CREATE INDEX, REINDEX} CONCURRENTLY improvements
In-Reply-To: <20210112202129.GA7219@alvherre.pgsql>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox