agora inbox for [email protected]
help / color / mirror / Atom feedFrom: Antonin Houska <[email protected]>
Subject: [PATCH] Allow progress tracking of sub-commands.
Date: Mon, 29 Jun 2026 13:10:49 +0200
Some commands that support progress reporting run other commands which also
need to report their progress. The typical case is that REPACK ("main"
command) runs index build (sub-command).
The current approach is to disable progress tracking when a command is running
as a sub-command of another command. However, the progress reporting can be
AM-specific, so it's not really practicable to suppress the progress reporting
of the sub-commands entirely. Even if we managed to pass a flag down to all
the relevant places, we could end up with progress reporting for the
sub-command disabled in cases it's important to the user.
Instead of disabling the progress tracking of the sub-commands, this patch
allows both the main command and the sub-command to report their progress at
the same time.
---
contrib/file_fdw/file_fdw.c | 44 +++++++++++
src/backend/access/brin/brin.c | 3 +
src/backend/access/transam/xact.c | 33 +++++++-
src/backend/bootstrap/bootstrap.c | 11 ++-
src/backend/catalog/heap.c | 13 ++-
src/backend/catalog/index.c | 50 ++++++------
src/backend/catalog/toasting.c | 9 +++
src/backend/commands/indexcmds.c | 62 ++++++++-------
src/backend/commands/repack.c | 35 ++++++--
src/backend/utils/activity/backend_progress.c | 79 ++++++++++++++++---
src/backend/utils/activity/backend_status.c | 1 +
src/backend/utils/adt/pgstatfuncs.c | 15 +++-
src/include/catalog/index.h | 9 +--
src/include/utils/backend_status.h | 19 +++++
14 files changed, 294 insertions(+), 89 deletions(-)
diff --git a/contrib/file_fdw/file_fdw.c b/contrib/file_fdw/file_fdw.c
index 33a37d832ce..a60fb226320 100644
--- a/contrib/file_fdw/file_fdw.c
+++ b/contrib/file_fdw/file_fdw.c
@@ -36,6 +36,7 @@
#include "optimizer/pathnode.h"
#include "optimizer/planmain.h"
#include "optimizer/restrictinfo.h"
+#include "utils/backend_status.h"
#include "utils/acl.h"
#include "utils/memutils.h"
#include "utils/rel.h"
@@ -119,6 +120,16 @@ typedef struct FileFdwExecutionState
CopyFromState cstate; /* COPY execution state */
} FileFdwExecutionState;
+/*
+ * Since progress tracking of multiple COPY commands is not supported, the
+ * first file_fdw node of the plan needs to set pgstat_track_activities to
+ * false during startup, and the last active node needs to restore the
+ * original value during shutdown.
+ */
+static bool save_pgstat_track_activities = false;
+static int fdw_nodes = 0;
+static int active_fdw_nodes = 0;
+
/*
* SQL functions
*/
@@ -616,6 +627,12 @@ fileGetForeignPlan(PlannerInfo *root,
{
Index scan_relid = baserel->relid;
+ /*
+ * This seems to be the appropriate place to count file_fdw nodes in the
+ * plan.
+ */
+ fdw_nodes++;
+
/*
* We have no native ability to evaluate restriction clauses, so we just
* put all the scan_clauses into the plan node's qual list for the
@@ -695,6 +712,18 @@ fileBeginForeignScan(ForeignScanState *node, int eflags)
/* Add any options from the plan (currently only convert_selectively) */
options = list_concat(options, plan->fdw_private);
+ /*
+ * Save the value of pgstat_track_activities if this is the first file_fdw
+ * node of a plan containing multiple file_fdw nodes, and disable the
+ * progress tracking. The monitoring infrastructure currently does not
+ * support monitoring of multiple COPY commands.
+ */
+ if (fdw_nodes > 1 && active_fdw_nodes++ == 0)
+ {
+ save_pgstat_track_activities = pgstat_track_activities;
+ pgstat_track_activities = false;
+ }
+
/*
* Create CopyState from FDW options. We always acquire all columns, so
* as to match the expected ScanTupleSlot signature.
@@ -861,6 +890,21 @@ fileEndForeignScan(ForeignScanState *node)
festate->cstate->num_errors));
EndCopyFrom(festate->cstate);
+
+
+ /*
+ * Restore the value of pgstat_track_activities if this is the last
+ * file_fdw node of a plan containing multiple file_fdw nodes, and enable
+ * progress tracking if we disabled it earlier.
+ */
+ if (active_fdw_nodes > 0)
+ {
+ if (--active_fdw_nodes == 0)
+ {
+ pgstat_track_activities = save_pgstat_track_activities;
+ fdw_nodes = 0;
+ }
+ }
}
/*
diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c
index bdb30752e09..f84b992478a 100644
--- a/src/backend/access/brin/brin.c
+++ b/src/backend/access/brin/brin.c
@@ -2847,8 +2847,11 @@ _brin_parallel_scan_and_build(BrinBuildState *state,
ParallelTableScanFromBrinShared(brinshared),
SO_NONE);
+ pgstat_progress_start_command(PROGRESS_COMMAND_CREATE_INDEX,
+ RelationGetRelid(heap));
reltuples = table_index_build_scan(heap, index, indexInfo, true, true,
brinbuildCallbackParallel, state, scan);
+ pgstat_progress_end_command();
/* insert the last item */
form_and_spill_tuple(state);
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index 3a89149016f..78a77fe4bb3 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -217,6 +217,8 @@ typedef struct TransactionStateData
bool parallelChildXact; /* is any parent transaction parallel? */
bool chain; /* start a new block after this one */
bool topXidLogged; /* for a subxact: is top-level XID logged? */
+ int progress_depth; /* progress tracking state before subxact
+ * started */
struct TransactionStateData *parent; /* back link to parent */
} TransactionStateData;
@@ -2253,6 +2255,12 @@ StartTransaction(void)
*/
s->state = TRANS_INPROGRESS;
+ /*
+ * Top transaction always terminates progress tracking on abort, so this
+ * field is not needed, but be tidy.
+ */
+ s->progress_depth = 0;
+
/* Schedule transaction timeout */
if (TransactionTimeout > 0)
enable_timeout_after(TRANSACTION_TIMEOUT, TransactionTimeout);
@@ -5139,6 +5147,12 @@ StartSubTransaction(void)
s->state = TRANS_INPROGRESS;
+ /*
+ * Subtransaction needs to know if it should cancel progress reporting on
+ * abort.
+ */
+ s->progress_depth = PGSTAT_PROGRESS_STATE(MyBEEntry);
+
/*
* Call start-of-subxact callbacks
*/
@@ -5298,7 +5312,24 @@ AbortSubTransaction(void)
WaitLSNCleanup();
pgstat_report_wait_end();
- pgstat_progress_end_command();
+
+ /*
+ * Abort of a subtransaction does not imply the end of monitoring for the
+ * whole transaction. Instead, we only cancel monitoring started within
+ * the subtransaction.
+ *
+ * One particular case it matters is an index function that catches an
+ * exception and lets the index build continue, so the abort should not
+ * cancel monitoring of that build.
+ *
+ * XXX Shouldn't we restore the progress state as it was before the
+ * subtransaction started? Not sure about use case - the index functions
+ * mentioned above probably don't start their own progress tracking. On
+ * the other hand, a monitored command started in a subtransaction usually
+ * does not have a parent command in the parent transaction.
+ */
+ for (int i = PGSTAT_PROGRESS_STATE(MyBEEntry); i > s->progress_depth; i--)
+ pgstat_progress_end_command();
pgaio_error_cleanup();
diff --git a/src/backend/bootstrap/bootstrap.c b/src/backend/bootstrap/bootstrap.c
index b0dcd9876c5..d15a06aa99c 100644
--- a/src/backend/bootstrap/bootstrap.c
+++ b/src/backend/bootstrap/bootstrap.c
@@ -40,6 +40,7 @@
#include "storage/ipc.h"
#include "storage/proc.h"
#include "storage/shmem_internal.h"
+#include "utils/backend_progress.h"
#include "utils/builtins.h"
#include "utils/fmgroids.h"
#include "utils/guc.h"
@@ -1188,7 +1189,15 @@ build_indices(void)
heap = table_open(ILHead->il_heap, NoLock);
ind = index_open(ILHead->il_ind, NoLock);
- index_build(heap, ind, ILHead->il_info, false, false, false);
+ /*
+ * Progress tracking is not really needed here, but it's simpler to
+ * start it than to disable it everywhere (including the specific
+ * AMs).
+ */
+ pgstat_progress_start_command(PROGRESS_COMMAND_CREATE_INDEX,
+ RelationGetRelid(heap));
+ index_build(heap, ind, ILHead->il_info, false, false);
+ pgstat_progress_end_command();
index_close(ind, NoLock);
table_close(heap, NoLock);
diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index 88087654de9..5339cfa51de 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -3588,10 +3588,15 @@ RelationTruncateIndexes(Relation heapRelation)
*/
RelationTruncate(currentIndex, 0);
- /* Initialize the index and rebuild */
- /* Note: we do not need to re-establish pkey setting */
- index_build(heapRelation, currentIndex, indexInfo, true, false,
- true);
+ /*
+ * Initialize the index and rebuild
+ *
+ * Note: we do not need to re-establish pkey setting
+ */
+ pgstat_progress_start_command(PROGRESS_COMMAND_CREATE_INDEX,
+ RelationGetRelid(heapRelation));
+ index_build(heapRelation, currentIndex, indexInfo, true, false);
+ pgstat_progress_end_command();
/* We're done with this index */
index_close(currentIndex, NoLock);
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 9407c357f27..e5d12308b6e 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -715,8 +715,6 @@ UpdateIndexRelation(Oid indexoid,
* already exists.
* INDEX_CREATE_PARTITIONED:
* create a partitioned index (table must be partitioned)
- * INDEX_CREATE_SUPPRESS_PROGRESS:
- * don't report progress during the index build.
*
* constr_flags: flags passed to index_constraint_create
* (only if INDEX_CREATE_ADD_CONSTRAINT is set)
@@ -763,7 +761,6 @@ index_create(Relation heapRelation,
bool invalid = (flags & INDEX_CREATE_INVALID) != 0;
bool concurrent = (flags & INDEX_CREATE_CONCURRENT) != 0;
bool partitioned = (flags & INDEX_CREATE_PARTITIONED) != 0;
- bool progress = (flags & INDEX_CREATE_SUPPRESS_PROGRESS) == 0;
char relkind;
TransactionId relfrozenxid;
MultiXactId relminmxid;
@@ -1280,8 +1277,7 @@ index_create(Relation heapRelation,
}
else
{
- index_build(heapRelation, indexRelation, indexInfo, false, true,
- progress);
+ index_build(heapRelation, indexRelation, indexInfo, false, true);
}
/*
@@ -1540,7 +1536,7 @@ index_concurrently_build(Oid heapRelationId,
indexInfo->ii_BrokenHotChain = false;
/* Now build the index */
- index_build(heapRel, indexRelation, indexInfo, false, true, true);
+ index_build(heapRel, indexRelation, indexInfo, false, true);
/* Roll back any GUC changes executed by index functions */
AtEOXact_GUC(false, save_nestlevel);
@@ -2221,6 +2217,8 @@ index_drop(Oid indexId, bool concurrent, bool concurrent_lock_mode)
*/
if (concurrent)
{
+ bool progress;
+
/*
* We must commit our transaction in order to make the first pg_index
* state update visible to other sessions. If the DROP machinery has
@@ -2291,11 +2289,12 @@ index_drop(Oid indexId, bool concurrent, bool concurrent_lock_mode)
* to acquire an exclusive lock on our table. The lock code will
* detect deadlock and error out properly.
*
- * Note: we report progress through WaitForLockers() unconditionally
- * here, even though it will only be used when we're called by REINDEX
- * CONCURRENTLY and not when called by DROP INDEX CONCURRENTLY.
+ * Note: we report progress through WaitForLockers() only when the
+ * command is REINDEX CONCURRENTLY. There's no progress reporting for
+ * DROP INDEX CONCURRENTLY.
*/
- WaitForLockers(heaplocktag, AccessExclusiveLock, true);
+ progress = MyBEEntry->st_progress_command == PROGRESS_COMMAND_CREATE_INDEX;
+ WaitForLockers(heaplocktag, AccessExclusiveLock, progress);
/*
* Updating pg_index might involve TOAST table access, so ensure we
@@ -2319,7 +2318,7 @@ index_drop(Oid indexId, bool concurrent, bool concurrent_lock_mode)
* Wait till every transaction that saw the old index state has
* finished. See above about progress reporting.
*/
- WaitForLockers(heaplocktag, AccessExclusiveLock, true);
+ WaitForLockers(heaplocktag, AccessExclusiveLock, progress);
/*
* Re-open relations to allow us to complete our actions.
@@ -3011,7 +3010,6 @@ index_update_stats(Relation rel,
*
* isreindex indicates we are recreating a previously-existing index.
* parallel indicates if parallelism may be useful.
- * progress indicates if the backend should update its progress info.
*
* Note: before Postgres 8.2, the passed-in heap and index Relations
* were automatically closed by this routine. This is no longer the case.
@@ -3022,8 +3020,7 @@ index_build(Relation heapRelation,
Relation indexRelation,
IndexInfo *indexInfo,
bool isreindex,
- bool parallel,
- bool progress)
+ bool parallel)
{
IndexBuildResult *stats;
Oid save_userid;
@@ -3074,7 +3071,6 @@ index_build(Relation heapRelation,
RestrictSearchPath();
/* Set up initial progress report status */
- if (progress)
{
const int progress_index[] = {
PROGRESS_CREATEIDX_PHASE,
@@ -3639,7 +3635,6 @@ reindex_index(const ReindexStmt *stmt, Oid indexId,
IndexInfo *indexInfo;
volatile bool skipped_constraint = false;
PGRUsage ru0;
- bool progress = ((params->options & REINDEXOPT_REPORT_PROGRESS) != 0);
bool set_tablespace = false;
pg_rusage_init(&ru0);
@@ -3674,7 +3669,6 @@ reindex_index(const ReindexStmt *stmt, Oid indexId,
save_nestlevel = NewGUCNestLevel();
RestrictSearchPath();
- if (progress)
{
const int progress_cols[] = {
PROGRESS_CREATEIDX_COMMAND,
@@ -3713,9 +3707,8 @@ reindex_index(const ReindexStmt *stmt, Oid indexId,
return;
}
- if (progress)
- pgstat_progress_update_param(PROGRESS_CREATEIDX_ACCESS_METHOD_OID,
- iRel->rd_rel->relam);
+ pgstat_progress_update_param(PROGRESS_CREATEIDX_ACCESS_METHOD_OID,
+ iRel->rd_rel->relam);
/*
* If a statement is available, telling that this comes from a REINDEX
@@ -3832,7 +3825,7 @@ reindex_index(const ReindexStmt *stmt, Oid indexId,
/* Initialize the index and rebuild */
/* Note: we do not need to re-establish pkey setting */
- index_build(heapRelation, iRel, indexInfo, true, true, progress);
+ index_build(heapRelation, iRel, indexInfo, true, true);
/* Re-allow use of target index */
ResetReindexProcessing();
@@ -3926,8 +3919,7 @@ reindex_index(const ReindexStmt *stmt, Oid indexId,
index_close(iRel, NoLock);
table_close(heapRelation, NoLock);
- if (progress)
- pgstat_progress_end_command();
+ pgstat_progress_end_command();
}
/*
@@ -4099,9 +4091,15 @@ reindex_relation(const ReindexStmt *stmt, Oid relid, int flags,
/* Index should no longer be in the pending list */
Assert(!ReindexIsProcessingIndex(indexOid));
- /* Set index rebuild count */
- pgstat_progress_update_param(PROGRESS_REPACK_INDEX_REBUILD_COUNT,
- i);
+ /*
+ * Set index rebuild count - only interesting for REPACK.
+ *
+ * XXX Shouldn't we add an argument indicating that the function is
+ * being called from REPACK (or its subroutine)?
+ */
+ if (MyBEEntry->st_progress_command == PROGRESS_COMMAND_REPACK)
+ pgstat_progress_update_param(PROGRESS_REPACK_INDEX_REBUILD_COUNT,
+ i);
i++;
}
diff --git a/src/backend/catalog/toasting.c b/src/backend/catalog/toasting.c
index 4aa52a4bd25..672dcba84f4 100644
--- a/src/backend/catalog/toasting.c
+++ b/src/backend/catalog/toasting.c
@@ -30,6 +30,7 @@
#include "catalog/toasting.h"
#include "miscadmin.h"
#include "nodes/makefuncs.h"
+#include "utils/backend_progress.h"
#include "utils/fmgroids.h"
#include "utils/rel.h"
#include "utils/syscache.h"
@@ -325,6 +326,13 @@ create_toast_table(Relation rel, Oid toastOid, Oid toastIndexOid,
coloptions[0] = 0;
coloptions[1] = 0;
+ /*
+ * XXX Progress reporting makes little sense for empty table, but
+ * index_create() does report the progress and it's not worth disabling it
+ * in this specific case.
+ */
+ pgstat_progress_start_command(PROGRESS_COMMAND_CREATE_INDEX,
+ RelationGetRelid(toast_rel));
index_create(toast_rel, toast_idxname, toastIndexOid, InvalidOid,
InvalidOid, InvalidOid,
indexInfo,
@@ -333,6 +341,7 @@ create_toast_table(Relation rel, Oid toastOid, Oid toastIndexOid,
rel->rd_rel->reltablespace,
collationIds, opclassIds, NULL, coloptions, NULL, (Datum) 0,
INDEX_CREATE_IS_PRIMARY, 0, true, true, NULL);
+ pgstat_progress_end_command();
table_close(toast_rel, NoLock);
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 9ab74c8df0a..c2d4b9ff087 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -594,6 +594,7 @@ DefineIndex(ParseState *pstate,
Oid root_save_userid;
int root_save_sec_context;
int root_save_nestlevel;
+ bool progress_started = false;
root_save_nestlevel = NewGUCNestLevel();
@@ -622,16 +623,21 @@ DefineIndex(ParseState *pstate,
concurrent = false;
/*
- * Start progress report. If we're building a partition, this was already
- * done.
+ * Start progress report if not done yet.
+ *
+ * We could check parentIndexId and only start the reporting when
+ * processing the parent index (i.e. parentIndexId is invalid), however
+ * that wouldn't work when adding a partition to an existing table:
+ * parentIndexId is always valid in that cases.
*/
- if (!OidIsValid(parentIndexId))
+ if (MyBEEntry->st_progress_command == PROGRESS_COMMAND_INVALID)
{
pgstat_progress_start_command(PROGRESS_COMMAND_CREATE_INDEX, tableId);
pgstat_progress_update_param(PROGRESS_CREATEIDX_COMMAND,
concurrent ?
PROGRESS_CREATEIDX_COMMAND_CREATE_CONCURRENTLY :
PROGRESS_CREATEIDX_COMMAND_CREATE);
+ progress_started = true;
}
/*
@@ -1277,8 +1283,7 @@ DefineIndex(ParseState *pstate,
table_close(rel, NoLock);
- /* If this is the top-level index, we're done */
- if (!OidIsValid(parentIndexId))
+ if (progress_started)
pgstat_progress_end_command();
return address;
@@ -1585,21 +1590,18 @@ DefineIndex(ParseState *pstate,
}
}
- /*
- * Indexes on partitioned tables are not themselves built, so we're
- * done here.
- */
AtEOXact_GUC(false, root_save_nestlevel);
SetUserIdAndSecContext(root_save_userid, root_save_sec_context);
table_close(rel, NoLock);
- if (!OidIsValid(parentIndexId))
- pgstat_progress_end_command();
- else
+ if (OidIsValid(parentIndexId))
{
/* Update progress for an intermediate partitioned index itself */
pgstat_progress_incr_param(PROGRESS_CREATEIDX_PARTITIONS_DONE, 1);
}
+ if (progress_started)
+ pgstat_progress_end_command();
+
return address;
}
@@ -1612,14 +1614,14 @@ DefineIndex(ParseState *pstate,
table_close(rel, NoLock);
/*
- * If this is the top-level index, the command is done overall;
- * otherwise, increment progress to report one child index is done.
+ * Increment progress to report one child index is done.
*/
- if (!OidIsValid(parentIndexId))
- pgstat_progress_end_command();
- else
+ if (OidIsValid(parentIndexId))
pgstat_progress_incr_param(PROGRESS_CREATEIDX_PARTITIONS_DONE, 1);
+ if (progress_started)
+ pgstat_progress_end_command();
+
return address;
}
@@ -1833,7 +1835,8 @@ DefineIndex(ParseState *pstate,
*/
UnlockRelationIdForSession(&heaprelid, ShareUpdateExclusiveLock);
- pgstat_progress_end_command();
+ if (progress_started)
+ pgstat_progress_end_command();
return address;
}
@@ -2987,7 +2990,6 @@ ReindexIndex(const ReindexStmt *stmt, const ReindexParams *params, bool isTopLev
{
ReindexParams newparams = *params;
- newparams.options |= REINDEXOPT_REPORT_PROGRESS;
reindex_index(stmt, indOid, false, persistence, &newparams);
}
}
@@ -3110,7 +3112,6 @@ ReindexTable(const ReindexStmt *stmt, const ReindexParams *params, bool isTopLev
{
ReindexParams newparams = *params;
- newparams.options |= REINDEXOPT_REPORT_PROGRESS;
result = reindex_relation(stmt, heapOid,
REINDEX_REL_PROCESS_TOAST |
REINDEX_REL_CHECK_CONSTRAINTS,
@@ -3535,8 +3536,7 @@ ReindexMultipleInternal(const ReindexStmt *stmt, const List *relids, const Reind
{
ReindexParams newparams = *params;
- newparams.options |=
- REINDEXOPT_REPORT_PROGRESS | REINDEXOPT_MISSING_OK;
+ newparams.options |= REINDEXOPT_MISSING_OK;
reindex_index(stmt, relid, false, relpersistence, &newparams);
PopActiveSnapshot();
/* reindex_index() does the verbose output */
@@ -3546,8 +3546,7 @@ ReindexMultipleInternal(const ReindexStmt *stmt, const List *relids, const Reind
bool result;
ReindexParams newparams = *params;
- newparams.options |=
- REINDEXOPT_REPORT_PROGRESS | REINDEXOPT_MISSING_OK;
+ newparams.options |= REINDEXOPT_MISSING_OK;
result = reindex_relation(stmt, relid,
REINDEX_REL_PROCESS_TOAST |
REINDEX_REL_CHECK_CONSTRAINTS,
@@ -3967,7 +3966,6 @@ ReindexRelationConcurrently(const ReindexStmt *stmt, Oid relationOid, const Rein
elog(ERROR, "cannot reindex a temporary table concurrently");
pgstat_progress_start_command(PROGRESS_COMMAND_CREATE_INDEX, idx->tableId);
-
progress_vals[0] = PROGRESS_CREATEIDX_COMMAND_REINDEX_CONCURRENTLY;
progress_vals[1] = 0; /* initializing */
progress_vals[2] = idx->indexId;
@@ -3991,11 +3989,11 @@ ReindexRelationConcurrently(const ReindexStmt *stmt, Oid relationOid, const Rein
/* Create new index definition based on given index */
newIndexId = index_create_copy(heapRel,
INDEX_CREATE_CONCURRENT |
- INDEX_CREATE_SKIP_BUILD |
- INDEX_CREATE_SUPPRESS_PROGRESS,
+ INDEX_CREATE_SKIP_BUILD,
idx->indexId,
tablespaceid,
concurrentName);
+ pgstat_progress_end_command();
/*
* Now open the relation of the new index, a session-level lock is
@@ -4114,9 +4112,11 @@ ReindexRelationConcurrently(const ReindexStmt *stmt, Oid relationOid, const Rein
* DefineIndex() for more details.
*/
+ pgstat_progress_start_command(PROGRESS_COMMAND_CREATE_INDEX, relationOid);
pgstat_progress_update_param(PROGRESS_CREATEIDX_PHASE,
PROGRESS_CREATEIDX_PHASE_WAIT_1);
WaitForLockersMultiple(lockTags, ShareLock, true);
+ pgstat_progress_end_command();
CommitTransactionCommand();
foreach(lc, newIndexIds)
@@ -4153,6 +4153,7 @@ ReindexRelationConcurrently(const ReindexStmt *stmt, Oid relationOid, const Rein
/* Perform concurrent build of new index */
index_concurrently_build(newidx->tableId, newidx->indexId);
+ pgstat_progress_end_command();
PopActiveSnapshot();
CommitTransactionCommand();
@@ -4173,9 +4174,11 @@ ReindexRelationConcurrently(const ReindexStmt *stmt, Oid relationOid, const Rein
* for more details.
*/
+ pgstat_progress_start_command(PROGRESS_COMMAND_CREATE_INDEX, relationOid);
pgstat_progress_update_param(PROGRESS_CREATEIDX_PHASE,
PROGRESS_CREATEIDX_PHASE_WAIT_2);
WaitForLockersMultiple(lockTags, ShareLock, true);
+ pgstat_progress_end_command();
CommitTransactionCommand();
foreach(lc, newIndexIds)
@@ -4246,6 +4249,7 @@ ReindexRelationConcurrently(const ReindexStmt *stmt, Oid relationOid, const Rein
pgstat_progress_update_param(PROGRESS_CREATEIDX_PHASE,
PROGRESS_CREATEIDX_PHASE_WAIT_3);
WaitForOlderSnapshots(limitXmin, true);
+ pgstat_progress_end_command();
CommitTransactionCommand();
}
@@ -4341,6 +4345,7 @@ ReindexRelationConcurrently(const ReindexStmt *stmt, Oid relationOid, const Rein
*/
INJECTION_POINT("reindex-relation-concurrently-before-set-dead", NULL);
+ pgstat_progress_start_command(PROGRESS_COMMAND_CREATE_INDEX, relationOid);
pgstat_progress_update_param(PROGRESS_CREATEIDX_PHASE,
PROGRESS_CREATEIDX_PHASE_WAIT_4);
WaitForLockersMultiple(lockTags, AccessExclusiveLock, true);
@@ -4386,6 +4391,7 @@ ReindexRelationConcurrently(const ReindexStmt *stmt, Oid relationOid, const Rein
pgstat_progress_update_param(PROGRESS_CREATEIDX_PHASE,
PROGRESS_CREATEIDX_PHASE_WAIT_5);
WaitForLockersMultiple(lockTags, AccessExclusiveLock, true);
+ pgstat_progress_end_command();
PushActiveSnapshot(GetTransactionSnapshot());
@@ -4461,8 +4467,6 @@ ReindexRelationConcurrently(const ReindexStmt *stmt, Oid relationOid, const Rein
MemoryContextDelete(private_context);
- pgstat_progress_end_command();
-
return true;
}
diff --git a/src/backend/commands/repack.c b/src/backend/commands/repack.c
index 4d177c868bb..51078d10202 100644
--- a/src/backend/commands/repack.c
+++ b/src/backend/commands/repack.c
@@ -1920,11 +1920,19 @@ finish_heap_swap(Oid OIDOldHeap, Oid OIDNewHeap,
{
ObjectAddress object;
Oid mapped_tables[4];
+ bool is_repack;
int i;
+ /*
+ * XXX Shouldn't we add an argument indicating that the function is being
+ * called from REPACK?
+ */
+ is_repack = MyBEEntry->st_progress_command == PROGRESS_COMMAND_REPACK;
+
/* Report that we are now swapping relation files */
- pgstat_progress_update_param(PROGRESS_REPACK_PHASE,
- PROGRESS_REPACK_PHASE_SWAP_REL_FILES);
+ if (is_repack)
+ pgstat_progress_update_param(PROGRESS_REPACK_PHASE,
+ PROGRESS_REPACK_PHASE_SWAP_REL_FILES);
/* Zero out possible results from swapped_relation_files */
memset(mapped_tables, 0, sizeof(mapped_tables));
@@ -1981,15 +1989,17 @@ finish_heap_swap(Oid OIDOldHeap, Oid OIDNewHeap,
reindex_flags |= REINDEX_REL_FORCE_INDEXES_PERMANENT;
/* Report that we are now reindexing relations */
- pgstat_progress_update_param(PROGRESS_REPACK_PHASE,
- PROGRESS_REPACK_PHASE_REBUILD_INDEX);
+ if (is_repack)
+ pgstat_progress_update_param(PROGRESS_REPACK_PHASE,
+ PROGRESS_REPACK_PHASE_REBUILD_INDEX);
reindex_relation(NULL, OIDOldHeap, reindex_flags, &reindex_params);
}
/* Report that we are now doing clean up */
- pgstat_progress_update_param(PROGRESS_REPACK_PHASE,
- PROGRESS_REPACK_PHASE_FINAL_CLEANUP);
+ if (is_repack)
+ pgstat_progress_update_param(PROGRESS_REPACK_PHASE,
+ PROGRESS_REPACK_PHASE_FINAL_CLEANUP);
/*
* If the relation being rebuilt is pg_class, swap_relation_files()
@@ -3348,9 +3358,20 @@ build_new_indexes(Relation NewHeap, Relation OldHeap, List *OldIndexes)
"repacknew",
get_rel_namespace(ind->rd_index->indrelid),
false);
- newindex = index_create_copy(NewHeap, INDEX_CREATE_SUPPRESS_PROGRESS,
+
+ /*
+ * We build the index on the new heap, but after the swap phase it'll
+ * become an index on the old heap. It makes more sense to report the
+ * progress this way. (The reporting API expects that both command and
+ * subcommand deal with the same target.)
+ */
+ pgstat_progress_start_command(PROGRESS_COMMAND_CREATE_INDEX,
+ RelationGetRelid(OldHeap));
+ newindex = index_create_copy(NewHeap, 0,
oldindex, ind->rd_rel->reltablespace,
newName);
+ pgstat_progress_end_command();
+
copy_index_constraints(ind, newindex, RelationGetRelid(NewHeap));
result = lappend_oid(result, newindex);
diff --git a/src/backend/utils/activity/backend_progress.c b/src/backend/utils/activity/backend_progress.c
index 6d2049105ab..ce7ca533627 100644
--- a/src/backend/utils/activity/backend_progress.c
+++ b/src/backend/utils/activity/backend_progress.c
@@ -22,6 +22,10 @@
*
* Set st_progress_command (and st_progress_command_target) in own backend
* entry. Also, zero-initialize st_progress_param array.
+ *
+ * If command has already been started, start a sub-command. Only parameters
+ * of the sub-command are updated until pgstat_progress_end_command() is
+ * called. (Target relation must be the same for both commands.)
*-----------
*/
void
@@ -33,9 +37,30 @@ pgstat_progress_start_command(ProgressCommandType cmdtype, Oid relid)
return;
PGSTAT_BEGIN_WRITE_ACTIVITY(beentry);
- beentry->st_progress_command = cmdtype;
- beentry->st_progress_command_target = relid;
- MemSet(&beentry->st_progress_param, 0, sizeof(beentry->st_progress_param));
+ /* Sub-command should not be started w/o parent command. */
+ if (beentry->st_progress_command == PROGRESS_COMMAND_INVALID)
+ {
+ Assert(beentry->st_progress_command2 == PROGRESS_COMMAND_INVALID);
+
+ beentry->st_progress_command = cmdtype;
+ beentry->st_progress_command_target = relid;
+ MemSet(&beentry->st_progress_param, 0,
+ sizeof(beentry->st_progress_param));
+ }
+ else if (beentry->st_progress_command2 == PROGRESS_COMMAND_INVALID)
+ {
+ Assert(beentry->st_progress_command != PROGRESS_COMMAND_INVALID);
+
+ beentry->st_progress_command2 = cmdtype;
+ beentry->st_progress_command_target2 = relid;
+ MemSet(&beentry->st_progress_param2, 0,
+ sizeof(beentry->st_progress_param2));
+ }
+ else
+ {
+ /* Only one level of nesting is supported. */
+ Assert(false);
+ }
PGSTAT_END_WRITE_ACTIVITY(beentry);
}
@@ -49,14 +74,21 @@ void
pgstat_progress_update_param(int index, int64 val)
{
volatile PgBackendStatus *beentry = MyBEEntry;
+ volatile int64 *params;
Assert(index >= 0 && index < PGSTAT_NUM_PROGRESS_PARAM);
if (!beentry || !pgstat_track_activities)
return;
+ Assert(beentry->st_progress_command != PROGRESS_COMMAND_INVALID ||
+ beentry->st_progress_command2 != PROGRESS_COMMAND_INVALID);
+
+ params = (beentry->st_progress_command2 == PROGRESS_COMMAND_INVALID) ?
+ beentry->st_progress_param : beentry->st_progress_param2;
+
PGSTAT_BEGIN_WRITE_ACTIVITY(beentry);
- beentry->st_progress_param[index] = val;
+ params[index] = val;
PGSTAT_END_WRITE_ACTIVITY(beentry);
}
@@ -70,14 +102,21 @@ void
pgstat_progress_incr_param(int index, int64 incr)
{
volatile PgBackendStatus *beentry = MyBEEntry;
+ volatile int64 *params;
Assert(index >= 0 && index < PGSTAT_NUM_PROGRESS_PARAM);
if (!beentry || !pgstat_track_activities)
return;
+ Assert(beentry->st_progress_command != PROGRESS_COMMAND_INVALID ||
+ beentry->st_progress_command2 != PROGRESS_COMMAND_INVALID);
+
+ params = (beentry->st_progress_command2 == PROGRESS_COMMAND_INVALID) ?
+ beentry->st_progress_param : beentry->st_progress_param2;
+
PGSTAT_BEGIN_WRITE_ACTIVITY(beentry);
- beentry->st_progress_param[index] += incr;
+ params[index] += incr;
PGSTAT_END_WRITE_ACTIVITY(beentry);
}
@@ -122,17 +161,24 @@ pgstat_progress_update_multi_param(int nparam, const int *index,
{
volatile PgBackendStatus *beentry = MyBEEntry;
int i;
+ volatile int64 *params;
if (!beentry || !pgstat_track_activities || nparam == 0)
return;
+ Assert(beentry->st_progress_command != PROGRESS_COMMAND_INVALID ||
+ beentry->st_progress_command2 != PROGRESS_COMMAND_INVALID);
+
+ params = (beentry->st_progress_command2 == PROGRESS_COMMAND_INVALID) ?
+ beentry->st_progress_param : beentry->st_progress_param2;
+
PGSTAT_BEGIN_WRITE_ACTIVITY(beentry);
for (i = 0; i < nparam; ++i)
{
Assert(index[i] >= 0 && index[i] < PGSTAT_NUM_PROGRESS_PARAM);
- beentry->st_progress_param[index[i]] = val[i];
+ params[index[i]] = val[i];
}
PGSTAT_END_WRITE_ACTIVITY(beentry);
@@ -142,7 +188,7 @@ pgstat_progress_update_multi_param(int nparam, const int *index,
* pgstat_progress_end_command() -
*
* Reset st_progress_command (and st_progress_command_target) in own backend
- * entry. This signals the end of the command.
+ * entry. This signals the end of the command (or a sub-command).
*-----------
*/
void
@@ -153,11 +199,20 @@ pgstat_progress_end_command(void)
if (!beentry || !pgstat_track_activities)
return;
- if (beentry->st_progress_command == PROGRESS_COMMAND_INVALID)
- return;
-
PGSTAT_BEGIN_WRITE_ACTIVITY(beentry);
- beentry->st_progress_command = PROGRESS_COMMAND_INVALID;
- beentry->st_progress_command_target = InvalidOid;
+
+ if (beentry->st_progress_command2 != PROGRESS_COMMAND_INVALID)
+ {
+ Assert(beentry->st_progress_command != PROGRESS_COMMAND_INVALID);
+
+ beentry->st_progress_command2 = PROGRESS_COMMAND_INVALID;
+ beentry->st_progress_command_target2 = InvalidOid;
+ }
+ else
+ {
+ beentry->st_progress_command = PROGRESS_COMMAND_INVALID;
+ beentry->st_progress_command_target = InvalidOid;
+ }
+
PGSTAT_END_WRITE_ACTIVITY(beentry);
}
diff --git a/src/backend/utils/activity/backend_status.c b/src/backend/utils/activity/backend_status.c
index d685fc5cd87..15675992415 100644
--- a/src/backend/utils/activity/backend_status.c
+++ b/src/backend/utils/activity/backend_status.c
@@ -284,6 +284,7 @@ pgstat_bestart_initial(void)
lbeentry.st_state = STATE_STARTING;
lbeentry.st_progress_command = PROGRESS_COMMAND_INVALID;
+ lbeentry.st_progress_command2 = PROGRESS_COMMAND_INVALID;
lbeentry.st_progress_command_target = InvalidOid;
lbeentry.st_query_id = INT64CONST(0);
lbeentry.st_plan_id = INT64CONST(0);
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index 6f9c9c72de5..d0513d6931b 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -314,6 +314,7 @@ pg_stat_get_progress_info(PG_FUNCTION_ARGS)
Datum values[PG_STAT_GET_PROGRESS_COLS] = {0};
bool nulls[PG_STAT_GET_PROGRESS_COLS] = {0};
int i;
+ volatile int64 *params;
local_beentry = pgstat_get_local_beentry_by_index(curr_backend);
beentry = &local_beentry->backendStatus;
@@ -322,7 +323,11 @@ pg_stat_get_progress_info(PG_FUNCTION_ARGS)
* Report values for only those backends which are running the given
* command.
*/
- if (beentry->st_progress_command != cmdtype)
+ if (beentry->st_progress_command == cmdtype)
+ params = beentry->st_progress_param;
+ else if (beentry->st_progress_command2 == cmdtype)
+ params = beentry->st_progress_param2;
+ else
continue;
/* Value available to all callers */
@@ -332,9 +337,13 @@ pg_stat_get_progress_info(PG_FUNCTION_ARGS)
/* show rest of the values including relid only to role members */
if (HAS_PGSTAT_PERMISSIONS(beentry->st_userid))
{
- values[2] = ObjectIdGetDatum(beentry->st_progress_command_target);
+ if (beentry->st_progress_command == cmdtype)
+ values[2] = ObjectIdGetDatum(beentry->st_progress_command_target);
+ else if (beentry->st_progress_command2 == cmdtype)
+ values[2] = ObjectIdGetDatum(beentry->st_progress_command_target2);
+
for (i = 0; i < PGSTAT_NUM_PROGRESS_PARAM; i++)
- values[i + 3] = Int64GetDatum(beentry->st_progress_param[i]);
+ values[i + 3] = Int64GetDatum(params[i]);
}
else
{
diff --git a/src/include/catalog/index.h b/src/include/catalog/index.h
index 9aee8226347..d4c77f3df3f 100644
--- a/src/include/catalog/index.h
+++ b/src/include/catalog/index.h
@@ -45,9 +45,8 @@ typedef struct ReindexParams
/* flag bits for ReindexParams->flags */
#define REINDEXOPT_VERBOSE 0x01 /* print progress info */
-#define REINDEXOPT_REPORT_PROGRESS 0x02 /* report pgstat progress */
-#define REINDEXOPT_MISSING_OK 0x04 /* skip missing relations */
-#define REINDEXOPT_CONCURRENTLY 0x08 /* concurrent mode */
+#define REINDEXOPT_MISSING_OK 0x02 /* skip missing relations */
+#define REINDEXOPT_CONCURRENTLY 0x04 /* concurrent mode */
/* state info for validate_index bulkdelete callback */
typedef struct ValidateIndexState
@@ -71,7 +70,6 @@ extern void index_check_primary_key(Relation heapRel,
#define INDEX_CREATE_IF_NOT_EXISTS (1 << 4)
#define INDEX_CREATE_PARTITIONED (1 << 5)
#define INDEX_CREATE_INVALID (1 << 6)
-#define INDEX_CREATE_SUPPRESS_PROGRESS (1 << 7)
extern Oid index_create(Relation heapRelation,
const char *indexRelationName,
@@ -149,8 +147,7 @@ extern void index_build(Relation heapRelation,
Relation indexRelation,
IndexInfo *indexInfo,
bool isreindex,
- bool parallel,
- bool progress);
+ bool parallel);
extern void validate_index(Oid heapId, Oid indexId, Snapshot snapshot);
diff --git a/src/include/utils/backend_status.h b/src/include/utils/backend_status.h
index a334e096e4a..8b39c59df7b 100644
--- a/src/include/utils/backend_status.h
+++ b/src/include/utils/backend_status.h
@@ -169,6 +169,15 @@ typedef struct PgBackendStatus
Oid st_progress_command_target;
int64 st_progress_param[PGSTAT_NUM_PROGRESS_PARAM];
+ /*
+ * Some commands have a sub-command, e.g. REPACK (re)builds indexes. The
+ * target can be different, e.g. when the sub-command builds an index on
+ * TOAST relation.
+ */
+ ProgressCommandType st_progress_command2;
+ Oid st_progress_command_target2;
+ int64 st_progress_param2[PGSTAT_NUM_PROGRESS_PARAM];
+
/* query identifier, optionally computed using post_parse_analyze_hook */
int64 st_query_id;
@@ -176,6 +185,16 @@ typedef struct PgBackendStatus
int64 st_plan_id;
} PgBackendStatus;
+/*
+ * Check the depth of progress tracking.
+ *
+ * 0 - tracking disabled
+ * 1 - enabled for the "main" command
+ * 2 - enabled for both main command and its sub-command.
+ */
+#define PGSTAT_PROGRESS_STATE(state) \
+ ((state)->st_progress_command == PROGRESS_COMMAND_INVALID ? 0 :\
+ (state)->st_progress_command2 == PROGRESS_COMMAND_INVALID ? 1 : 2)
/*
* Macros to load and store st_changecount with appropriate memory barriers.
--
2.52.0
--=-=-=--
view thread (34+ messages) latest in thread
reply
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: [email protected]
Cc: [email protected]
Subject: Re: [PATCH] Allow progress tracking of sub-commands.
In-Reply-To: <no-message-id-661718@localhost>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox