On Wed, Apr 1, 2015 at 1:45 AM, Noah Misch <n...@leadboat.com> wrote:
>
> On Tue, Mar 31, 2015 at 01:17:03PM -0400, Robert Haas wrote:
> > On Tue, Mar 31, 2015 at 9:11 AM, Fabrízio de Royes Mello
> > <fabriziome...@gmail.com> wrote:
> > > Attached a very WIP patch to reduce lock level when setting autovacuum
> > > reloptions in "ALTER TABLE .. SET ( .. )" statement.
> >
> > I think the first thing we need to here is analyze all of the options
> > and determine what the appropriate lock level is for each, and why.
>
> Agreed.  Fabrízio, see this message for the discussion that led to the
code
> comment you found (search for "relopt_gen"):
>
>
http://www.postgresql.org/message-id/20140321034556.ga3927...@tornado.leadboat.com

Ok guys. The attached patch refactor the reloptions adding a new field
"lockmode" in "relopt_gen" struct and a new method to determine the
required lock level from an option list.

We need determine the appropriate lock level for each reloption:

- boolRelopts:
  * autovacuum_enabled  (AccessShareLock)
  * user_catalog_table  (AccessExclusiveLock)
  * fastupdate  (AccessExclusiveLock)
  * security_barrier  (AccessExclusiveLock)

- intRelOpts:
  * fillfactor (heap)  (AccessExclusiveLock)
  * fillfactor (btree)  (AccessExclusiveLock)
  * fillfactor (gist)  (AccessExclusiveLock)
  * fillfactor (spgist)  (AccessExclusiveLock)
  * autovacuum_vacuum_threshold  (AccessShareLock)
  * autovacuum_analyze_threshold  (AccessShareLock)
  * autovacuum_vacuum_cost_delay  (AccessShareLock)
  * autovacuum_vacuum_cost_limit  (AccessShareLock)
  * autovacuum_freeze_min_age  (AccessShareLock)
  * autovacuum_multixact_freeze_min_age  (AccessShareLock)
  * autovacuum_freeze_max_age  (AccessShareLock)
  * autovacuum_multixact_freeze_max_age  (AccessShareLock)
  * autovacuum_freeze_table_age  (AccessShareLock)
  * autovacuum_multixact_freeze_table_age  (AccessShareLock)
  * log_autovacuum_min_duration  (AccessShareLock)
  * pages_per_range  (AccessExclusiveLock)
  * gin_pending_list_limit  (AccessExclusiveLock)

- realRelOpts:
  * autovacuum_vacuum_scale_factor  (AccessShareLock)
  * autovacuum_analyze_scale_factor  (AccessShareLock)
  * seq_page_cost  (AccessExclusiveLock)
  * random_page_cost  (AccessExclusiveLock)
  * n_distinct  (AccessExclusiveLock)
  * n_distinct_inherited  (AccessExclusiveLock)

- stringRelOpts:
  * buffering  (AccessExclusiveLock)
  * check_option  (AccessExclusiveLock)


In the above list I just change lock level from AccessExclusiveLock to
AccessShareLock to all "autovacuum" related reloptions because it was the
motivation of this patch.

I need some help to define the others.

Regards,

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog: http://fabriziomello.github.io
>> Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
>> Github: http://github.com/fabriziomello
diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c
index 8176b6a..f83ce2f 100644
--- a/src/backend/access/common/reloptions.c
+++ b/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,
+			AccessExclusiveLock
 		},
 		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,
+			AccessExclusiveLock
 		},
 		-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,
+			AccessExclusiveLock
 		},
 		-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,
+			AccessExclusiveLock
 		},
 		-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,
+			AccessExclusiveLock
 		},
 		-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,
+			AccessExclusiveLock
 		},
 		-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,
+			AccessExclusiveLock
 		},
 		-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,
+			AccessExclusiveLock
 		},
 		-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,
+			AccessExclusiveLock
 		},
 		-1, 100000000, 2000000000
 	},
@@ -199,14 +216,16 @@ 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,
+			AccessExclusiveLock
 		}, -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,
+			AccessExclusiveLock
 		}, -1, 0, 2000000000
 	},
 	{
@@ -221,14 +240,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 +264,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,
+			AccessExclusiveLock
 		},
 		-1, 0.0, 100.0
 	},
@@ -251,7 +273,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,
+			AccessExclusiveLock
 		},
 		-1, 0.0, 100.0
 	},
@@ -259,7 +282,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 +291,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 +300,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 +309,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 +324,8 @@ static relopt_string stringRelOpts[] =
 		{
 			"buffering",
 			"Enables buffering build for this GiST index",
-			RELOPT_KIND_GIST
+			RELOPT_KIND_GIST,
+			AccessExclusiveLock
 		},
 		4,
 		false,
@@ -308,7 +336,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,
@@ -1406,3 +1435,36 @@ tablespace_reloptions(Datum reloptions, bool validate)
 
 	return (bytea *) tsopts;
 }
+
+/*
+ * Determine the required LOCKMODE from an option list
+ */
+LOCKMODE
+GetRelOptionsLockLevel(List *defList)
+{
+	LOCKMODE    lockmode = NoLock;
+	ListCell    *cell;
+
+	if (defList == NIL)
+		return lockmode;
+
+	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;
+}
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 06e4332..41462af 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -3025,16 +3025,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 = GetRelOptionsLockLevel((List *) cmd->def);
 				break;
 
 			default:			/* oops */
diff --git a/src/include/access/reloptions.h b/src/include/access/reloptions.h
index e7b6bb5..c444c71 100644
--- a/src/include/access/reloptions.h
+++ b/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 GetRelOptionsLockLevel(List *defList);
 
 #endif   /* RELOPTIONS_H */
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to