Getting rid of "tuple concurrently updated" elog()s with concurrent DDLs (at least ALTER TABLE)

[Michael Paquier]

Hi all,

Triggering "tuple concurrently updated" from heapam.c, which is an elog(),
is not that difficult for some DDL commands. For example with ALTER ROLE,
just create a role:
psql --no-psqlrc -c 'create role popo'

And then run that in two sessions and the error will show up, triggered
from CatalogTupleUpdate():
while true; do psql --no-psqlrc -c "alter role popo password 'foo'"; done

In this case, upgrading the lock taken on pg_authid from RowExclusiveLock
to ShareRowExclusiveLock prevents concurrent sessions to update each other,
which is what the patch attached does.

I think that we should get rid of all those errors, for many applications
doing concurrent DDLs getting this error is surprising, and could be thought
as a corrupted instance. I am just touching the tip of the iceberg here, as
I have the gut feeling that there are other objects where it is possible to
trigger the failure, and an analysis effort is going to be costly. So I'd
like to get first the temperature about such analysis.

So, opinions?
--
Michael

diff --git a/doc/src/sgml/mvcc.sgml b/doc/src/sgml/mvcc.sgml
index 24613e3c75..a93b31ae6d 100644
--- a/doc/src/sgml/mvcc.sgml
+++ b/doc/src/sgml/mvcc.sgml
@@ -971,6 +971,7 @@ ERROR:  could not serialize access due to read/write 
dependencies among transact
 
         <para>
          Acquired by <command>CREATE COLLATION</command>,
+         <command>ALTER ROLE</command>,
          <command>CREATE TRIGGER</command>, and many forms of
          <command>ALTER TABLE</command> (see <xref linkend="sql-altertable"/>).
         </para>
diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c
index f2941352d7..d2e300d949 100644
--- a/src/backend/commands/user.c
+++ b/src/backend/commands/user.c
@@ -671,9 +671,11 @@ AlterRole(AlterRoleStmt *stmt)
                bypassrls = intVal(dbypassRLS->arg);
 
        /*
-        * Scan the pg_authid relation to be certain the user exists.
+        * Scan the pg_authid relation to be certain the user exists. Take
+        * ShareRowExclusiveLock to prevent other sessions to update this
+        * tuple in parallel with another ALTER ROLE command.
         */
-       pg_authid_rel = heap_open(AuthIdRelationId, RowExclusiveLock);
+       pg_authid_rel = heap_open(AuthIdRelationId, ShareRowExclusiveLock);
        pg_authid_dsc = RelationGetDescr(pg_authid_rel);
 
        tuple = get_rolespec_tuple(stmt->role);

signature.asc
Description: PGP signature