From 29d4c1bdf536222c1e86c7502bbbf3fd49751a90 Mon Sep 17 00:00:00 2001 From: Andres Freund 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 ] [ table_and_columnsboolean ] SKIP_LOCKED [ boolean ] + INTERRUPTIBLE [ boolean ] and table_and_columns is: @@ -95,6 +96,21 @@ ANALYZE [ VERBOSE ] [ table_and_columns + + INTERRUPTIBLE + + + Specifies whether ANALYZE may be cancelled when + another connection needs a lock conflicting with this. + ANALYZE. That can be useful to reduce the impact of + running ANALYZE in a busy system, by e.g. allowing + DDL commands to run before ANALYZE has finished. If + the option is not specified ANALYZE is not + interruptible. + + + + boolean 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 ] [ boolean ] TRUNCATE [ boolean ] PARALLEL integer + INTERRUPTIBLE [ boolean ] and table_and_columns is: @@ -255,6 +256,22 @@ VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] [ ANALYZE ] [ boolean -- 2.25.0.114.g5b0ca878e0