On Mon, Mar 30, 2015 at 8:14 PM, Fabrízio de Royes Mello <
fabriziome...@gmail.com> wrote:
>
>
>
> On Mon, Mar 30, 2015 at 7:41 PM, Jim Nasby <jim.na...@bluetreble.com>
wrote:
> >
> > On 3/27/15 2:23 PM, Fabrízio de Royes Mello wrote:
> >>
> >> Hi all,
> >>
> >> I'm tweaking some autovacuum settings in a table with high write usage
> >> but with ALTER TABLE .. SET ( .. ) this task was impossible, so I did a
> >> catalog update (pg_class) to change reloptions.
> >>
> >> Maybe it's a stupid doubt, but why we need to get an
AccessExclusiveLock
> >> on relation to set reloptions if we just touch in pg_class tuples
> >> (RowExclusiveLock) ?
> >
> >
> > For a very long time catalog access was not MVCC safe. I think that's
been changed, so at this point it may be OK to relax the lock, at least in
the case of autovac settings. There may well be other settings in there
where it would not be safe.
> >
>
> Hummm.... There are a comment in AlterTableGetLockLevel:
>
> 3017 /*
> 3018 * Rel options are more complex than first appears.
Options
> 3019 * are set here for tables, views and indexes; for
historical
> 3020 * reasons these can all be used with ALTER TABLE,
so we can't
> 3021 * decide between them using the basic grammar.
> 3022 *
> 3023 * XXX Look in detail at each option to determine
lock level,
> 3024 * e.g. cmd_lockmode = GetRelOptionsLockLevel((List
*)
> 3025 * cmd->def);
> 3026 */
> 3027 case AT_SetRelOptions: /* Uses MVCC in
getIndexes() and
> 3028 * getTables() */
> 3029 case AT_ResetRelOptions: /* Uses MVCC in
getIndexes() and
> 3030 * getTables() */
> 3031 cmd_lockmode = AccessExclusiveLock;
> 3032 break;
>
>
> Maybe it's time to implement "GetRelOptionsLockLevel" to relax the lock
to autovac settings (AccessShareLock). To other settings we continue using
AccessExclusiveLock.
>
> There are some objection to implement in that way?
>
Attached a very WIP patch to reduce lock level when setting autovacuum
reloptions in "ALTER TABLE .. SET ( .. )" statement.
I confess the implementation is ugly, maybe we should add a new item to
reloptions constants in src/backend/access/common/reloptions.c and a proper
function to get lock level by reloption. Thoughts?
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/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 32e19c5..0be658f 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -402,6 +402,7 @@ static void ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, LOCKMODE lockmo
static void ATExecSetRelOptions(Relation rel, List *defList,
AlterTableType operation,
LOCKMODE lockmode);
+static LOCKMODE GetRelOptionsLockLevel(List *defList);
static void ATExecEnableDisableTrigger(Relation rel, char *trigname,
char fires_when, bool skip_system, LOCKMODE lockmode);
static void ATExecEnableDisableRule(Relation rel, char *rulename,
@@ -2783,6 +2784,39 @@ AlterTableInternal(Oid relid, List *cmds, bool recurse)
}
/*
+ * GetRelOptionsLockLevel
+ *
+ * Return AccessShareLock if all reloptions is related to autovacuum
+ * else return AccessExclusiveLock.
+ *
+ */
+LOCKMODE
+GetRelOptionsLockLevel(List *defList)
+{
+ LOCKMODE lockmode;
+ ListCell *cell;
+
+ if (defList == NIL)
+ return NoLock;
+
+ foreach(cell, defList)
+ {
+ DefElem *def = (DefElem *) lfirst(cell);
+
+ /* relax lock for autovacuum reloptions */
+ if (pg_strncasecmp("autovacuum_", def->defname, 11) == 0)
+ lockmode = AccessShareLock;
+ else
+ {
+ lockmode = AccessExclusiveLock;
+ break;
+ }
+ }
+
+ return lockmode;
+}
+
+/*
* AlterTableGetLockLevel
*
* Sets the overall lock level required for the supplied list of subcommands.
@@ -3028,7 +3062,7 @@ AlterTableGetLockLevel(List *cmds)
* getTables() */
case AT_ResetRelOptions: /* Uses MVCC in getIndexes() and
* getTables() */
- cmd_lockmode = AccessExclusiveLock;
+ cmd_lockmode = GetRelOptionsLockLevel((List *) cmd->def);
break;
default: /* oops */
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
