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

diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml
index 1c1c18168e0c7656ac7bbcf4b833034424422c57..ad985cd73110037328aba9bc29097d48d3ea58e3 100644 (file)
--- a/doc/src/sgml/ref/alter_table.sgml
+++ b/doc/src/sgml/ref/alter_table.sgml
@@ -543,6 +543,10 @@ ALTER TABLE ALL IN TABLESPACE name
       of ALTER TABLE that forces a table rewrite.
      
 
+     
+      Changing autovacuum storage parameters acquires a SHARE UPDATE EXCLUSIVE lock.
+     
+
      
       
        While CREATE TABLE allows OIDS to be specified

diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c
index 180f529060d1a502e3a52fe916626bba77fa2f1c..7479d40b67b360befe9af5ef8fa2e9d83e1a604d 100644 (file)
--- 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,
+           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;
+}

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 970abd4b50c9240f5c67c355604e3453218f6f49..126b11923f06c53f7afd0fad08c36e51c8cde7af 100644 (file)
--- a/src/backend/commands/tablecmds.c
+++ b/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 */

diff --git a/src/include/access/reloptions.h b/src/include/access/reloptions.h
index e7b6bb52cad8c96a78628f0c8699fc115814ed0c..2a3cbcdd7089fb97b95357cc45a40a5194de313a 100644 (file)
--- 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 AlterTableGetRelOptionsLockLevel(List *defList);
 
 #endif   /* RELOPTIONS_H */

diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index 47871b2be4f81a0697680303b5c4e0270befbc30..28422eaaf01430c1c8408b1619d884ccccc8932a 100644 (file)
--- a/src/test/regress/expected/alter_table.out
+++ b/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;

diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index 63cca34cd75058ad622e165f1f54fa2fce2ccf95..3ef55d943163314d11d3cdb472c4d35f4d5452be 100644 (file)
--- a/src/test/regress/sql/alter_table.sql
+++ b/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;