0003_add_nowait_vacuum_option_v3.patch

application/octet-stream

Filename: 0003_add_nowait_vacuum_option_v3.patch
Type: application/octet-stream
Part: 0
Message: Re: BUG #14941: Vacuum crashes

Patch

Format: unified
Series: patch v3
File+
doc/src/sgml/ref/analyze.sgml 12 0
doc/src/sgml/ref/vacuum.sgml 12 0
src/backend/commands/analyze.c 16 7
src/backend/commands/vacuum.c 21 12
src/backend/parser/gram.y 2 0
src/include/nodes/parsenodes.h 1 1
src/test/isolation/expected/vacuum-nowait.out 67 0
src/test/isolation/isolation_schedule 1 0
src/test/isolation/specs/vacuum-nowait.spec 42 0
src/test/regress/expected/vacuum.out 4 1
src/test/regress/sql/vacuum.sql 5 1
diff --git a/doc/src/sgml/ref/analyze.sgml b/doc/src/sgml/ref/analyze.sgml
index 10b3a9a..05882c3 100644
--- a/doc/src/sgml/ref/analyze.sgml
+++ b/doc/src/sgml/ref/analyze.sgml
@@ -27,6 +27,7 @@ ANALYZE [ VERBOSE ] [ <replaceable class="parameter">table_and_columns</replacea
 <phrase>where <replaceable class="parameter">option</replaceable> can be one of:</phrase>
 
     VERBOSE
+    NOWAIT
 
 <phrase>and <replaceable class="parameter">table_and_columns</replaceable> is:</phrase>
 
@@ -77,6 +78,17 @@ ANALYZE [ VERBOSE ] [ <replaceable class="parameter">table_and_columns</replacea
    </varlistentry>
 
    <varlistentry>
+    <term><literal>NOWAIT</literal></term>
+    <listitem>
+     <para>
+      Specifies that <command>ANALYZE</command> should not wait for any
+      conflicting locks to be released: if a relation cannot be locked
+      immediately without waiting, the relation is skipped.
+     </para>
+    </listitem>
+   </varlistentry>
+
+   <varlistentry>
     <term><replaceable class="parameter">table_name</replaceable></term>
     <listitem>
      <para>
diff --git a/doc/src/sgml/ref/vacuum.sgml b/doc/src/sgml/ref/vacuum.sgml
index b760e8e..1d7ffef 100644
--- a/doc/src/sgml/ref/vacuum.sgml
+++ b/doc/src/sgml/ref/vacuum.sgml
@@ -31,6 +31,7 @@ VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] [ ANALYZE ] [ <replaceable class="paramet
     VERBOSE
     ANALYZE
     DISABLE_PAGE_SKIPPING
+    NOWAIT
 
 <phrase>and <replaceable class="parameter">table_and_columns</replaceable> is:</phrase>
 
@@ -161,6 +162,17 @@ VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] [ ANALYZE ] [ <replaceable class="paramet
    </varlistentry>
 
    <varlistentry>
+    <term><literal>NOWAIT</literal></term>
+    <listitem>
+     <para>
+      Specifies that <command>VACUUM</command> should not wait for any
+      conflicting locks to be released: if a relation cannot be locked
+      immediately without waiting, the relation is skipped.
+     </para>
+    </listitem>
+   </varlistentry>
+
+   <varlistentry>
     <term><replaceable class="parameter">table_name</replaceable></term>
     <listitem>
      <para>
diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c
index f952b3c..e3e2444 100644
--- a/src/backend/commands/analyze.c
+++ b/src/backend/commands/analyze.c
@@ -159,13 +159,22 @@ analyze_rel(Oid relid, RangeVar *relation, int options,
 	 */
 	if (!onerel)
 	{
+		char	   *relname = NULL;
+
 		/*
-		 * If the RangeVar is not defined, we do not have enough information
-		 * to provide a meaningful log statement.  Chances are that
-		 * analyze_rel's caller has intentionally not provided this
-		 * information so that this logging is skipped, anyway.
+		 * If relation is NULL, we do not have enough information to provide a
+		 * meaningful log statement.  Chances are that this information was
+		 * intentionally not provided so that this logging is skipped, anyway.
+		 * However, if VACOPT_NOWAIT is specified, we always try to emit a log
+		 * statement if a relation is skipped, even if the provided RangeVar
+		 * is NULL.
 		 */
-		if (relation == NULL)
+		if (relation != NULL)
+			relname = relation->relname;
+		else if (!rel_lock)
+			relname = get_rel_name(relid);
+
+		if (relname == NULL)
 			return;
 
 		/*
@@ -185,12 +194,12 @@ analyze_rel(Oid relid, RangeVar *relation, int options,
 			ereport(elevel,
 					(errcode(ERRCODE_LOCK_NOT_AVAILABLE),
 					 errmsg("skipping analyze of \"%s\" --- lock not available",
-							relation->relname)));
+							relname)));
 		else
 			ereport(elevel,
 					(errcode(ERRCODE_UNDEFINED_TABLE),
 					 errmsg("skipping analyze of \"%s\" --- relation no longer exists",
-							relation->relname)));
+							relname)));
 
 		return;
 	}
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index 4abe6b1..60ac885 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -47,6 +47,7 @@
 #include "utils/acl.h"
 #include "utils/fmgroids.h"
 #include "utils/guc.h"
+#include "utils/lsyscache.h"
 #include "utils/memutils.h"
 #include "utils/snapmgr.h"
 #include "utils/syscache.h"
@@ -1411,21 +1412,29 @@ vacuum_rel(Oid relid, RangeVar *relation, int options, VacuumParams *params)
 	if (!onerel)
 	{
 		int			elevel = 0;
+		char	   *relname = NULL;
+
+		/*
+		 * If relation is NULL, we do not have enough information to provide a
+		 * meaningful log statement.  Chances are that this information was
+		 * intentionally not provided so that this logging is skipped, anyway.
+		 * However, if VACOPT_NOWAIT is specified, we always try to emit a log
+		 * statement if a relation is skipped, even if the provided RangeVar
+		 * is NULL.
+		 */
+		if (relation != NULL)
+			relname = relation->relname;
+		else if (!rel_lock)
+			relname = get_rel_name(relid);
 
 		/*
 		 * Determine the log level.
 		 *
-		 * If the RangeVar is not defined, we do not have enough information
-		 * to provide a meaningful log statement.  Chances are that
-		 * vacuum_rel's caller has intentionally not provided this information
-		 * so that this logging is skipped, anyway.
-		 *
-		 * Otherwise, for autovacuum logs, we emit a LOG if
-		 * log_autovacuum_min_duration is not disabled.  For manual VACUUM, we
-		 * emit a WARNING to match the log statements in the permission
-		 * checks.
+		 * For autovacuum logs, we emit a LOG if log_autovacuum_min_duration
+		 * is not disabled.  For manual VACUUM, we emit a WARNING to match the
+		 * log statements in the permission checks.
 		 */
-		if (relation != NULL)
+		if (relname != NULL)
 		{
 			if (!IsAutoVacuumWorkerProcess())
 				elevel = WARNING;
@@ -1439,12 +1448,12 @@ vacuum_rel(Oid relid, RangeVar *relation, int options, VacuumParams *params)
 				ereport(elevel,
 						(errcode(ERRCODE_LOCK_NOT_AVAILABLE),
 						 errmsg("skipping vacuum of \"%s\" --- lock not available",
-								relation->relname)));
+								relname)));
 			else
 				ereport(elevel,
 						(errcode(ERRCODE_UNDEFINED_TABLE),
 						 errmsg("skipping vacuum of \"%s\" --- relation no longer exists",
-								relation->relname)));
+								relname)));
 		}
 
 		PopActiveSnapshot();
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index d0a5203..f7d5fb5 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -10507,6 +10507,7 @@ vacuum_option_elem:
 							 errmsg("unrecognized VACUUM option \"%s\"", $1),
 									 parser_errposition(@1)));
 				}
+			| NOWAIT			{ $$ = VACOPT_NOWAIT; }
 		;
 
 AnalyzeStmt: analyze_keyword opt_verbose opt_vacuum_relation_list
@@ -10534,6 +10535,7 @@ analyze_option_list:
 
 analyze_option_elem:
 			VERBOSE				{ $$ = VACOPT_VERBOSE; }
+			| NOWAIT			{ $$ = VACOPT_NOWAIT; }
 		;
 
 analyze_keyword:
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 2eaa6b2..27e06fb 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -3116,7 +3116,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_NOWAIT = 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-nowait.out b/src/test/isolation/expected/vacuum-nowait.out
new file mode 100644
index 0000000..a65f8be
--- /dev/null
+++ b/src/test/isolation/expected/vacuum-nowait.out
@@ -0,0 +1,67 @@
+Parsed test spec with 2 sessions
+
+starting permutation: lock vac_specified commit
+step lock: 
+	BEGIN;
+	LOCK part1 IN SHARE MODE;
+
+WARNING:  skipping vacuum of "part1" --- lock not available
+step vac_specified: VACUUM (NOWAIT) part1, part2;
+step commit: 
+	COMMIT;
+
+
+starting permutation: lock vac_all_parts commit
+step lock: 
+	BEGIN;
+	LOCK part1 IN SHARE MODE;
+
+WARNING:  skipping vacuum of "part1" --- lock not available
+step vac_all_parts: VACUUM (NOWAIT) parted;
+step commit: 
+	COMMIT;
+
+
+starting permutation: lock analyze_specified commit
+step lock: 
+	BEGIN;
+	LOCK part1 IN SHARE MODE;
+
+WARNING:  skipping analyze of "part1" --- lock not available
+step analyze_specified: ANALYZE (NOWAIT) part1, part2;
+step commit: 
+	COMMIT;
+
+
+starting permutation: lock analyze_all_parts commit
+step lock: 
+	BEGIN;
+	LOCK part1 IN SHARE MODE;
+
+WARNING:  skipping analyze of "part1" --- lock not available
+step analyze_all_parts: ANALYZE (NOWAIT) parted;
+step commit: 
+	COMMIT;
+
+
+starting permutation: lock vac_analyze_specified commit
+step lock: 
+	BEGIN;
+	LOCK part1 IN SHARE MODE;
+
+WARNING:  skipping vacuum of "part1" --- lock not available
+step vac_analyze_specified: VACUUM (ANALYZE, NOWAIT) part1, part2;
+step commit: 
+	COMMIT;
+
+
+starting permutation: lock vac_analyze_all_parts commit
+step lock: 
+	BEGIN;
+	LOCK part1 IN SHARE MODE;
+
+WARNING:  skipping vacuum of "part1" --- lock not available
+step vac_analyze_all_parts: VACUUM (ANALYZE, NOWAIT) parted;
+step commit: 
+	COMMIT;
+
diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule
index eb566eb..c1c09ea 100644
--- a/src/test/isolation/isolation_schedule
+++ b/src/test/isolation/isolation_schedule
@@ -64,3 +64,4 @@ test: async-notify
 test: vacuum-reltuples
 test: timeouts
 test: vacuum-concurrent-drop
+test: vacuum-nowait
diff --git a/src/test/isolation/specs/vacuum-nowait.spec b/src/test/isolation/specs/vacuum-nowait.spec
new file mode 100644
index 0000000..ed27d4f
--- /dev/null
+++ b/src/test/isolation/specs/vacuum-nowait.spec
@@ -0,0 +1,42 @@
+# Test for the NOWAIT option for VACUUM and ANALYZE commands.
+#
+# Skipped relations should be logged regardless of whether it was explicitly
+# specified in the 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"
+{
+	BEGIN;
+	LOCK part1 IN SHARE MODE;
+}
+step "commit"
+{
+	COMMIT;
+}
+
+session "s2"
+step "vac_specified"		{ VACUUM (NOWAIT) part1, part2; }
+step "vac_all_parts"		{ VACUUM (NOWAIT) parted; }
+step "analyze_specified"	{ ANALYZE (NOWAIT) part1, part2; }
+step "analyze_all_parts"	{ ANALYZE (NOWAIT) parted; }
+step "vac_analyze_specified"	{ VACUUM (ANALYZE, NOWAIT) part1, part2; }
+step "vac_analyze_all_parts"	{ VACUUM (ANALYZE, NOWAIT) parted; }
+
+permutation "lock" "vac_specified" "commit"
+permutation "lock" "vac_all_parts" "commit"
+permutation "lock" "analyze_specified" "commit"
+permutation "lock" "analyze_all_parts" "commit"
+permutation "lock" "vac_analyze_specified" "commit"
+permutation "lock" "vac_analyze_all_parts" "commit"
diff --git a/src/test/regress/expected/vacuum.out b/src/test/regress/expected/vacuum.out
index 199dda6..99aad21 100644
--- a/src/test/regress/expected/vacuum.out
+++ b/src/test/regress/expected/vacuum.out
@@ -113,8 +113,11 @@ ERROR:  relation "does_not_exist" does not exist
 ANALYZE vactst (i), vacparted (does_not_exist);
 ERROR:  column "does_not_exist" of relation "vacparted" does not exist
 -- parenthesized syntax for ANALYZE
-ANALYZE (VERBOSE) does_not_exist;
+ANALYZE (NOWAIT, VERBOSE) does_not_exist;
 ERROR:  relation "does_not_exist" does not exist
+-- nowait option
+VACUUM (NOWAIT) vactst;
+ANALYZE (NOWAIT) 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 e7b7a7f..5e88c68 100644
--- a/src/test/regress/sql/vacuum.sql
+++ b/src/test/regress/sql/vacuum.sql
@@ -90,7 +90,11 @@ ANALYZE vactst, does_not_exist, vacparted;
 ANALYZE vactst (i), vacparted (does_not_exist);
 
 -- parenthesized syntax for ANALYZE
-ANALYZE (VERBOSE) does_not_exist;
+ANALYZE (NOWAIT, VERBOSE) does_not_exist;
+
+-- nowait option
+VACUUM (NOWAIT) vactst;
+ANALYZE (NOWAIT) vactst;
 
 DROP TABLE vaccluster;
 DROP TABLE vactst;