Reduce lock levels for ALTER TABLE SET autovacuum storage options

Reduce lock levels down to ShareUpdateExclusiveLock for all autovacuum-related
relation options when setting them using ALTER TABLE.

Add infrastructure to allow varying lock levels for relation options in later
patches. Setting multiple options together uses the highest lock level required
for any option. Works for both main and toast tables.

Fabrízio Mello, reviewed by Michael Paquier, mild edit and additional regression
tests from myself

doc/src/sgml/ref/alter_table.sgml

@@ -543,6 +543,10 @@ ALTER TABLE ALL IN TABLESPACE <replaceable class="PARAMETER">name</replaceable>
       of <command>ALTER TABLE</> that forces a table rewrite.
      </para>
 
+     <para>
+      Changing autovacuum storage parameters acquires a <literal>SHARE UPDATE EXCLUSIVE</literal> lock.
+     </para>
+
      <note>
       <para>
        While <command>CREATE TABLE</> allows <literal>OIDS</> to be specified

src/backend/access/common/reloptions.c

@@ -57,7 +57,8 @@ static relopt_bool boolRelOpts[] =
 		{
 			"autovacuum_enabled",
 			"Enables autovacuum in this relation",
-			RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
+			RELOPT_KIND_HEAP | RELOPT_KIND_TOAST,
+			ShareUpdateExclusiveLock
 		},
 		true
 	},
@@ -65,7 +66,8 @@ static relopt_bool boolRelOpts[] =
 		{
 			"user_catalog_table",
 			"Declare a table as an additional catalog table, e.g. for the purpose of logical replication",
-			RELOPT_KIND_HEAP
+			RELOPT_KIND_HEAP,
+			AccessExclusiveLock
 		},
 		false
 	},
@@ -73,7 +75,8 @@ static relopt_bool boolRelOpts[] =
 		{
 			"fastupdate",
 			"Enables \"fast update\" feature for this GIN index",
-			RELOPT_KIND_GIN
+			RELOPT_KIND_GIN,
+			AccessExclusiveLock
 		},
 		true
 	},
@@ -81,7 +84,8 @@ static relopt_bool boolRelOpts[] =
 		{
 			"security_barrier",
 			"View acts as a row security barrier",
-			RELOPT_KIND_VIEW
+			RELOPT_KIND_VIEW,
+			AccessExclusiveLock
 		},
 		false
 	},
@@ -95,7 +99,8 @@ static relopt_int intRelOpts[] =
 		{
 			"fillfactor",
 			"Packs table pages only to this percentage",
-			RELOPT_KIND_HEAP
+			RELOPT_KIND_HEAP,
+			AccessExclusiveLock
 		},
 		HEAP_DEFAULT_FILLFACTOR, HEAP_MIN_FILLFACTOR, 100
 	},
@@ -103,7 +108,8 @@ static relopt_int intRelOpts[] =
 		{
 			"fillfactor",
 			"Packs btree index pages only to this percentage",
-			RELOPT_KIND_BTREE
+			RELOPT_KIND_BTREE,
+			AccessExclusiveLock
 		},
 		BTREE_DEFAULT_FILLFACTOR, BTREE_MIN_FILLFACTOR, 100
 	},
@@ -111,7 +117,8 @@ static relopt_int intRelOpts[] =
 		{
 			"fillfactor",
 			"Packs hash index pages only to this percentage",
-			RELOPT_KIND_HASH
+			RELOPT_KIND_HASH,
+			AccessExclusiveLock
 		},
 		HASH_DEFAULT_FILLFACTOR, HASH_MIN_FILLFACTOR, 100
 	},
@@ -119,7 +126,8 @@ static relopt_int intRelOpts[] =
 		{
 			"fillfactor",
 			"Packs gist index pages only to this percentage",
-			RELOPT_KIND_GIST
+			RELOPT_KIND_GIST,
+			AccessExclusiveLock
 		},
 		GIST_DEFAULT_FILLFACTOR, GIST_MIN_FILLFACTOR, 100
 	},
@@ -127,7 +135,8 @@ static relopt_int intRelOpts[] =
 		{
 			"fillfactor",
 			"Packs spgist index pages only to this percentage",
-			RELOPT_KIND_SPGIST
+			RELOPT_KIND_SPGIST,
+			AccessExclusiveLock
 		},
 		SPGIST_DEFAULT_FILLFACTOR, SPGIST_MIN_FILLFACTOR, 100
 	},
@@ -135,7 +144,8 @@ static relopt_int intRelOpts[] =
 		{
 			"autovacuum_vacuum_threshold",
 			"Minimum number of tuple updates or deletes prior to vacuum",
-			RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
+			RELOPT_KIND_HEAP | RELOPT_KIND_TOAST,
+			ShareUpdateExclusiveLock
 		},
 		-1, 0, INT_MAX
 	},
@@ -143,7 +153,8 @@ static relopt_int intRelOpts[] =
 		{
 			"autovacuum_analyze_threshold",
 			"Minimum number of tuple inserts, updates or deletes prior to analyze",
-			RELOPT_KIND_HEAP
+			RELOPT_KIND_HEAP,
+			ShareUpdateExclusiveLock
 		},
 		-1, 0, INT_MAX
 	},
@@ -151,7 +162,8 @@ static relopt_int intRelOpts[] =
 		{
 			"autovacuum_vacuum_cost_delay",
 			"Vacuum cost delay in milliseconds, for autovacuum",
-			RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
+			RELOPT_KIND_HEAP | RELOPT_KIND_TOAST,
+			ShareUpdateExclusiveLock
 		},
 		-1, 0, 100
 	},
@@ -159,7 +171,8 @@ static relopt_int intRelOpts[] =
 		{
 			"autovacuum_vacuum_cost_limit",
 			"Vacuum cost amount available before napping, for autovacuum",
-			RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
+			RELOPT_KIND_HEAP | RELOPT_KIND_TOAST,
+			ShareUpdateExclusiveLock
 		},
 		-1, 1, 10000
 	},
@@ -167,7 +180,8 @@ static relopt_int intRelOpts[] =
 		{
 			"autovacuum_freeze_min_age",
 			"Minimum age at which VACUUM should freeze a table row, for autovacuum",
-			RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
+			RELOPT_KIND_HEAP | RELOPT_KIND_TOAST,
+			ShareUpdateExclusiveLock
 		},
 		-1, 0, 1000000000
 	},
@@ -175,7 +189,8 @@ static relopt_int intRelOpts[] =
 		{
 			"autovacuum_multixact_freeze_min_age",
 			"Minimum multixact age at which VACUUM should freeze a row multixact's, for autovacuum",
-			RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
+			RELOPT_KIND_HEAP | RELOPT_KIND_TOAST,
+			ShareUpdateExclusiveLock
 		},
 		-1, 0, 1000000000
 	},
@@ -183,7 +198,8 @@ static relopt_int intRelOpts[] =
 		{
 			"autovacuum_freeze_max_age",
 			"Age at which to autovacuum a table to prevent transaction ID wraparound",
-			RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
+			RELOPT_KIND_HEAP | RELOPT_KIND_TOAST,
+			ShareUpdateExclusiveLock
 		},
 		-1, 100000000, 2000000000
 	},
@@ -191,7 +207,8 @@ static relopt_int intRelOpts[] =
 		{
 			"autovacuum_multixact_freeze_max_age",
 			"Multixact age at which to autovacuum a table to prevent multixact wraparound",
-			RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
+			RELOPT_KIND_HEAP | RELOPT_KIND_TOAST,
+			ShareUpdateExclusiveLock
 		},
 		-1, 100000000, 2000000000
 	},
@@ -199,21 +216,24 @@ static relopt_int intRelOpts[] =
 		{
 			"autovacuum_freeze_table_age",
 			"Age at which VACUUM should perform a full table sweep to freeze row versions",
-			RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
+			RELOPT_KIND_HEAP | RELOPT_KIND_TOAST,
+			ShareUpdateExclusiveLock
 		}, -1, 0, 2000000000
 	},
 	{
 		{
 			"autovacuum_multixact_freeze_table_age",
 			"Age of multixact at which VACUUM should perform a full table sweep to freeze row versions",
-			RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
+			RELOPT_KIND_HEAP | RELOPT_KIND_TOAST,
+			ShareUpdateExclusiveLock
 		}, -1, 0, 2000000000
 	},
 	{
 		{
 			"log_autovacuum_min_duration",
 			"Sets the minimum execution time above which autovacuum actions will be logged",
-			RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
+			RELOPT_KIND_HEAP | RELOPT_KIND_TOAST,
+			ShareUpdateExclusiveLock
 		},
 		-1, -1, INT_MAX
 	},
@@ -221,14 +241,16 @@ static relopt_int intRelOpts[] =
 		{
 			"pages_per_range",
 			"Number of pages that each page range covers in a BRIN index",
-			RELOPT_KIND_BRIN
+			RELOPT_KIND_BRIN,
+			AccessExclusiveLock
 		}, 128, 1, 131072
 	},
 	{
 		{
 			"gin_pending_list_limit",
 			"Maximum size of the pending list for this GIN index, in kilobytes.",
-			RELOPT_KIND_GIN
+			RELOPT_KIND_GIN,
+			AccessExclusiveLock
 		},
 		-1, 64, MAX_KILOBYTES
 	},
@@ -243,7 +265,8 @@ static relopt_real realRelOpts[] =
 		{
 			"autovacuum_vacuum_scale_factor",
 			"Number of tuple updates or deletes prior to vacuum as a fraction of reltuples",
-			RELOPT_KIND_HEAP | RELOPT_KIND_TOAST
+			RELOPT_KIND_HEAP | RELOPT_KIND_TOAST,
+			ShareUpdateExclusiveLock
 		},
 		-1, 0.0, 100.0
 	},
@@ -251,7 +274,8 @@ static relopt_real realRelOpts[] =
 		{
 			"autovacuum_analyze_scale_factor",
 			"Number of tuple inserts, updates or deletes prior to analyze as a fraction of reltuples",
-			RELOPT_KIND_HEAP
+			RELOPT_KIND_HEAP,
+			ShareUpdateExclusiveLock
 		},
 		-1, 0.0, 100.0
 	},
@@ -259,7 +283,8 @@ static relopt_real realRelOpts[] =
 		{
 			"seq_page_cost",
 			"Sets the planner's estimate of the cost of a sequentially fetched disk page.",
-			RELOPT_KIND_TABLESPACE
+			RELOPT_KIND_TABLESPACE,
+			AccessExclusiveLock
 		},
 		-1, 0.0, DBL_MAX
 	},
@@ -267,7 +292,8 @@ static relopt_real realRelOpts[] =
 		{
 			"random_page_cost",
 			"Sets the planner's estimate of the cost of a nonsequentially fetched disk page.",
-			RELOPT_KIND_TABLESPACE
+			RELOPT_KIND_TABLESPACE,
+			AccessExclusiveLock
 		},
 		-1, 0.0, DBL_MAX
 	},
@@ -275,7 +301,8 @@ static relopt_real realRelOpts[] =
 		{
 			"n_distinct",
 			"Sets the planner's estimate of the number of distinct values appearing in a column (excluding child relations).",
-			RELOPT_KIND_ATTRIBUTE
+			RELOPT_KIND_ATTRIBUTE,
+			AccessExclusiveLock
 		},
 		0, -1.0, DBL_MAX
 	},
@@ -283,7 +310,8 @@ static relopt_real realRelOpts[] =
 		{
 			"n_distinct_inherited",
 			"Sets the planner's estimate of the number of distinct values appearing in a column (including child relations).",
-			RELOPT_KIND_ATTRIBUTE
+			RELOPT_KIND_ATTRIBUTE,
+			AccessExclusiveLock
 		},
 		0, -1.0, DBL_MAX
 	},
@@ -297,7 +325,8 @@ static relopt_string stringRelOpts[] =
 		{
 			"buffering",
 			"Enables buffering build for this GiST index",
-			RELOPT_KIND_GIST
+			RELOPT_KIND_GIST,
+			AccessExclusiveLock
 		},
 		4,
 		false,
@@ -308,7 +337,8 @@ static relopt_string stringRelOpts[] =
 		{
 			"check_option",
 			"View has WITH CHECK OPTION defined (local or cascaded).",
-			RELOPT_KIND_VIEW
+			RELOPT_KIND_VIEW,
+			AccessExclusiveLock
 		},
 		0,
 		true,
@@ -344,13 +374,29 @@ initialize_reloptions(void)
 
 	j = 0;
 	for (i = 0; boolRelOpts[i].gen.name; i++)
+	{
+		Assert(DoLockModesConflict(boolRelOpts[i].gen.lockmode,
+								   boolRelOpts[i].gen.lockmode));
 		j++;
+	}
 	for (i = 0; intRelOpts[i].gen.name; i++)
+	{
+		Assert(DoLockModesConflict(intRelOpts[i].gen.lockmode,
+								   intRelOpts[i].gen.lockmode));
 		j++;
+	}
 	for (i = 0; realRelOpts[i].gen.name; i++)
+	{
+		Assert(DoLockModesConflict(realRelOpts[i].gen.lockmode,
+								   realRelOpts[i].gen.lockmode));
 		j++;
+	}
 	for (i = 0; stringRelOpts[i].gen.name; i++)
+	{
+		Assert(DoLockModesConflict(stringRelOpts[i].gen.lockmode,
+								   stringRelOpts[i].gen.lockmode));
 		j++;
+	}
 	j += num_custom_options;
 
 	if (relOpts)
@@ -1411,3 +1457,41 @@ tablespace_reloptions(Datum reloptions, bool validate)
 
 	return (bytea *) tsopts;
 }
+
+/*
+ * Determine the required LOCKMODE from an option list.
+ *
+ * Called from AlterTableGetLockLevel(), see that function
+ * for a longer explanation of how this works.
+ */
+LOCKMODE
+AlterTableGetRelOptionsLockLevel(List *defList)
+{
+	LOCKMODE    lockmode = NoLock;
+	ListCell    *cell;
+
+	if (defList == NIL)
+		return AccessExclusiveLock;
+
+	if (need_initialization)
+		initialize_reloptions();
+
+	foreach(cell, defList)
+	{
+		DefElem *def = (DefElem *) lfirst(cell);
+		int		i;
+
+		for (i = 0; relOpts[i]; i++)
+		{
+			if (pg_strncasecmp(relOpts[i]->name,
+							   def->defname,
+							   relOpts[i]->namelen + 1) == 0)
+			{
+				if (lockmode < relOpts[i]->lockmode)
+					lockmode = relOpts[i]->lockmode;
+			}
+		}
+	}
+
+	return lockmode;
+}

src/backend/commands/tablecmds.c

@@ -3038,16 +3038,12 @@ AlterTableGetLockLevel(List *cmds)
 				 * are set here for tables, views and indexes; for historical
 				 * reasons these can all be used with ALTER TABLE, so we can't
 				 * decide between them using the basic grammar.
-				 *
-				 * XXX Look in detail at each option to determine lock level,
-				 * e.g. cmd_lockmode = GetRelOptionsLockLevel((List *)
-				 * cmd->def);
 				 */
 			case AT_SetRelOptions:		/* Uses MVCC in getIndexes() and
 										 * getTables() */
 			case AT_ResetRelOptions:	/* Uses MVCC in getIndexes() and
 										 * getTables() */
-				cmd_lockmode = AccessExclusiveLock;
+				cmd_lockmode = AlterTableGetRelOptionsLockLevel((List *) cmd->def);
 				break;
 
 			default:			/* oops */

src/include/access/reloptions.h

@@ -22,6 +22,7 @@
 #include "access/htup.h"
 #include "access/tupdesc.h"
 #include "nodes/pg_list.h"
+#include "storage/lock.h"
 
 /* types supported by reloptions */
 typedef enum relopt_type
@@ -62,6 +63,7 @@ typedef struct relopt_gen
 								 * marker) */
 	const char *desc;
 	bits32		kinds;
+	LOCKMODE	lockmode;
 	int			namelen;
 	relopt_type type;
 } relopt_gen;
@@ -274,5 +276,6 @@ extern bytea *index_reloptions(RegProcedure amoptions, Datum reloptions,
 				 bool validate);
 extern bytea *attribute_reloptions(Datum reloptions, bool validate);
 extern bytea *tablespace_reloptions(Datum reloptions, bool validate);
+extern LOCKMODE AlterTableGetRelOptionsLockLevel(List *defList);
 
 #endif   /* RELOPTIONS_H */

src/test/regress/expected/alter_table.out

@@ -1914,19 +1914,19 @@ select * from my_locks order by 1;
 commit;
 begin; alter table alterlock set (toast.autovacuum_enabled = off);
 select * from my_locks order by 1;
-  relname  |    max_lockmode     
------------+---------------------
- alterlock | AccessExclusiveLock
- pg_toast  | AccessExclusiveLock
+  relname  |       max_lockmode       
+-----------+--------------------------
+ alterlock | ShareUpdateExclusiveLock
+ pg_toast  | ShareUpdateExclusiveLock
 (2 rows)
 
 commit;
 begin; alter table alterlock set (autovacuum_enabled = off);
 select * from my_locks order by 1;
-  relname  |    max_lockmode     
------------+---------------------
- alterlock | AccessExclusiveLock
- pg_toast  | AccessExclusiveLock
+  relname  |       max_lockmode       
+-----------+--------------------------
+ alterlock | ShareUpdateExclusiveLock
+ pg_toast  | ShareUpdateExclusiveLock
 (2 rows)
 
 commit;
@@ -1938,6 +1938,16 @@ select * from my_locks order by 1;
 (1 row)
 
 rollback;
+-- test that mixing options with different lock levels works as expected
+begin; alter table alterlock set (autovacuum_enabled = off, fillfactor = 80);
+select * from my_locks order by 1;
+  relname  |    max_lockmode     
+-----------+---------------------
+ alterlock | AccessExclusiveLock
+ pg_toast  | AccessExclusiveLock
+(2 rows)
+
+commit;
 begin; alter table alterlock alter column f2 set storage extended;
 select * from my_locks order by 1;
   relname  |    max_lockmode     
@@ -2006,6 +2016,47 @@ select * from my_locks order by 1;
  alterlock_pkey  | AccessShareLock
 (4 rows)
 
+rollback;
+create or replace view my_locks as
+select case when c.relname like 'pg_toast%' then 'pg_toast' else c.relname end, max(mode::lockmodes) as max_lockmode
+from pg_locks l join pg_class c on l.relation = c.oid
+where virtualtransaction = (
+        select virtualtransaction
+        from pg_locks
+        where transactionid = txid_current()::integer)
+and locktype = 'relation'
+and relnamespace != (select oid from pg_namespace where nspname = 'pg_catalog')
+and c.relname = 'my_locks'
+group by c.relname;
+-- raise exception
+alter table my_locks set (autovacuum_enabled = false);
+ERROR:  unrecognized parameter "autovacuum_enabled"
+alter view my_locks set (autovacuum_enabled = false);
+ERROR:  unrecognized parameter "autovacuum_enabled"
+alter table my_locks reset (autovacuum_enabled);
+alter view my_locks reset (autovacuum_enabled);
+begin;
+alter view my_locks set (security_barrier=off);
+select * from my_locks order by 1;
+ relname  |    max_lockmode     
+----------+---------------------
+ my_locks | AccessExclusiveLock
+(1 row)
+
+alter view my_locks reset (security_barrier);
+rollback;
+-- this test intentionally applies the ALTER TABLE command against a view, but
+-- uses a view option so we expect this to succeed. This form of SQL is
+-- accepted for historical reasons, as shown in the docs for ALTER VIEW
+begin;
+alter table my_locks set (security_barrier=off);
+select * from my_locks order by 1;
+ relname  |    max_lockmode     
+----------+---------------------
+ my_locks | AccessExclusiveLock
+(1 row)
+
+alter table my_locks reset (security_barrier);
 rollback;
 -- cleanup
 drop table alterlock2;

src/test/regress/sql/alter_table.sql

@@ -1332,6 +1332,11 @@ begin; alter table alterlock alter column f2 set (n_distinct = 1);
 select * from my_locks order by 1;
 rollback;
 
+-- test that mixing options with different lock levels works as expected
+begin; alter table alterlock set (autovacuum_enabled = off, fillfactor = 80);
+select * from my_locks order by 1;
+commit;
+
 begin; alter table alterlock alter column f2 set storage extended;
 select * from my_locks order by 1;
 rollback;
@@ -1365,6 +1370,39 @@ alter table alterlock2 validate constraint alterlock2nv;
 select * from my_locks order by 1;
 rollback;
 
+create or replace view my_locks as
+select case when c.relname like 'pg_toast%' then 'pg_toast' else c.relname end, max(mode::lockmodes) as max_lockmode
+from pg_locks l join pg_class c on l.relation = c.oid
+where virtualtransaction = (
+        select virtualtransaction
+        from pg_locks
+        where transactionid = txid_current()::integer)
+and locktype = 'relation'
+and relnamespace != (select oid from pg_namespace where nspname = 'pg_catalog')
+and c.relname = 'my_locks'
+group by c.relname;
+
+-- raise exception
+alter table my_locks set (autovacuum_enabled = false);
+alter view my_locks set (autovacuum_enabled = false);
+alter table my_locks reset (autovacuum_enabled);
+alter view my_locks reset (autovacuum_enabled);
+
+begin;
+alter view my_locks set (security_barrier=off);
+select * from my_locks order by 1;
+alter view my_locks reset (security_barrier);
+rollback;
+
+-- this test intentionally applies the ALTER TABLE command against a view, but
+-- uses a view option so we expect this to succeed. This form of SQL is
+-- accepted for historical reasons, as shown in the docs for ALTER VIEW
+begin;
+alter table my_locks set (security_barrier=off);
+select * from my_locks order by 1;
+alter table my_locks reset (security_barrier);
+rollback;
+
 -- cleanup
 drop table alterlock2;
 drop table alterlock;