Hi,
On 2020-09-08 19:30:40 -0700, Andres Freund wrote:
> It was fairly straight forward to implement the basics of
> INTERRUPTIBLE. However it seems like it might not actually address my
> main concern, i.e. making this reliably testable with isolation
> tester. At least not the way I envisioned it...
>
> My idea for the test was to have one isolationtester session start a
> seqscan, which would then reliably block a concurrent VACUUM (FREEZE,
> INTERRUPTIBLE). That'd then give a stable point to switch back to the
> first session, which would interrupt the VACUUM by doing a LOCK.
>
> But because there's no known waiting-for pid for a buffer pin wait, we
> currently don't detect that we're blocked :(.
>
>
> Now, it'd not be too hard to make pg_isolation_test_session_is_blocked
> also report being blocked when we're waiting for a buffer pin (ignoring
> interesting_pids), similar to the safe snapshot test. However I'm
> worried that that could lead to "false" reports of blocking? But maybe
> that's a small enough risk because there's few unconditional cleanup
> lock acquisitions?
>
> Hacking such a wait condition test into
> pg_isolation_test_session_is_blocked() successfully allows my test to
> work for VACUUM.
Here's a patch series implementing that:
0001: Adds INTERRUPTIBLE to VACUUM ANALYZE
There's quite a few XXX's inside. And it needs some none
isolationtester test.
0002: Treat BufferPin as a waiting condition in isolationtest.
That's the aforementioned workaround.
0003: A test, finally.
But it only tests VACUUM, not yet ANALYZE. Perhaps also a test for
not allowing interruptions, somehow?
Clearly WIP, but good enough for some comments, I hope?
A few comments:
- Right now there can only be one such blocking task, because
PROC_IS_INTERRUPTIBLE is only set by vacuum / analyze, and the lock
levels are self exclusive. But it's generically named now, so it seems
just a matter of time until somebody adds that to other commands. I
think it's ok to not add support for ProcSleep() killing multiple
other processes?
- 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?
- The order in which VACUUM parameters are documented & implemented
seems fairly random. Perhaps it'd make sense to reorder
alphabetically?
> 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.
Greetings,
Andres Freund
>From 29d4c1bdf536222c1e86c7502bbbf3fd49751a90 Mon Sep 17 00:00:00 2001
From: Andres Freund <[email protected]>
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
>From 6613850df90f60a6e9daa0677bd81c64d5eb3107 Mon Sep 17 00:00:00 2001
From: Andres Freund <[email protected]>
Date: Tue, 8 Sep 2020 20:49:37 -0700
Subject: [PATCH v1 2/3] WIP: Treat BufferPin as a waiting condition in
isolationtest.
---
src/include/storage/procarray.h | 1 +
src/backend/storage/ipc/procarray.c | 24 ++++++++++++++++++++++++
src/backend/utils/adt/lockfuncs.c | 25 +++++++++++++++++++++++++
3 files changed, 50 insertions(+)
diff --git a/src/include/storage/procarray.h b/src/include/storage/procarray.h
index ea8a876ca45..62ee6833787 100644
--- a/src/include/storage/procarray.h
+++ b/src/include/storage/procarray.h
@@ -66,6 +66,7 @@ extern PGPROC *BackendPidGetProc(int pid);
extern PGPROC *BackendPidGetProcWithLock(int pid);
extern int BackendXidGetPid(TransactionId xid);
extern bool IsBackendPid(int pid);
+extern uint32 BackendPidGetWaitEvent(int pid);
extern VirtualTransactionId *GetCurrentVirtualXIDs(TransactionId limitXmin,
bool excludeXmin0, bool allDbs, int excludeVacuum,
diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c
index 1c0cd6b2487..5fb67e0e217 100644
--- a/src/backend/storage/ipc/procarray.c
+++ b/src/backend/storage/ipc/procarray.c
@@ -3023,6 +3023,30 @@ BackendPidGetProcWithLock(int pid)
return result;
}
+/*
+ * BackendPidGetWaitEvent -- return wait event of a backend
+ *
+ * This return 0 both if there is no backend with the passed PID and if the
+ * backend exists but is not currently waiting.
+ */
+uint32
+BackendPidGetWaitEvent(int pid)
+{
+ PGPROC *proc;
+ uint32 raw_wait_event = 0;
+
+ LWLockAcquire(ProcArrayLock, LW_SHARED);
+
+ proc = BackendPidGetProcWithLock(pid);
+
+ if (proc)
+ raw_wait_event = UINT32_ACCESS_ONCE(proc->wait_event_info);
+
+ LWLockRelease(ProcArrayLock);
+
+ return raw_wait_event;
+}
+
/*
* BackendXidGetPid -- get a backend's pid given its XID
*
diff --git a/src/backend/utils/adt/lockfuncs.c b/src/backend/utils/adt/lockfuncs.c
index f592292d067..1f41a85d496 100644
--- a/src/backend/utils/adt/lockfuncs.c
+++ b/src/backend/utils/adt/lockfuncs.c
@@ -17,7 +17,9 @@
#include "catalog/pg_type.h"
#include "funcapi.h"
#include "miscadmin.h"
+#include "pgstat.h"
#include "storage/predicate_internals.h"
+#include "storage/procarray.h"
#include "utils/array.h"
#include "utils/builtins.h"
@@ -601,6 +603,7 @@ pg_isolation_test_session_is_blocked(PG_FUNCTION_ARGS)
int num_interesting_pids;
int num_blocking_pids;
int dummy;
+ uint32 raw_wait_event;
int i,
j;
@@ -653,6 +656,28 @@ pg_isolation_test_session_is_blocked(PG_FUNCTION_ARGS)
if (GetSafeSnapshotBlockingPids(blocked_pid, &dummy, 1) > 0)
PG_RETURN_BOOL(true);
+ raw_wait_event = BackendPidGetWaitEvent(blocked_pid);
+
+ if (raw_wait_event != 0)
+ {
+ const char* wait_event_type;
+ const char* wait_event;
+
+ /*
+ * FIXME: probably better to match on the integer value itself. But
+ * currently the class / event mask is not exposed outside pgstat.c...
+ */
+ wait_event_type = pgstat_get_wait_event_type(raw_wait_event);
+ wait_event = pgstat_get_wait_event(raw_wait_event);
+
+ if (wait_event_type != NULL && wait_event_type != NULL &&
+ strcmp(wait_event_type, "BufferPin") == 0 &&
+ strcmp(wait_event, "BufferPin") == 0)
+ {
+ PG_RETURN_BOOL(true);
+ }
+ }
+
PG_RETURN_BOOL(false);
}
--
2.25.0.114.g5b0ca878e0
>From c5061503e34fe4676388b17297a4c2f02354bcd5 Mon Sep 17 00:00:00 2001
From: Andres Freund <[email protected]>
Date: Tue, 8 Sep 2020 20:51:09 -0700
Subject: [PATCH v1 3/3] WIP: Test for VACUUM (INTERRUPTIBLE) cancellation
working.
Needs ANALYZE support. Perhaps also a better approach for switching
sessions once the VACUUM has started (see previous commit).
Author:
Reviewed-By:
Discussion: https://postgr.es/m/
Backpatch:
---
src/test/isolation/expected/vacuum-cancel.out | 45 +++++++++++++++++++
src/test/isolation/isolation_schedule | 1 +
src/test/isolation/specs/vacuum-cancel.spec | 37 +++++++++++++++
3 files changed, 83 insertions(+)
create mode 100644 src/test/isolation/expected/vacuum-cancel.out
create mode 100644 src/test/isolation/specs/vacuum-cancel.spec
diff --git a/src/test/isolation/expected/vacuum-cancel.out b/src/test/isolation/expected/vacuum-cancel.out
new file mode 100644
index 00000000000..2532fb4c7c7
--- /dev/null
+++ b/src/test/isolation/expected/vacuum-cancel.out
@@ -0,0 +1,45 @@
+Parsed test spec with 2 sessions
+
+starting permutation: s1_begin s1_pin s2_delete s2_vacuum_interruptible s1_commit
+step s1_begin: BEGIN;
+step s1_pin:
+ DECLARE pin_page CURSOR WITHOUT HOLD FOR SELECT * FROM test_vacuum_cancel;
+ FETCH NEXT FROM pin_page;
+
+data
+
+somedata
+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_pin s2_delete s2_analyze_interruptible s1_commit
+step s1_begin: BEGIN;
+step s1_pin:
+ DECLARE pin_page CURSOR WITHOUT HOLD FOR SELECT * FROM test_vacuum_cancel;
+ FETCH NEXT FROM pin_page;
+
+data
+
+somedata
+step s2_delete: DELETE FROM test_vacuum_cancel;
+step s2_analyze_interruptible: ANALYZE (INTERRUPTIBLE) test_vacuum_cancel;
+step s1_commit: COMMIT;
+
+starting permutation: s1_begin s1_pin s2_delete s2_vacuum_interruptible s1_lock s1_commit
+step s1_begin: BEGIN;
+step s1_pin:
+ DECLARE pin_page CURSOR WITHOUT HOLD FOR SELECT * FROM test_vacuum_cancel;
+ FETCH NEXT FROM pin_page;
+
+data
+
+somedata
+step s2_delete: DELETE FROM test_vacuum_cancel;
+step s2_vacuum_interruptible: VACUUM (FREEZE, INTERRUPTIBLE) test_vacuum_cancel; <waiting ...>
+step s1_lock: LOCK test_vacuum_cancel; <waiting ...>
+step s1_lock: <... completed>
+step s2_vacuum_interruptible: <... completed>
+error in steps s1_lock s2_vacuum_interruptible: ERROR: canceling statement due to user request
+step s1_commit: COMMIT;
diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule
index 6acbb695ece..c02be96fb8c 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..e41877dce42
--- /dev/null
+++ b/src/test/isolation/specs/vacuum-cancel.spec
@@ -0,0 +1,37 @@
+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');
+ INSERT INTO test_vacuum_cancel VALUES('otherdata');
+}
+
+teardown
+{
+ DROP TABLE test_vacuum_cancel;
+}
+
+session "s1"
+step "s1_begin" { BEGIN; }
+step "s1_pin" {
+ DECLARE pin_page CURSOR WITHOUT HOLD FOR SELECT * FROM test_vacuum_cancel;
+ FETCH NEXT FROM pin_page;
+}
+step "s1_lock" { LOCK test_vacuum_cancel; }
+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; }
+
+# First test pin release resolves issues
+permutation "s1_begin" "s1_pin" "s2_delete" "s2_vacuum_interruptible" "s1_commit"
+# XXX: This doesn't actually wait on the pin, need alternative wait trick
+permutation "s1_begin" "s1_pin" "s2_delete" "s2_analyze_interruptible" "s1_commit"
+
+# Then track that concurrent TRUNCATE interrupts VACUUM / ANALYZE
+permutation "s1_begin" "s1_pin" "s2_delete" "s2_vacuum_interruptible" "s1_lock" "s1_commit"
+#permutation "s1_begin" "s1_pin" "s2_delete" "s2_analyze_interruptible" "s3_truncate"
--
2.25.0.114.g5b0ca878e0
