Re: Insufficient locking for ALTER DEFAULT PRIVILEGES - Mailing list pgsql-hackers

From Alvaro Herrera
Subject Re: Insufficient locking for ALTER DEFAULT PRIVILEGES
Date
Msg-id 20150620172716.GZ133018@postgresql.org
Whole thread Raw
In response to Insufficient locking for ALTER DEFAULT PRIVILEGES  (Vik Fearing <vik@2ndquadrant.fr>)
Responses Re: Insufficient locking for ALTER DEFAULT PRIVILEGES  (Alvaro Herrera <alvherre@2ndquadrant.com>)
Re: Insufficient locking for ALTER DEFAULT PRIVILEGES  (Amit Kapila <amit.kapila16@gmail.com>)
List pgsql-hackers
Vik Fearing wrote:

> Session 1:
> begin;
> alter default privileges in schema bug grant all on tables to postgres;
> 
> Session 2:
> alter default privileges in schema bug grant all on tables to postgres;
> <hangs>
> 
> Session 1:
> commit;
> 
> Session 2:
> ERROR:  tuple concurrently updated

So it turns out we don't have any locking here at all.  I don't believe
we have it for all object types, but in most cases it's not as obnoxious
as this one.  But at least for relations we have some nice coding in
RangeVarGetRelidExtended and RangeVarGetAndCheckCreationNamespace that
protect things.

I was thinking of adding some similar locking-and-looping logic in
StoreDefaultACL: grab the tuple from catalogs, LockDatabaseObject()
using the OID of the tuple so obtained; check the sinval counter like
RangeVarGetRelidExtended, if no change we're okay; if it changed, go
grab the OID once again, and if it changed, restart from the top; if
the OID did not change, then we're done.

This sounds complicated, but it's actually reasonably straightforward
and contained within a single routine.

But then, it doesn't handle the case that two transactions try to start
a row for the same combination at the same time.  One of them is going
to get the heap_insert() call to succeed and the other one is going to
get an ugly error message.

I wonder if I'm over-thinking this.  Other thoughts?

-- 
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services



pgsql-hackers by date:

Previous
From: Tomas Vondra
Date:
Subject: Re: pretty bad n_distinct estimate, causing HashAgg OOM on TPC-H
Next
From: Alvaro Herrera
Date:
Subject: Re: Insufficient locking for ALTER DEFAULT PRIVILEGES