From 05299c00c6933eaf0e97b8b5f512e190c5abe8d3 Mon Sep 17 00:00:00 2001 From: Nathan Bossart Date: Mon, 5 Mar 2018 21:46:01 +0000 Subject: [PATCH v6 2/2] Add NOWAIT option to VACUUM and ANALYZE. FIXME: blocking doesn't work for AEL FIXME: NOWAIT isn't actually implemented, closer to SKIP LOCKED Author: Nathan Bossart Reviewed-By: Michael Paquier, Andres Freund, Masahiko Sawada Discussion: https://postgr.es/m/D3FC73E2-9B1A-4DB4-8180-55F57D116B4E@amazon.com --- doc/src/sgml/ref/analyze.sgml | 12 ++ doc/src/sgml/ref/vacuum.sgml | 12 ++ src/backend/commands/analyze.c | 2 +- src/backend/commands/vacuum.c | 55 ++++++++- src/backend/parser/gram.y | 2 + src/backend/postmaster/autovacuum.c | 2 +- src/include/nodes/parsenodes.h | 2 +- src/test/isolation/expected/vacuum-skip-locked.out | 129 +++++++++++++++++++++ src/test/isolation/isolation_schedule | 1 + src/test/isolation/specs/vacuum-skip-locked.spec | 53 +++++++++ src/test/regress/expected/vacuum.out | 9 ++ src/test/regress/sql/vacuum.sql | 9 ++ 12 files changed, 280 insertions(+), 8 deletions(-) create mode 100644 src/test/isolation/expected/vacuum-skip-locked.out create mode 100644 src/test/isolation/specs/vacuum-skip-locked.spec diff --git a/doc/src/sgml/ref/analyze.sgml b/doc/src/sgml/ref/analyze.sgml index 10b3a9a..4e6ae38 100644 --- a/doc/src/sgml/ref/analyze.sgml +++ b/doc/src/sgml/ref/analyze.sgml @@ -27,6 +27,7 @@ ANALYZE [ VERBOSE ] [ table_and_columnswhere option can be one of: VERBOSE + SKIP LOCKED and table_and_columns is: @@ -77,6 +78,17 @@ ANALYZE [ VERBOSE ] [ table_and_columns + SKIP LOCKED + + + Specifies that ANALYZE should not wait for any + conflicting locks to be released: if a relation cannot be locked + immediately without waiting, the relation is skipped. + + + + + table_name diff --git a/doc/src/sgml/ref/vacuum.sgml b/doc/src/sgml/ref/vacuum.sgml index b760e8e..1ef5bc9 100644 --- a/doc/src/sgml/ref/vacuum.sgml +++ b/doc/src/sgml/ref/vacuum.sgml @@ -31,6 +31,7 @@ VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] [ ANALYZE ] [ table_and_columns is: @@ -161,6 +162,17 @@ VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] [ ANALYZE ] [ table_name diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c index 5f21fcb..68f84c9 100644 --- a/src/backend/commands/analyze.c +++ b/src/backend/commands/analyze.c @@ -143,7 +143,7 @@ analyze_rel(Oid relid, RangeVar *relation, int options, * matter if we ever try to accumulate stats on dead tuples.) If the rel * has been dropped since we last saw it, we don't need to process it. */ - if (!(options & VACOPT_NOWAIT)) + if (!(options & VACOPT_SKIP_LOCKED)) onerel = try_relation_open(relid, ShareUpdateExclusiveLock); else if (ConditionalLockRelationOid(relid, ShareUpdateExclusiveLock)) onerel = try_relation_open(relid, NoLock); diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c index 7aca69a..62eb95c 100644 --- a/src/backend/commands/vacuum.c +++ b/src/backend/commands/vacuum.c @@ -46,6 +46,7 @@ #include "storage/procarray.h" #include "utils/acl.h" #include "utils/fmgroids.h" +#include "utils/formatting.h" #include "utils/guc.h" #include "utils/memutils.h" #include "utils/snapmgr.h" @@ -68,7 +69,7 @@ static BufferAccessStrategy vac_strategy; /* non-export function prototypes */ -static List *expand_vacuum_rel(VacuumRelation *vrel); +static List *expand_vacuum_rel(VacuumRelation *vrel, int options, const char *stmttype); static List *get_all_vacuum_rels(void); static void vac_truncate_clog(TransactionId frozenXID, MultiXactId minMulti, @@ -250,6 +251,13 @@ vacuum(int options, List *relations, VacuumParams *params, { List *newrels = NIL; ListCell *lc; + const char *stmttype_lower; + + /* + * We must downcase the statement type for log message consistency + * between expand_vacuum_rel(), vacuum_rel(), and analyze_rel(). + */ + stmttype_lower = asc_tolower(stmttype, strlen(stmttype)); foreach(lc, relations) { @@ -257,7 +265,7 @@ vacuum(int options, List *relations, VacuumParams *params, List *sublist; MemoryContext old_context; - sublist = expand_vacuum_rel(vrel); + sublist = expand_vacuum_rel(vrel, options, stmttype_lower); old_context = MemoryContextSwitchTo(vac_context); newrels = list_concat(newrels, sublist); MemoryContextSwitchTo(old_context); @@ -423,7 +431,7 @@ vacuum(int options, List *relations, VacuumParams *params, * are made in vac_context. */ static List * -expand_vacuum_rel(VacuumRelation *vrel) +expand_vacuum_rel(VacuumRelation *vrel, int options, const char *stmttype) { List *vacrels = NIL; MemoryContext oldcontext; @@ -444,11 +452,48 @@ expand_vacuum_rel(VacuumRelation *vrel) bool include_parts; /* + * Since autovacuum workers supply OIDs when calling vacuum(), no + * autovacuum worker should reach this code, and we can make + * assumptions about the logging levels we should use in the checks + * below. + */ + Assert(!IsAutoVacuumWorkerProcess()); + + /* * We transiently take AccessShareLock to protect the syscache lookup * below, as well as find_all_inheritors's expectation that the caller * holds some lock on the starting relation. */ - relid = RangeVarGetRelid(vrel->relation, AccessShareLock, false); + if (!(options & VACOPT_SKIP_LOCKED)) + relid = RangeVarGetRelid(vrel->relation, AccessShareLock, false); + else + { + relid = RangeVarGetRelid(vrel->relation, NoLock, false); + + /* + * The relation may go away before we can obtain the lock, so we + * must verify that it still exists afterwards. If the lock is + * unavailable or the relation has disappeared, emit the same log + * statement that vacuum_rel() and analyze_rel() would. + */ + if (!ConditionalLockRelationOid(relid, AccessShareLock)) + { + ereport(WARNING, + (errcode(ERRCODE_LOCK_NOT_AVAILABLE), + errmsg("skipping %s of \"%s\" --- lock not available", + stmttype, vrel->relation->relname))); + return vacrels; + } + else if (!SearchSysCacheExists1(RELOID, ObjectIdGetDatum(relid))) + { + UnlockRelationOid(relid, AccessShareLock); + ereport(WARNING, + (errcode(ERRCODE_UNDEFINED_TABLE), + errmsg("skipping %s of \"%s\" --- relation no longer exists", + stmttype, vrel->relation->relname))); + return vacrels; + } + } /* * Make a returnable VacuumRelation for this rel. @@ -1395,7 +1440,7 @@ vacuum_rel(Oid relid, RangeVar *relation, int options, VacuumParams *params) * If we've been asked not to wait for the relation lock, acquire it first * in non-blocking mode, before calling try_relation_open(). */ - if (!(options & VACOPT_NOWAIT)) + if (!(options & VACOPT_SKIP_LOCKED)) onerel = try_relation_open(relid, lmode); else if (ConditionalLockRelationOid(relid, lmode)) onerel = try_relation_open(relid, NoLock); diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index 1d7317c..977a030 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -10538,6 +10538,7 @@ vacuum_option_elem: errmsg("unrecognized VACUUM option \"%s\"", $1), parser_errposition(@1))); } + | SKIP LOCKED { $$ = VACOPT_SKIP_LOCKED; } ; AnalyzeStmt: analyze_keyword opt_verbose opt_vacuum_relation_list @@ -10565,6 +10566,7 @@ analyze_option_list: analyze_option_elem: VERBOSE { $$ = VACOPT_VERBOSE; } + | SKIP LOCKED { $$ = VACOPT_SKIP_LOCKED; } ; analyze_keyword: diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c index 21f5e2c..77cbfe9 100644 --- a/src/backend/postmaster/autovacuum.c +++ b/src/backend/postmaster/autovacuum.c @@ -2883,7 +2883,7 @@ table_recheck_autovac(Oid relid, HTAB *table_toast_map, tab->at_vacoptions = VACOPT_SKIPTOAST | (dovacuum ? VACOPT_VACUUM : 0) | (doanalyze ? VACOPT_ANALYZE : 0) | - (!wraparound ? VACOPT_NOWAIT : 0); + (!wraparound ? VACOPT_SKIP_LOCKED : 0); tab->at_params.freeze_min_age = freeze_min_age; tab->at_params.freeze_table_age = freeze_table_age; tab->at_params.multixact_freeze_min_age = multixact_freeze_min_age; diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index ac292bc..744fdc4 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -3125,7 +3125,7 @@ typedef enum VacuumOption VACOPT_VERBOSE = 1 << 2, /* print progress info */ VACOPT_FREEZE = 1 << 3, /* FREEZE option */ VACOPT_FULL = 1 << 4, /* FULL (non-concurrent) vacuum */ - VACOPT_NOWAIT = 1 << 5, /* don't wait to get lock (autovacuum only) */ + VACOPT_SKIP_LOCKED = 1 << 5, /* don't wait to get lock */ VACOPT_SKIPTOAST = 1 << 6, /* don't process the TOAST table, if any */ VACOPT_DISABLE_PAGE_SKIPPING = 1 << 7 /* don't skip any pages */ } VacuumOption; diff --git a/src/test/isolation/expected/vacuum-skip-locked.out b/src/test/isolation/expected/vacuum-skip-locked.out new file mode 100644 index 0000000..cb11672 --- /dev/null +++ b/src/test/isolation/expected/vacuum-skip-locked.out @@ -0,0 +1,129 @@ +Parsed test spec with 2 sessions + +starting permutation: lock_share vac_specified commit +step lock_share: + BEGIN; + LOCK part1 IN SHARE MODE; + +WARNING: skipping vacuum of "part1" --- lock not available +step vac_specified: VACUUM (SKIP LOCKED) part1, part2; +step commit: + COMMIT; + + +starting permutation: lock_share vac_all_parts commit +step lock_share: + BEGIN; + LOCK part1 IN SHARE MODE; + +step vac_all_parts: VACUUM (SKIP LOCKED) parted; +step commit: + COMMIT; + + +starting permutation: lock_share analyze_specified commit +step lock_share: + BEGIN; + LOCK part1 IN SHARE MODE; + +WARNING: skipping analyze of "part1" --- lock not available +step analyze_specified: ANALYZE (SKIP LOCKED) part1, part2; +step commit: + COMMIT; + + +starting permutation: lock_share analyze_all_parts commit +step lock_share: + BEGIN; + LOCK part1 IN SHARE MODE; + +step analyze_all_parts: ANALYZE (SKIP LOCKED) parted; +step commit: + COMMIT; + + +starting permutation: lock_share vac_analyze_specified commit +step lock_share: + BEGIN; + LOCK part1 IN SHARE MODE; + +WARNING: skipping vacuum of "part1" --- lock not available +step vac_analyze_specified: VACUUM (ANALYZE, SKIP LOCKED) part1, part2; +step commit: + COMMIT; + + +starting permutation: lock_share vac_analyze_all_parts commit +step lock_share: + BEGIN; + LOCK part1 IN SHARE MODE; + +step vac_analyze_all_parts: VACUUM (ANALYZE, SKIP LOCKED) parted; +step commit: + COMMIT; + + +starting permutation: lock_access_exclusive vac_specified commit +step lock_access_exclusive: + BEGIN; + LOCK part1 IN ACCESS EXCLUSIVE MODE; + +WARNING: skipping vacuum of "part1" --- lock not available +step vac_specified: VACUUM (SKIP LOCKED) part1, part2; +step commit: + COMMIT; + + +starting permutation: lock_access_exclusive vac_all_parts commit +step lock_access_exclusive: + BEGIN; + LOCK part1 IN ACCESS EXCLUSIVE MODE; + +step vac_all_parts: VACUUM (SKIP LOCKED) parted; +step commit: + COMMIT; + + +starting permutation: lock_access_exclusive analyze_specified commit +step lock_access_exclusive: + BEGIN; + LOCK part1 IN ACCESS EXCLUSIVE MODE; + +WARNING: skipping analyze of "part1" --- lock not available +step analyze_specified: ANALYZE (SKIP LOCKED) part1, part2; +step commit: + COMMIT; + + +starting permutation: lock_access_exclusive analyze_all_parts commit +step lock_access_exclusive: + BEGIN; + LOCK part1 IN ACCESS EXCLUSIVE MODE; + +step analyze_all_parts: ANALYZE (SKIP LOCKED) parted; +step commit: + COMMIT; + +step analyze_all_parts: <... completed> + +starting permutation: lock_access_exclusive vac_analyze_specified commit +step lock_access_exclusive: + BEGIN; + LOCK part1 IN ACCESS EXCLUSIVE MODE; + +WARNING: skipping vacuum of "part1" --- lock not available +step vac_analyze_specified: VACUUM (ANALYZE, SKIP LOCKED) part1, part2; +step commit: + COMMIT; + + +starting permutation: lock_access_exclusive vac_analyze_all_parts commit +step lock_access_exclusive: + BEGIN; + LOCK part1 IN ACCESS EXCLUSIVE MODE; + +step vac_analyze_all_parts: VACUUM (ANALYZE, SKIP LOCKED) parted; +step commit: + COMMIT; + +step vac_analyze_all_parts: <... completed> diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule index 74d7d59..520734b 100644 --- a/src/test/isolation/isolation_schedule +++ b/src/test/isolation/isolation_schedule @@ -66,3 +66,4 @@ test: async-notify test: vacuum-reltuples test: timeouts test: vacuum-concurrent-drop +test: vacuum-skip-locked diff --git a/src/test/isolation/specs/vacuum-skip-locked.spec b/src/test/isolation/specs/vacuum-skip-locked.spec new file mode 100644 index 0000000..96c3bf4 --- /dev/null +++ b/src/test/isolation/specs/vacuum-skip-locked.spec @@ -0,0 +1,53 @@ +# Test for the SKIP LOCKED option for VACUUM and ANALYZE commands. +# +# This also verifies that log messages are not emitted for skipped relations +# that were not specified in the VACUUM or ANALYZE command. + +setup +{ + CREATE TABLE parted (a INT) PARTITION BY LIST (a); + CREATE TABLE part1 PARTITION OF parted FOR VALUES IN (1); + CREATE TABLE part2 PARTITION OF parted FOR VALUES IN (2); +} + +teardown +{ + DROP TABLE IF EXISTS parted; +} + +session "s1" +step "lock_share" +{ + BEGIN; + LOCK part1 IN SHARE MODE; +} +step "lock_access_exclusive" +{ + BEGIN; + LOCK part1 IN ACCESS EXCLUSIVE MODE; +} +step "commit" +{ + COMMIT; +} + +session "s2" +step "vac_specified" { VACUUM (SKIP LOCKED) part1, part2; } +step "vac_all_parts" { VACUUM (SKIP LOCKED) parted; } +step "analyze_specified" { ANALYZE (SKIP LOCKED) part1, part2; } +step "analyze_all_parts" { ANALYZE (SKIP LOCKED) parted; } +step "vac_analyze_specified" { VACUUM (ANALYZE, SKIP LOCKED) part1, part2; } +step "vac_analyze_all_parts" { VACUUM (ANALYZE, SKIP LOCKED) parted; } + +permutation "lock_share" "vac_specified" "commit" +permutation "lock_share" "vac_all_parts" "commit" +permutation "lock_share" "analyze_specified" "commit" +permutation "lock_share" "analyze_all_parts" "commit" +permutation "lock_share" "vac_analyze_specified" "commit" +permutation "lock_share" "vac_analyze_all_parts" "commit" +permutation "lock_access_exclusive" "vac_specified" "commit" +permutation "lock_access_exclusive" "vac_all_parts" "commit" +permutation "lock_access_exclusive" "analyze_specified" "commit" +permutation "lock_access_exclusive" "analyze_all_parts" "commit" +permutation "lock_access_exclusive" "vac_analyze_specified" "commit" +permutation "lock_access_exclusive" "vac_analyze_all_parts" "commit" diff --git a/src/test/regress/expected/vacuum.out b/src/test/regress/expected/vacuum.out index d66e2aa..5adfdb1 100644 --- a/src/test/regress/expected/vacuum.out +++ b/src/test/regress/expected/vacuum.out @@ -119,6 +119,15 @@ ANALYZE (nonexistant-arg) does_not_exist; ERROR: syntax error at or near "nonexistant" LINE 1: ANALYZE (nonexistant-arg) does_not_exist; ^ +-- ensure argument order independence, and that SKIP LOCKED on non-existing +-- relation still errors out. +ANALYZE (SKIP LOCKED, VERBOSE) does_not_exist; +ERROR: relation "does_not_exist" does not exist +ANALYZE (VERBOSE, SKIP LOCKED) does_not_exist; +ERROR: relation "does_not_exist" does not exist +-- nowait option +VACUUM (SKIP LOCKED) vactst; +ANALYZE (SKIP LOCKED) vactst; DROP TABLE vaccluster; DROP TABLE vactst; DROP TABLE vacparted; diff --git a/src/test/regress/sql/vacuum.sql b/src/test/regress/sql/vacuum.sql index 275ce2e..908a288 100644 --- a/src/test/regress/sql/vacuum.sql +++ b/src/test/regress/sql/vacuum.sql @@ -93,6 +93,15 @@ ANALYZE vactst (i), vacparted (does_not_exist); ANALYZE (VERBOSE) does_not_exist; ANALYZE (nonexistant-arg) does_not_exist; +-- ensure argument order independence, and that SKIP LOCKED on non-existing +-- relation still errors out. +ANALYZE (SKIP LOCKED, VERBOSE) does_not_exist; +ANALYZE (VERBOSE, SKIP LOCKED) does_not_exist; + +-- nowait option +VACUUM (SKIP LOCKED) vactst; +ANALYZE (SKIP LOCKED) vactst; + DROP TABLE vaccluster; DROP TABLE vactst; DROP TABLE vacparted; -- 2.7.3.AMZN