From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: Pg Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: monitoring CREATE INDEX [CONCURRENTLY]
Date: Mon, 31 Dec 2018 21:38:58 -0300
Message-ID: <201901010038.dyhveufqcxc3@alvherre.pgsql> (raw)
In-Reply-To: <20181220220022.mg63bhk26zdpvmcj@alvherre.pgsql>
For discussion, here's an preliminary patch. This is just a first
skeleton; needs to grow a lot of flesh yet, per my previous proposal.
As far as the generic CREATE INDEX stuff goes, I think this is complete;
it's missing the AM-specific bits.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
[text/x-diff] create-index-progress.patch (13.9K, ../201901010038.dyhveufqcxc3@alvherre.pgsql/2-create-index-progress.patch)
download | inline diff:diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.cindex c2ad944e04..f4cb28c6d6 100644--- a/src/backend/catalog/index.c+++ b/src/backend/catalog/index.c@@ -1590,7 +1590,7 @@ index_drop(Oid indexId, bool concurrent)
* to acquire an exclusive lock on our table. The lock code will
* detect deadlock and error out properly.
*/
- WaitForLockers(heaplocktag, AccessExclusiveLock);+ WaitForLockers(heaplocktag, AccessExclusiveLock, true);
/*
* No more predicate locks will be acquired on this index, and we're
@@ -1634,7 +1634,7 @@ index_drop(Oid indexId, bool concurrent)
* Wait till every transaction that saw the old index state has
* finished.
*/
- WaitForLockers(heaplocktag, AccessExclusiveLock);+ WaitForLockers(heaplocktag, AccessExclusiveLock, true);
/*
* Re-open relations to allow us to complete our actions.
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sqlindex 5253837b54..921a84eb45 100644--- a/src/backend/catalog/system_views.sql+++ b/src/backend/catalog/system_views.sql@@ -904,6 +904,24 @@ CREATE VIEW pg_stat_progress_vacuum AS
FROM pg_stat_get_progress_info('VACUUM') AS S
LEFT JOIN pg_database D ON S.datid = D.oid;
+CREATE VIEW pg_stat_progress_create_index AS+ SELECT+ s.pid AS pid, S.datid AS datid, D.datname AS datname,+ S.relid AS relid,+ CASE s.param1 WHEN 0 THEN 'initializing'+ WHEN 1 THEN 'waiting for lockers 1'+ WHEN 2 THEN 'building index'+ WHEN 3 THEN 'waiting for lockers 2'+ WHEN 4 THEN 'validating index'+ WHEN 5 THEN 'waiting for lockers 3'+ END as phase,+ S.param2 AS procs_to_wait_for,+ S.param3 AS procs_waited_for,+ S.param4 AS partitions_to_build,+ S.param5 AS partitions_built+ FROM pg_stat_get_progress_info('CREATE INDEX') AS S+ LEFT JOIN pg_database D ON S.datid = D.oid;+
CREATE VIEW pg_user_mappings AS
SELECT
U.oid AS umid,
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.cindex 816c73a36a..03321c2dc4 100644--- a/src/backend/commands/indexcmds.c+++ b/src/backend/commands/indexcmds.c@@ -35,6 +35,7 @@
#include "commands/dbcommands.h"
#include "commands/defrem.h"
#include "commands/event_trigger.h"
+#include "commands/progress.h"
#include "commands/tablecmds.h"
#include "commands/tablespace.h"
#include "mb/pg_wchar.h"
@@ -47,6 +48,7 @@
#include "parser/parse_coerce.h"
#include "parser/parse_func.h"
#include "parser/parse_oper.h"
+#include "pgstat.h"
#include "rewrite/rewriteManip.h"
#include "storage/lmgr.h"
#include "storage/proc.h"
@@ -370,6 +372,15 @@ DefineIndex(Oid relationId,
Snapshot snapshot;
int i;
++ /*+ * Start progress report. If we're building a partition, this was already+ * done.+ */+ if (!OidIsValid(parentIndexId))+ pgstat_progress_start_command(PROGRESS_COMMAND_CREATE_INDEX,+ relationId);+
/*
* count key attributes in index
*/
@@ -866,6 +877,11 @@ DefineIndex(Oid relationId,
if (!OidIsValid(indexRelationId))
{
heap_close(rel, NoLock);
++ /* If this is the top-level index, we're done */+ if (!OidIsValid(parentIndexId))+ pgstat_progress_end_command();+
return address;
}
@@ -891,6 +907,9 @@ DefineIndex(Oid relationId,
TupleDesc parentDesc;
Oid *opfamOids;
+ pgstat_progress_update_param(PROGRESS_CREATEIDX_PARTITIONS_TOTAL,+ nparts);+
memcpy(part_oids, partdesc->oids, sizeof(Oid) * nparts);
parentDesc = CreateTupleDescCopy(RelationGetDescr(rel));
@@ -1040,6 +1059,8 @@ DefineIndex(Oid relationId,
skip_build, quiet);
}
+ pgstat_progress_update_param(PROGRESS_CREATEIDX_PARTITIONS_DONE,+ i + 1);
pfree(attmap);
}
@@ -1074,6 +1095,8 @@ DefineIndex(Oid relationId,
* Indexes on partitioned tables are not themselves built, so we're
* done here.
*/
+ if (!OidIsValid(parentIndexId))+ pgstat_progress_end_command();
return address;
}
@@ -1081,6 +1104,11 @@ DefineIndex(Oid relationId,
{
/* Close the heap and we're done, in the non-concurrent case */
heap_close(rel, NoLock);
++ /* If this is the top-level index, we're done. */+ if (!OidIsValid(parentIndexId))+ pgstat_progress_end_command();+
return address;
}
@@ -1132,7 +1160,9 @@ DefineIndex(Oid relationId,
* exclusive lock on our table. The lock code will detect deadlock and
* error out properly.
*/
- WaitForLockers(heaplocktag, ShareLock);+ pgstat_progress_update_param(PROGRESS_CREATEIDX_PHASE,+ PROGRESS_CREATEIDX_PHASE_WAIT_1);+ WaitForLockers(heaplocktag, ShareLock, true);
/*
* At this moment we are sure that there are no transactions with the
@@ -1168,6 +1198,8 @@ DefineIndex(Oid relationId,
indexInfo->ii_BrokenHotChain = false;
/* Now build the index */
+ pgstat_progress_update_param(PROGRESS_CREATEIDX_PHASE,+ PROGRESS_CREATEIDX_PHASE_BUILD);
index_build(rel, indexRelation, indexInfo, stmt->primary, false, true);
/* Close both the relations, but keep the locks */
@@ -1196,7 +1228,9 @@ DefineIndex(Oid relationId,
* We once again wait until no transaction can have the table open with
* the index marked as read-only for updates.
*/
- WaitForLockers(heaplocktag, ShareLock);+ pgstat_progress_update_param(PROGRESS_CREATEIDX_PHASE,+ PROGRESS_CREATEIDX_PHASE_WAIT_2);+ WaitForLockers(heaplocktag, ShareLock, true);
/*
* Now take the "reference snapshot" that will be used by validate_index()
@@ -1219,6 +1253,8 @@ DefineIndex(Oid relationId,
/*
* Scan the index and the heap, insert any missing index entries.
*/
+ pgstat_progress_update_param(PROGRESS_CREATEIDX_PHASE,+ PROGRESS_CREATEIDX_PHASE_VALIDATE);
validate_index(relationId, indexRelationId, snapshot);
/*
@@ -1282,6 +1318,9 @@ DefineIndex(Oid relationId,
old_snapshots = GetCurrentVirtualXIDs(limitXmin, true, false,
PROC_IS_AUTOVACUUM | PROC_IN_VACUUM,
&n_old_snapshots);
+ pgstat_progress_update_param(PROGRESS_CREATEIDX_PHASE,+ PROGRESS_CREATEIDX_PHASE_WAIT_3);+ pgstat_progress_update_param(PROGRESS_WAITFOR_TOTAL, n_old_snapshots);
for (i = 0; i < n_old_snapshots; i++)
{
@@ -1318,6 +1357,8 @@ DefineIndex(Oid relationId,
if (VirtualTransactionIdIsValid(old_snapshots[i]))
VirtualXactLock(old_snapshots[i], true);
++ pgstat_progress_update_param(PROGRESS_WAITFOR_DONE, i);
}
/*
@@ -1340,6 +1381,8 @@ DefineIndex(Oid relationId,
*/
UnlockRelationIdForSession(&heaprelid, ShareUpdateExclusiveLock);
+ pgstat_progress_end_command();+
return address;
}
diff --git a/src/backend/storage/ipc/standby.c b/src/backend/storage/ipc/standby.cindex c9bb3e987d..f151a07ec1 100644--- a/src/backend/storage/ipc/standby.c+++ b/src/backend/storage/ipc/standby.c@@ -401,7 +401,7 @@ ResolveRecoveryConflictWithLock(LOCKTAG locktag)
*/
VirtualTransactionId *backends;
- backends = GetLockConflicts(&locktag, AccessExclusiveLock);+ backends = GetLockConflicts(&locktag, AccessExclusiveLock, NULL);
ResolveRecoveryConflictWithVirtualXIDs(backends,
PROCSIG_RECOVERY_CONFLICT_LOCK);
}
diff --git a/src/backend/storage/lmgr/lmgr.c b/src/backend/storage/lmgr/lmgr.cindex 3f57507bce..144ccd7ffd 100644--- a/src/backend/storage/lmgr/lmgr.c+++ b/src/backend/storage/lmgr/lmgr.c@@ -19,7 +19,9 @@
#include "access/transam.h"
#include "access/xact.h"
#include "catalog/catalog.h"
+#include "commands/progress.h"
#include "miscadmin.h"
+#include "pgstat.h"
#include "storage/lmgr.h"
#include "storage/procarray.h"
#include "utils/inval.h"
@@ -857,10 +859,12 @@ XactLockTableWaitErrorCb(void *arg)
* after we obtained our initial list of lockers, we will not wait for them.
*/
void
-WaitForLockersMultiple(List *locktags, LOCKMODE lockmode)+WaitForLockersMultiple(List *locktags, LOCKMODE lockmode, bool progress)
{
List *holders = NIL;
ListCell *lc;
+ int total = 0;+ int done = 0;
/* Done if no locks to wait for */
if (list_length(locktags) == 0)
@@ -870,10 +874,16 @@ WaitForLockersMultiple(List *locktags, LOCKMODE lockmode)
foreach(lc, locktags)
{
LOCKTAG *locktag = lfirst(lc);
+ int count;- holders = lappend(holders, GetLockConflicts(locktag, lockmode));+ holders = lappend(holders,+ GetLockConflicts(locktag, lockmode, &count));+ total += count;
}
+ if (progress)+ pgstat_progress_update_param(PROGRESS_WAITFOR_TOTAL, total);+
/*
* Note: GetLockConflicts() never reports our own xid, hence we need not
* check for that. Also, prepared xacts are not reported, which is fine
@@ -889,6 +899,9 @@ WaitForLockersMultiple(List *locktags, LOCKMODE lockmode)
{
VirtualXactLock(*lockholders, true);
lockholders++;
++ if (progress)+ pgstat_progress_update_param(PROGRESS_WAITFOR_DONE, ++done);
}
}
@@ -901,12 +914,12 @@ WaitForLockersMultiple(List *locktags, LOCKMODE lockmode)
* Same as WaitForLockersMultiple, for a single lock tag.
*/
void
-WaitForLockers(LOCKTAG heaplocktag, LOCKMODE lockmode)+WaitForLockers(LOCKTAG heaplocktag, LOCKMODE lockmode, bool progress)
{
List *l;
l = list_make1(&heaplocktag);
- WaitForLockersMultiple(l, lockmode);+ WaitForLockersMultiple(l, lockmode, progress);
list_free(l);
}
diff --git a/src/backend/storage/lmgr/lock.c b/src/backend/storage/lmgr/lock.cindex 10f6f60f1e..3034fb3f27 100644--- a/src/backend/storage/lmgr/lock.c+++ b/src/backend/storage/lmgr/lock.c@@ -2807,6 +2807,7 @@ FastPathGetRelationLockEntry(LOCALLOCK *locallock)
* xacts merely awaiting such a lock are NOT reported.
*
* The result array is palloc'd and is terminated with an invalid VXID.
+ * *ocount, if not null, is updated to the number of items set.
*
* Of course, the result could be out of date by the time it's returned,
* so use of this function has to be thought about carefully.
@@ -2817,7 +2818,7 @@ FastPathGetRelationLockEntry(LOCALLOCK *locallock)
* uses of the result.
*/
VirtualTransactionId *
-GetLockConflicts(const LOCKTAG *locktag, LOCKMODE lockmode)+GetLockConflicts(const LOCKTAG *locktag, LOCKMODE lockmode, int *ocount)
{
static VirtualTransactionId *vxids;
LOCKMETHODID lockmethodid = locktag->locktag_lockmethodid;
@@ -2964,6 +2965,8 @@ GetLockConflicts(const LOCKTAG *locktag, LOCKMODE lockmode)
LWLockRelease(partitionLock);
vxids[count].backendId = InvalidBackendId;
vxids[count].localTransactionId = InvalidLocalTransactionId;
+ if (ocount)+ *ocount = count;
return vxids;
}
@@ -3019,6 +3022,8 @@ GetLockConflicts(const LOCKTAG *locktag, LOCKMODE lockmode)
vxids[count].backendId = InvalidBackendId;
vxids[count].localTransactionId = InvalidLocalTransactionId;
+ if (ocount)+ *ocount = count;
return vxids;
}
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.cindex f955f1912a..7c93efb362 100644--- a/src/backend/utils/adt/pgstatfuncs.c+++ b/src/backend/utils/adt/pgstatfuncs.c@@ -468,6 +468,8 @@ pg_stat_get_progress_info(PG_FUNCTION_ARGS)
/* Translate command name into command type code. */
if (pg_strcasecmp(cmd, "VACUUM") == 0)
cmdtype = PROGRESS_COMMAND_VACUUM;
+ else if (pg_strcasecmp(cmd, "CREATE INDEX") == 0)+ cmdtype = PROGRESS_COMMAND_CREATE_INDEX;
else
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
diff --git a/src/include/commands/progress.h b/src/include/commands/progress.hindex 6a6b467fee..f02fedf3ad 100644--- a/src/include/commands/progress.h+++ b/src/include/commands/progress.h@@ -34,4 +34,22 @@
#define PROGRESS_VACUUM_PHASE_TRUNCATE 5
#define PROGRESS_VACUUM_PHASE_FINAL_CLEANUP 6
++/* Progress parameters for CREATE INDEX */+#define PROGRESS_CREATEIDX_PHASE 0+/* 1 and 2 reserved for "waitfor" metrics */+#define PROGRESS_CREATEIDX_PARTITIONS_TOTAL 3+#define PROGRESS_CREATEIDX_PARTITIONS_DONE 4++/* Phases of CREATE INDEX */+#define PROGRESS_CREATEIDX_PHASE_WAIT_1 1+#define PROGRESS_CREATEIDX_PHASE_BUILD 2+#define PROGRESS_CREATEIDX_PHASE_WAIT_2 3+#define PROGRESS_CREATEIDX_PHASE_VALIDATE 4+#define PROGRESS_CREATEIDX_PHASE_WAIT_3 5++/* Lock holder wait counts */+#define PROGRESS_WAITFOR_TOTAL 1+#define PROGRESS_WAITFOR_DONE 2+
#endif
diff --git a/src/include/pgstat.h b/src/include/pgstat.hindex f1c10d16b8..9542bf024f 100644--- a/src/include/pgstat.h+++ b/src/include/pgstat.h@@ -934,7 +934,8 @@ typedef enum
typedef enum ProgressCommandType
{
PROGRESS_COMMAND_INVALID,
- PROGRESS_COMMAND_VACUUM+ PROGRESS_COMMAND_VACUUM,+ PROGRESS_COMMAND_CREATE_INDEX
} ProgressCommandType;
#define PGSTAT_NUM_PROGRESS_PARAM 10
diff --git a/src/include/storage/lmgr.h b/src/include/storage/lmgr.hindex e5356b7d54..ee1070f321 100644--- a/src/include/storage/lmgr.h+++ b/src/include/storage/lmgr.h@@ -78,8 +78,8 @@ extern void XactLockTableWait(TransactionId xid, Relation rel,
extern bool ConditionalXactLockTableWait(TransactionId xid);
/* Lock VXIDs, specified by conflicting locktags */
-extern void WaitForLockers(LOCKTAG heaplocktag, LOCKMODE lockmode);-extern void WaitForLockersMultiple(List *locktags, LOCKMODE lockmode);+extern void WaitForLockers(LOCKTAG heaplocktag, LOCKMODE lockmode, bool progress);+extern void WaitForLockersMultiple(List *locktags, LOCKMODE lockmode, bool progress);
/* Lock an XID for tuple insertion (used to wait for an insertion to finish) */
extern uint32 SpeculativeInsertionLockAcquire(TransactionId xid);
diff --git a/src/include/storage/lock.h b/src/include/storage/lock.hindex a37fda7b63..a0bfc670b6 100644--- a/src/include/storage/lock.h+++ b/src/include/storage/lock.h@@ -544,7 +544,7 @@ extern bool LockHeldByMe(const LOCKTAG *locktag, LOCKMODE lockmode);
extern bool LockHasWaiters(const LOCKTAG *locktag,
LOCKMODE lockmode, bool sessionLock);
extern VirtualTransactionId *GetLockConflicts(const LOCKTAG *locktag,
- LOCKMODE lockmode);+ LOCKMODE lockmode, int *ocount);
extern void AtPrepare_Locks(void);
extern void PostPrepare_Locks(TransactionId xid);
extern int LockCheckConflicts(LockMethod lockMethodTable,
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@2ndquadrant.com, pgsql-hackers@lists.postgresql.org
Subject: Re: monitoring CREATE INDEX [CONCURRENTLY]
In-Reply-To: <201901010038.dyhveufqcxc3@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