Hi,

On 2020-09-08 21:11:47 -0700, Andres Freund wrote:
> > Not yet sure about what a suitable way to test this for analyze would
> > be, unless we'd also accept VacuumDelay as a wait condition :(. I guess
> > one fairly easy way would be to include an expression index, and have
> > the expression index acquire an unavailable lock. Which'd allow to
> > switch to another session.
> 
> But here I've not yet done anything. That just seems too ugly & fragile.

I found a better way after a bunch of tinkering. Having an
isolationtester session lock pg_class in SHARE mode works, because both
VACUUM and ANALYZE update the relation's row. I *think* this is
reliable, due to the report_multiple_error_messages() logic.

This version also adds setting of PROC_INTERRUPTIBLE for ANALYZE
(INTERUPTIBLE), which the previous version didn't yet contain. As
currently implemented this means that autovacuum started ANALYZEs are
interruptible, which they were not before. That's dead trivial to
change, but to me it makes sense to leave it this way. But I also see an
argument that it'd be better to do so separately?


> - It's a bit annoying that normal user backends just see a generic
>   "ERROR: canceling statement due to user request". Do we need something
>   better?

I think this is ok for this iteration, and instead at some point solve
this in a bit more generic manner. This isn't the only place where
carrying a bit more information about why and by whom a query is
cancelled would be very useful.

There's one XXX left in here, which is:

@@ -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)));
 

given that this is a DEBUG1 I'm inclined to just replace this with
"... blocking interruptible process with PID %d"
or such?

Comments?

Greetings,

Andres Freund
>From bd3f6fdc187f92e8e98018e9c9cfd9cac8753dc7 Mon Sep 17 00:00:00 2001
From: Andres Freund <and...@anarazel.de>
Date: Wed, 30 Sep 2020 20:24:38 -0700
Subject: [PATCH v2] Add INTERRUPTIBLE option to VACUUM & ANALYZE, test
 interruption.

Partially because that's practically useful, partially because it's
useful for testing (see 5871f09c985).

To implement add the new PROC_IS_INTERRUPTIBLE flag, controlling
whether a process should be interrupted if it holds a required
lock. As PROC_VACUUM_FOR_WRAPAROUND was only used to signal that a
PROC_IS_AUTOVACUUM proc should not be interrupted, I have removed
that.

Now that it's easier to trigger interruptible vacuums, add a test
doing so.

FIXME: change commit message based on whether we keep the behaviour of
autovacuum cancelling analyze or not.

Discussion: https://www.postgresql.org/message-id/20200909041147.amcitkxmlexgjfty%40alap3.anarazel.de
---
 src/include/commands/vacuum.h                 |  1 +
 src/include/storage/lock.h                    |  6 +-
 src/include/storage/proc.h                    |  5 +-
 src/backend/commands/analyze.c                | 22 +++++++
 src/backend/commands/vacuum.c                 | 14 +++--
 src/backend/postmaster/autovacuum.c           |  2 +
 src/backend/storage/lmgr/deadlock.c           | 63 ++++++++++---------
 src/backend/storage/lmgr/proc.c               | 24 +++----
 doc/src/sgml/ref/analyze.sgml                 | 16 +++++
 doc/src/sgml/ref/vacuum.sgml                  | 17 +++++
 src/test/isolation/expected/vacuum-cancel.out | 41 ++++++++++++
 src/test/isolation/isolation_schedule         |  1 +
 src/test/isolation/specs/vacuum-cancel.spec   | 55 ++++++++++++++++
 13 files changed, 218 insertions(+), 49 deletions(-)
 create mode 100644 src/test/isolation/expected/vacuum-cancel.out
 create mode 100644 src/test/isolation/specs/vacuum-cancel.spec

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/analyze.c b/src/backend/commands/analyze.c
index 8af12b5c6b2..9bafd07e758 100644
--- a/src/backend/commands/analyze.c
+++ b/src/backend/commands/analyze.c
@@ -252,6 +252,15 @@ analyze_rel(Oid relid, RangeVar *relation,
 	pgstat_progress_start_command(PROGRESS_COMMAND_ANALYZE,
 								  RelationGetRelid(onerel));
 
+	/* mark us as interruptible if appropriate */
+	if (params->is_interruptible)
+	{
+		LWLockAcquire(ProcArrayLock, LW_EXCLUSIVE);
+		MyProc->vacuumFlags |= PROC_IS_INTERRUPTIBLE;
+		ProcGlobal->vacuumFlags[MyProc->pgxactoff] = MyProc->vacuumFlags;
+		LWLockRelease(ProcArrayLock);
+	}
+
 	/*
 	 * Do the normal non-recursive ANALYZE.  We can skip this for partitioned
 	 * tables, which don't contain any rows.
@@ -276,6 +285,19 @@ analyze_rel(Oid relid, RangeVar *relation,
 	relation_close(onerel, NoLock);
 
 	pgstat_progress_end_command();
+
+	/*
+	 * Reset flags if we set them. Need to that (in contrast to VACUUM)
+	 * because analyze can be run within a transaction, so we can't rely on
+	 * the end-of-xact code doing this for us.
+	 */
+	if (params->is_interruptible)
+	{
+		LWLockAcquire(ProcArrayLock, LW_EXCLUSIVE);
+		MyProc->vacuumFlags &= ~PROC_IS_INTERRUPTIBLE;
+		ProcGlobal->vacuumFlags[MyProc->pgxactoff] = MyProc->vacuumFlags;
+		LWLockRelease(ProcArrayLock);
+	}
 }
 
 /*
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 2cef56f115f..630e068649e 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -2915,6 +2915,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..425a81194f7 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,37 @@ FindLockCycleRecurseMember(PGPROC *checkProc,
 					}
 
 					/*
-					 * No deadlock here, but see if this proc is an autovacuum
-					 * 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.
+					 * 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 88566bd9fab..f95eb549e3c 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 sent again 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 26ede69bb31..b3e31423f17 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>
diff --git a/src/test/isolation/expected/vacuum-cancel.out b/src/test/isolation/expected/vacuum-cancel.out
new file mode 100644
index 00000000000..7914df3f8a2
--- /dev/null
+++ b/src/test/isolation/expected/vacuum-cancel.out
@@ -0,0 +1,41 @@
+Parsed test spec with 3 sessions
+
+starting permutation: s1_begin s1_block_vacuum s2_delete s2_vacuum_interruptible s1_commit
+step s1_begin: BEGIN;
+step s1_block_vacuum: LOCK pg_class IN SHARE MODE;
+step s2_delete: DELETE FROM test_vacuum_cancel;
+step s2_vacuum_interruptible: VACUUM (FREEZE, INTERRUPTIBLE) test_vacuum_cancel; <waiting ...>
+step s1_commit: COMMIT;
+step s2_vacuum_interruptible: <... completed>
+
+starting permutation: s1_begin s1_block_vacuum s2_delete s2_analyze_interruptible s1_commit
+step s1_begin: BEGIN;
+step s1_block_vacuum: LOCK pg_class IN SHARE MODE;
+step s2_delete: DELETE FROM test_vacuum_cancel;
+step s2_analyze_interruptible: ANALYZE (INTERRUPTIBLE) test_vacuum_cancel; <waiting ...>
+step s1_commit: COMMIT;
+step s2_analyze_interruptible: <... completed>
+
+starting permutation: s1_begin s1_block_vacuum s2_delete s2_vacuum_interruptible s3_lock s3_commit s1_commit
+step s1_begin: BEGIN;
+step s1_block_vacuum: LOCK pg_class IN SHARE MODE;
+step s2_delete: DELETE FROM test_vacuum_cancel;
+step s2_vacuum_interruptible: VACUUM (FREEZE, INTERRUPTIBLE) test_vacuum_cancel; <waiting ...>
+step s3_lock: BEGIN; LOCK test_vacuum_cancel; <waiting ...>
+step s3_lock: <... completed>
+step s2_vacuum_interruptible: <... completed>
+error in steps s3_lock s2_vacuum_interruptible: ERROR:  canceling statement due to user request
+step s3_commit: COMMIT;
+step s1_commit: COMMIT;
+
+starting permutation: s1_begin s1_block_vacuum s2_delete s2_analyze_interruptible s3_lock s3_commit s1_commit
+step s1_begin: BEGIN;
+step s1_block_vacuum: LOCK pg_class IN SHARE MODE;
+step s2_delete: DELETE FROM test_vacuum_cancel;
+step s2_analyze_interruptible: ANALYZE (INTERRUPTIBLE) test_vacuum_cancel; <waiting ...>
+step s3_lock: BEGIN; LOCK test_vacuum_cancel; <waiting ...>
+step s3_lock: <... completed>
+step s2_analyze_interruptible: <... completed>
+error in steps s3_lock s2_analyze_interruptible: ERROR:  canceling statement due to user request
+step s3_commit: COMMIT;
+step s1_commit: COMMIT;
diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule
index aa386ab1a25..ff437fcbdd3 100644
--- a/src/test/isolation/isolation_schedule
+++ b/src/test/isolation/isolation_schedule
@@ -78,6 +78,7 @@ test: timeouts
 test: vacuum-concurrent-drop
 test: vacuum-conflict
 test: vacuum-skip-locked
+test: vacuum-cancel
 test: predicate-hash
 test: predicate-gist
 test: predicate-gin
diff --git a/src/test/isolation/specs/vacuum-cancel.spec b/src/test/isolation/specs/vacuum-cancel.spec
new file mode 100644
index 00000000000..f8e502bb99f
--- /dev/null
+++ b/src/test/isolation/specs/vacuum-cancel.spec
@@ -0,0 +1,55 @@
+setup
+{
+    CREATE TABLE test_vacuum_cancel(data text);
+    /* don't want autovacuum clean up to-be-cleaned data concurrently */
+    ALTER TABLE test_vacuum_cancel SET (AUTOVACUUM_ENABLED = false);
+    /* insert some data  */
+    INSERT INTO test_vacuum_cancel VALUES('somedata'),('otherdata');
+}
+
+teardown
+{
+    DROP TABLE test_vacuum_cancel;
+}
+
+session "s1"
+step "s1_begin" { BEGIN; }
+step "s1_block_vacuum" { LOCK pg_class IN SHARE MODE; }
+step "s1_commit" { COMMIT; }
+
+session "s2"
+step "s2_delete" { DELETE FROM test_vacuum_cancel; }
+step "s2_vacuum_interruptible" { VACUUM (FREEZE, INTERRUPTIBLE) test_vacuum_cancel; }
+step "s2_analyze_interruptible" { ANALYZE (INTERRUPTIBLE) test_vacuum_cancel; }
+
+session "s3"
+step "s3_lock" { BEGIN; LOCK test_vacuum_cancel; }
+step "s3_commit" { COMMIT; }
+
+# First test if releasing the lock on pg_class allows to continue
+permutation
+    # block vacuum
+    "s1_begin" "s1_block_vacuum" "s2_delete"
+    # vacuum, which will be blocked by the above
+    "s2_vacuum_interruptible"
+    # and allow to continue
+    "s1_commit"
+permutation "s1_begin" "s1_block_vacuum" "s2_delete" "s2_analyze_interruptible" "s1_commit"
+
+# Then track that concurrent LOCK interrupts VACUUM / ANALYZE
+permutation
+    # block vacuum
+    "s1_begin" "s1_block_vacuum" "s2_delete"
+    # vacuum, which will be blocked by the above
+    "s2_vacuum_interruptible"
+    # issue lock request conflicting with vacuum
+    "s3_lock"
+    # and release the blocking lock request again. Done separately, to
+    # ensure isolationtester schedules predictably
+    "s3_commit" "s1_commit"
+
+permutation
+    "s1_begin" "s1_block_vacuum" "s2_delete"
+    "s2_analyze_interruptible"
+    "s3_lock"
+    "s3_commit" "s1_commit"
-- 
2.28.0.651.g306ee63a70

Reply via email to