Fix progress reporting of REINDEX CONCURRENTLY
authorMichael Paquier <[email protected]>
Tue, 29 Sep 2020 05:15:57 +0000 (14:15 +0900)
committerMichael Paquier <[email protected]>
Tue, 29 Sep 2020 05:15:57 +0000 (14:15 +0900)
This addresses a couple of issues with the so-said subject:
- Report the correct parent relation with the index actually being
rebuilt or validated.  Previously, the command status remained set to
the last index created for the progress of the index build and
validation, which would be incorrect when working on a table that has
more than one index.
- Use the correct phase when waiting before the drop of the old
indexes.  Previously, this was reported with the same status as when
waiting before the old indexes are marked as dead.

Author: Matthias van de Meent, Michael Paquier
Discussion: https://postgr.es/m/CAEze2WhqFgcwe1_tv=sFYhLWV2AdpfukumotJ6JNcAOQs3jufg@mail.gmail.com
Backpatch-through: 12

src/backend/commands/indexcmds.c

index f1b5f87e6a8cc207dc517f24ac8b63a2382e3b69..59e04b47dfb3659f5f5ef4cb571496459323e1e8 100644 (file)
@@ -3015,6 +3015,13 @@ ReindexRelationConcurrently(Oid relationOid, int options)
    char       *relationName = NULL;
    char       *relationNamespace = NULL;
    PGRUsage    ru0;
+   const int   progress_index[] = {
+       PROGRESS_CREATEIDX_COMMAND,
+       PROGRESS_CREATEIDX_PHASE,
+       PROGRESS_CREATEIDX_INDEX_OID,
+       PROGRESS_CREATEIDX_ACCESS_METHOD_OID
+   };
+   int64       progress_vals[4];
 
    /*
     * Create a memory context that will survive forced transaction commits we
@@ -3294,12 +3301,11 @@ ReindexRelationConcurrently(Oid relationOid, int options)
 
        pgstat_progress_start_command(PROGRESS_COMMAND_CREATE_INDEX,
                                      RelationGetRelid(heapRel));
-       pgstat_progress_update_param(PROGRESS_CREATEIDX_COMMAND,
-                                    PROGRESS_CREATEIDX_COMMAND_REINDEX_CONCURRENTLY);
-       pgstat_progress_update_param(PROGRESS_CREATEIDX_INDEX_OID,
-                                    indexId);
-       pgstat_progress_update_param(PROGRESS_CREATEIDX_ACCESS_METHOD_OID,
-                                    indexRel->rd_rel->relam);
+       progress_vals[0] = PROGRESS_CREATEIDX_COMMAND_REINDEX_CONCURRENTLY;
+       progress_vals[1] = 0;   /* initializing */
+       progress_vals[2] = indexId;
+       progress_vals[3] = indexRel->rd_rel->relam;
+       pgstat_progress_update_multi_param(4, progress_index, progress_vals);
 
        /* Choose a temporary relation name for the new index */
        concurrentName = ChooseRelationName(get_rel_name(indexId),
@@ -3403,12 +3409,12 @@ ReindexRelationConcurrently(Oid relationOid, int options)
    WaitForLockersMultiple(lockTags, ShareLock, true);
    CommitTransactionCommand();
 
-   forboth(lc, indexIds, lc2, newIndexIds)
+   foreach(lc, newIndexIds)
    {
-       Relation    indexRel;
-       Oid         oldIndexId = lfirst_oid(lc);
-       Oid         newIndexId = lfirst_oid(lc2);
+       Relation    newIndexRel;
+       Oid         newIndexId = lfirst_oid(lc);
        Oid         heapId;
+       Oid         indexam;
 
        /* Start new transaction for this index's concurrent build */
        StartTransactionCommand();
@@ -3427,9 +3433,21 @@ ReindexRelationConcurrently(Oid relationOid, int options)
         * Index relation has been closed by previous commit, so reopen it to
         * get its information.
         */
-       indexRel = index_open(oldIndexId, ShareUpdateExclusiveLock);
-       heapId = indexRel->rd_index->indrelid;
-       index_close(indexRel, NoLock);
+       newIndexRel = index_open(newIndexId, ShareUpdateExclusiveLock);
+       heapId = newIndexRel->rd_index->indrelid;
+       indexam = newIndexRel->rd_rel->relam;
+       index_close(newIndexRel, NoLock);
+
+       /*
+        * Update progress for the index to build, with the correct parent
+        * table involved.
+        */
+       pgstat_progress_start_command(PROGRESS_COMMAND_CREATE_INDEX, heapId);
+       progress_vals[0] = PROGRESS_CREATEIDX_COMMAND_REINDEX_CONCURRENTLY;
+       progress_vals[1] = PROGRESS_CREATEIDX_PHASE_BUILD;
+       progress_vals[2] = newIndexId;
+       progress_vals[3] = indexam;
+       pgstat_progress_update_multi_param(4, progress_index, progress_vals);
 
        /* Perform concurrent build of new index */
        index_concurrently_build(heapId, newIndexId);
@@ -3458,6 +3476,8 @@ ReindexRelationConcurrently(Oid relationOid, int options)
        Oid         heapId;
        TransactionId limitXmin;
        Snapshot    snapshot;
+       Relation    newIndexRel;
+       Oid         indexam;
 
        StartTransactionCommand();
 
@@ -3468,8 +3488,6 @@ ReindexRelationConcurrently(Oid relationOid, int options)
         */
        CHECK_FOR_INTERRUPTS();
 
-       heapId = IndexGetRelation(newIndexId, false);
-
        /*
         * Take the "reference snapshot" that will be used by validate_index()
         * to filter candidate tuples.
@@ -3477,6 +3495,26 @@ ReindexRelationConcurrently(Oid relationOid, int options)
        snapshot = RegisterSnapshot(GetTransactionSnapshot());
        PushActiveSnapshot(snapshot);
 
+       /*
+        * Index relation has been closed by previous commit, so reopen it to
+        * get its information.
+        */
+       newIndexRel = index_open(newIndexId, ShareUpdateExclusiveLock);
+       heapId = newIndexRel->rd_index->indrelid;
+       indexam = newIndexRel->rd_rel->relam;
+       index_close(newIndexRel, NoLock);
+
+       /*
+        * Update progress for the index to build, with the correct parent
+        * table involved.
+        */
+       pgstat_progress_start_command(PROGRESS_COMMAND_CREATE_INDEX, heapId);
+       progress_vals[0] = PROGRESS_CREATEIDX_COMMAND_REINDEX_CONCURRENTLY;
+       progress_vals[1] = PROGRESS_CREATEIDX_PHASE_VALIDATE_IDXSCAN;
+       progress_vals[2] = newIndexId;
+       progress_vals[3] = indexam;
+       pgstat_progress_update_multi_param(4, progress_index, progress_vals);
+
        validate_index(heapId, newIndexId, snapshot);
 
        /*
@@ -3611,7 +3649,7 @@ ReindexRelationConcurrently(Oid relationOid, int options)
     */
 
    pgstat_progress_update_param(PROGRESS_CREATEIDX_PHASE,
-                                PROGRESS_CREATEIDX_PHASE_WAIT_4);
+                                PROGRESS_CREATEIDX_PHASE_WAIT_5);
    WaitForLockersMultiple(lockTags, AccessExclusiveLock, true);
 
    PushActiveSnapshot(GetTransactionSnapshot());