From 29d4c1bdf536222c1e86c7502bbbf3fd49751a90 Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Tue, 8 Sep 2020 20:47:38 -0700
Subject: [PATCH v1 1/3] WIP: Add INTERRUPTIBLE option to VACUUM & ANALYZE.

Partially because that's practically useful, partially because it's
useful for testing.

There's a few XXXs in the code, and there's a comment or two I have
intentionally not yet rewrapped.
---
 src/include/commands/vacuum.h       |  1 +
 src/include/storage/lock.h          |  6 ++--
 src/include/storage/proc.h          |  5 +--
 src/backend/commands/vacuum.c       | 14 +++++---
 src/backend/postmaster/autovacuum.c |  2 ++
 src/backend/storage/lmgr/deadlock.c | 54 ++++++++++++++++-------------
 src/backend/storage/lmgr/proc.c     | 24 +++++++------
 doc/src/sgml/ref/analyze.sgml       | 16 +++++++++
 doc/src/sgml/ref/vacuum.sgml        | 17 +++++++++
 9 files changed, 94 insertions(+), 45 deletions(-)

diff --git a/src/include/commands/vacuum.h b/src/include/commands/vacuum.h
index d9475c99890..7e064ef45e9 100644
--- a/src/include/commands/vacuum.h
+++ b/src/include/commands/vacuum.h
@@ -215,6 +215,7 @@ typedef struct VacuumParams
 	int			multixact_freeze_table_age; /* multixact age at which to scan
 											 * whole table */
 	bool		is_wraparound;	/* force a for-wraparound vacuum */
+	bool		is_interruptible;	/* allow cancelling on lock conflict */
 	int			log_min_duration;	/* minimum execution threshold in ms at
 									 * which  verbose logs are activated, -1
 									 * to use default */
diff --git a/src/include/storage/lock.h b/src/include/storage/lock.h
index 1c3e9c1999f..f21f9f3f974 100644
--- a/src/include/storage/lock.h
+++ b/src/include/storage/lock.h
@@ -499,8 +499,8 @@ typedef enum
 	DS_NO_DEADLOCK,				/* no deadlock detected */
 	DS_SOFT_DEADLOCK,			/* deadlock avoided by queue rearrangement */
 	DS_HARD_DEADLOCK,			/* deadlock, no way out but ERROR */
-	DS_BLOCKED_BY_AUTOVACUUM	/* no deadlock; queue blocked by autovacuum
-								 * worker */
+	DS_BLOCKED_BY_INTERRUPTIBLE	/* no deadlock; queue blocked by interruptible
+								 * task (e.g. autovacuum worker) */
 } DeadLockState;
 
 /*
@@ -588,7 +588,7 @@ extern void lock_twophase_standby_recover(TransactionId xid, uint16 info,
 										  void *recdata, uint32 len);
 
 extern DeadLockState DeadLockCheck(PGPROC *proc);
-extern PGPROC *GetBlockingAutoVacuumPgproc(void);
+extern PGPROC *GetBlockingInterruptiblePgproc(void);
 extern void DeadLockReport(void) pg_attribute_noreturn();
 extern void RememberSimpleDeadLock(PGPROC *proc1,
 								   LOCKMODE lockmode,
diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h
index 9c9a50ae457..e660972a010 100644
--- a/src/include/storage/proc.h
+++ b/src/include/storage/proc.h
@@ -53,13 +53,14 @@ struct XidCache
  */
 #define		PROC_IS_AUTOVACUUM	0x01	/* is it an autovac worker? */
 #define		PROC_IN_VACUUM		0x02	/* currently running lazy vacuum */
-#define		PROC_VACUUM_FOR_WRAPAROUND	0x08	/* set by autovac only */
+#define		PROC_IS_INTERRUPTIBLE	0x08	/* vacuum / analyze may be interrupted
+										 * when locks conflict */
 #define		PROC_IN_LOGICAL_DECODING	0x10	/* currently doing logical
 												 * decoding outside xact */
 
 /* flags reset at EOXact */
 #define		PROC_VACUUM_STATE_MASK \
-	(PROC_IN_VACUUM | PROC_VACUUM_FOR_WRAPAROUND)
+	(PROC_IN_VACUUM | PROC_IS_INTERRUPTIBLE)
 
 /*
  * We allow a small number of "weak" relation locks (AccessShareLock,
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index ddeec870d81..df717d2951a 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -123,6 +123,8 @@ ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel)
 			verbose = defGetBoolean(opt);
 		else if (strcmp(opt->defname, "skip_locked") == 0)
 			skip_locked = defGetBoolean(opt);
+		else if (strcmp(opt->defname, "interruptible") == 0)
+			params.is_interruptible = defGetBoolean(opt);
 		else if (!vacstmt->is_vacuumcmd)
 			ereport(ERROR,
 					(errcode(ERRCODE_SYNTAX_ERROR),
@@ -1754,19 +1756,21 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params)
 		 * contents of other tables is arguably broken, but we won't break it
 		 * here by violating transaction semantics.)
 		 *
-		 * We also set the VACUUM_FOR_WRAPAROUND flag, which is passed down by
-		 * autovacuum; it's used to avoid canceling a vacuum that was invoked
-		 * in an emergency.
+		 * We also set the INTERRUPTIBLE flag when user indicated or when
+		 * started by autovacuum (except for anti-wraparound autovacuum, which
+		 * we do not want to cancel), that governs whether the process may be
+		 * cancelled upon a lock conflict.
 		 *
 		 * Note: these flags remain set until CommitTransaction or
 		 * AbortTransaction.  We don't want to clear them until we reset
 		 * MyProc->xid/xmin, otherwise GetOldestNonRemovableTransactionId()
 		 * might appear to go backwards, which is probably Not Good.
 		 */
+		Assert(!params->is_wraparound || !params->is_interruptible);
 		LWLockAcquire(ProcArrayLock, LW_EXCLUSIVE);
 		MyProc->vacuumFlags |= PROC_IN_VACUUM;
-		if (params->is_wraparound)
-			MyProc->vacuumFlags |= PROC_VACUUM_FOR_WRAPAROUND;
+		if (params->is_interruptible)
+			MyProc->vacuumFlags |= PROC_IS_INTERRUPTIBLE;
 		ProcGlobal->vacuumFlags[MyProc->pgxactoff] = MyProc->vacuumFlags;
 		LWLockRelease(ProcArrayLock);
 	}
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index 1b8cd7bacd4..9fdc3f8ade1 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -2899,6 +2899,8 @@ table_recheck_autovac(Oid relid, HTAB *table_toast_map,
 		tab->at_params.multixact_freeze_min_age = multixact_freeze_min_age;
 		tab->at_params.multixact_freeze_table_age = multixact_freeze_table_age;
 		tab->at_params.is_wraparound = wraparound;
+		/* Allow interrupting autovacuum unless it's an emergency one */
+		tab->at_params.is_interruptible = !wraparound;
 		tab->at_params.log_min_duration = log_min_duration;
 		tab->at_vacuum_cost_limit = vac_cost_limit;
 		tab->at_vacuum_cost_delay = vac_cost_delay;
diff --git a/src/backend/storage/lmgr/deadlock.c b/src/backend/storage/lmgr/deadlock.c
index e1246b8a4da..770b871f761 100644
--- a/src/backend/storage/lmgr/deadlock.c
+++ b/src/backend/storage/lmgr/deadlock.c
@@ -124,8 +124,8 @@ static int	maxPossibleConstraints;
 static DEADLOCK_INFO *deadlockDetails;
 static int	nDeadlockDetails;
 
-/* PGPROC pointer of any blocking autovacuum worker found */
-static PGPROC *blocking_autovacuum_proc = NULL;
+/* PGPROC pointer of any blocking but interruptible process found */
+static PGPROC *blocking_interruptible_proc = NULL;
 
 
 /*
@@ -224,8 +224,8 @@ DeadLockCheck(PGPROC *proc)
 	nPossibleConstraints = 0;
 	nWaitOrders = 0;
 
-	/* Initialize to not blocked by an autovacuum worker */
-	blocking_autovacuum_proc = NULL;
+	/* Initialize to not blocked by an interruptible process */
+	blocking_interruptible_proc = NULL;
 
 	/* Search for deadlocks and possible fixes */
 	if (DeadLockCheckRecurse(proc))
@@ -278,24 +278,24 @@ DeadLockCheck(PGPROC *proc)
 	/* Return code tells caller if we had to escape a deadlock or not */
 	if (nWaitOrders > 0)
 		return DS_SOFT_DEADLOCK;
-	else if (blocking_autovacuum_proc != NULL)
-		return DS_BLOCKED_BY_AUTOVACUUM;
+	else if (blocking_interruptible_proc != NULL)
+		return DS_BLOCKED_BY_INTERRUPTIBLE;
 	else
 		return DS_NO_DEADLOCK;
 }
 
 /*
- * Return the PGPROC of the autovacuum that's blocking a process.
+ * Return the PGPROC of an interruptible process that's blocking a process.
  *
  * We reset the saved pointer as soon as we pass it back.
  */
 PGPROC *
-GetBlockingAutoVacuumPgproc(void)
+GetBlockingInterruptiblePgproc(void)
 {
 	PGPROC	   *ptr;
 
-	ptr = blocking_autovacuum_proc;
-	blocking_autovacuum_proc = NULL;
+	ptr = blocking_interruptible_proc;
+	blocking_interruptible_proc = NULL;
 
 	return ptr;
 }
@@ -606,30 +606,36 @@ FindLockCycleRecurseMember(PGPROC *checkProc,
 					}
 
 					/*
-					 * No deadlock here, but see if this proc is an autovacuum
+					 * No deadlock here, but see if this proc is an interruptible process
 					 * that is directly hard-blocking our own proc.  If so,
 					 * report it so that the caller can send a cancel signal
 					 * to it, if appropriate.  If there's more than one such
 					 * proc, it's indeterminate which one will be reported.
 					 *
-					 * We don't touch autovacuums that are indirectly blocking
+					 * We don't touch processes that are indirectly blocking
 					 * us; it's up to the direct blockee to take action.  This
 					 * rule simplifies understanding the behavior and ensures
-					 * that an autovacuum won't be canceled with less than
-					 * deadlock_timeout grace period.
+					 * such a process won't be canceled with a grace period of
+					 * less than deadlock_timeout.
 					 *
-					 * Note we read vacuumFlags without any locking.  This is
-					 * OK only for checking the PROC_IS_AUTOVACUUM flag,
-					 * because that flag is set at process start and never
-					 * reset.  There is logic elsewhere to avoid canceling an
-					 * autovacuum that is working to prevent XID wraparound
-					 * problems (which needs to read a different vacuumFlag
-					 * bit), but we don't do that here to avoid grabbing
-					 * ProcArrayLock.
+					 * Note we read vacuumFlags without any locking.  That is
+					 * ok because 32bit reads are atomic, because surrounding
+					 * operations provide strong enough memory barriers, and
+					 * because we re-check whether IS_INTERRUPTIBLE is still
+					 * set before actually cancelling the backend.
 					 */
 					if (checkProc == MyProc &&
-						proc->vacuumFlags & PROC_IS_AUTOVACUUM)
-						blocking_autovacuum_proc = proc;
+						proc->vacuumFlags & PROC_IS_INTERRUPTIBLE)
+					{
+						/*
+						 * XXX: At some point there could be more than one
+						 * proc blocking us here. Currently that's not
+						 * possible because of the lock levels used by VACUUM
+						 * / ANALYZE, which are the only ones to set this flag
+						 * currently, but ...
+						 */
+						blocking_interruptible_proc = proc;
+					}
 
 					/* We're done looking at this proclock */
 					break;
diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c
index 19a9f939492..0b288ac6f2d 100644
--- a/src/backend/storage/lmgr/proc.c
+++ b/src/backend/storage/lmgr/proc.c
@@ -1064,7 +1064,7 @@ ProcSleep(LOCALLOCK *locallock, LockMethod lockMethodTable)
 	PROC_QUEUE *waitQueue = &(lock->waitProcs);
 	LOCKMASK	myHeldLocks = MyProc->heldLocks;
 	bool		early_deadlock = false;
-	bool		allow_autovacuum_cancel = true;
+	bool		allow_interruptible_cancel = true;
 	ProcWaitStatus myWaitStatus;
 	PGPROC	   *proc;
 	PGPROC	   *leader = MyProc->lockGroupLeader;
@@ -1304,23 +1304,21 @@ ProcSleep(LOCALLOCK *locallock, LockMethod lockMethodTable)
 		myWaitStatus = *((volatile ProcWaitStatus *) &MyProc->waitStatus);
 
 		/*
-		 * If we are not deadlocked, but are waiting on an autovacuum-induced
-		 * task, send a signal to interrupt it.
+		 * If we are not deadlocked, but are waiting on an interruptible
+		 * (e.g. autovacuum-induced) task, send a signal to interrupt it.
 		 */
-		if (deadlock_state == DS_BLOCKED_BY_AUTOVACUUM && allow_autovacuum_cancel)
+		if (deadlock_state == DS_BLOCKED_BY_INTERRUPTIBLE && allow_interruptible_cancel)
 		{
-			PGPROC	   *autovac = GetBlockingAutoVacuumPgproc();
+			PGPROC	   *autovac = GetBlockingInterruptiblePgproc();
 			uint8		vacuumFlags;
 
 			LWLockAcquire(ProcArrayLock, LW_EXCLUSIVE);
 
 			/*
-			 * Only do it if the worker is not working to protect against Xid
-			 * wraparound.
+			 * Only do it if the process is still interruptible.
 			 */
 			vacuumFlags = ProcGlobal->vacuumFlags[autovac->pgxactoff];
-			if ((vacuumFlags & PROC_IS_AUTOVACUUM) &&
-				!(vacuumFlags & PROC_VACUUM_FOR_WRAPAROUND))
+			if (vacuumFlags & PROC_IS_INTERRUPTIBLE)
 			{
 				int			pid = autovac->pid;
 				StringInfoData locktagbuf;
@@ -1340,8 +1338,12 @@ ProcSleep(LOCALLOCK *locallock, LockMethod lockMethodTable)
 				LWLockRelease(ProcArrayLock);
 
 				/* send the autovacuum worker Back to Old Kent Road */
+				/*
+				 * FIXME: do we want to continue to identify autovacuum here?
+				 * Could do so based on PROC_IS_AUTOVACUUM.
+				 */
 				ereport(DEBUG1,
-						(errmsg("sending cancel to blocking autovacuum PID %d",
+						(errmsg("sending cancel to blocking autovacuum|XXX PID %d",
 								pid),
 						 errdetail_log("%s", logbuf.data)));
 
@@ -1370,7 +1372,7 @@ ProcSleep(LOCALLOCK *locallock, LockMethod lockMethodTable)
 				LWLockRelease(ProcArrayLock);
 
 			/* prevent signal from being resent more than once */
-			allow_autovacuum_cancel = false;
+			allow_interruptible_cancel = false;
 		}
 
 		/*
diff --git a/doc/src/sgml/ref/analyze.sgml b/doc/src/sgml/ref/analyze.sgml
index 5ac3ba83219..c7f3f58c6da 100644
--- a/doc/src/sgml/ref/analyze.sgml
+++ b/doc/src/sgml/ref/analyze.sgml
@@ -28,6 +28,7 @@ ANALYZE [ VERBOSE ] [ <replaceable class="parameter">table_and_columns</replacea
 
     VERBOSE [ <replaceable class="parameter">boolean</replaceable> ]
     SKIP_LOCKED [ <replaceable class="parameter">boolean</replaceable> ]
+    INTERRUPTIBLE [ <replaceable class="parameter">boolean</replaceable> ]
 
 <phrase>and <replaceable class="parameter">table_and_columns</replaceable> is:</phrase>
 
@@ -95,6 +96,21 @@ ANALYZE [ VERBOSE ] [ <replaceable class="parameter">table_and_columns</replacea
     </listitem>
    </varlistentry>
 
+   <varlistentry>
+    <term><literal>INTERRUPTIBLE</literal></term>
+    <listitem>
+     <para>
+      Specifies whether <command>ANALYZE</command> may be cancelled when
+      another connection needs a lock conflicting with this.
+      <command>ANALYZE</command>. That can be useful to reduce the impact of
+      running <command>ANALYZE</command> in a busy system, by e.g. allowing
+      DDL commands to run before <command>ANALYZE</command> has finished.  If
+      the option is not specified <command>ANALYZE</command> is not
+      interruptible.
+     </para>
+    </listitem>
+   </varlistentry>
+
    <varlistentry>
     <term><replaceable class="parameter">boolean</replaceable></term>
     <listitem>
diff --git a/doc/src/sgml/ref/vacuum.sgml b/doc/src/sgml/ref/vacuum.sgml
index a48f75ad7ba..1d69041566e 100644
--- a/doc/src/sgml/ref/vacuum.sgml
+++ b/doc/src/sgml/ref/vacuum.sgml
@@ -35,6 +35,7 @@ VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] [ ANALYZE ] [ <replaceable class="paramet
     INDEX_CLEANUP [ <replaceable class="parameter">boolean</replaceable> ]
     TRUNCATE [ <replaceable class="parameter">boolean</replaceable> ]
     PARALLEL <replaceable class="parameter">integer</replaceable>
+    INTERRUPTIBLE [ <replaceable class="parameter">boolean</replaceable> ]
 
 <phrase>and <replaceable class="parameter">table_and_columns</replaceable> is:</phrase>
 
@@ -255,6 +256,22 @@ VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] [ ANALYZE ] [ <replaceable class="paramet
     </listitem>
    </varlistentry>
 
+   <varlistentry>
+    <term><literal>INTERRUPTIBLE</literal></term>
+    <listitem>
+     <para>
+      Specifies whether <command>VACUUM</command> may be cancelled when
+      another connection needs a lock conflicting with this.
+      <command>VACUUM</command>. That can be useful to reduce the impact of
+      running <command>VACUUM</command> in a busy system, by e.g. allowing DDL
+      commands to run before <command>VACUUM</command> has finished.  This
+      option is currently ignored if the <literal>FULL</literal> option is
+      used. If the option is not specified <command>VACUUM</command> is not
+      interruptible.
+     </para>
+    </listitem>
+   </varlistentry>
+
    <varlistentry>
     <term><replaceable class="parameter">boolean</replaceable></term>
     <listitem>
-- 
2.25.0.114.g5b0ca878e0

