On Wed, Apr 26, 2017 at 4:17 AM, Peter Eisentraut
<peter(dot)eisentraut(at)2ndquadrant(dot)com> wrote:
> On 4/25/17 00:26, Michael Paquier wrote:
>> So things are broken for sequences since commit 1753b1b0 (adding Peter
>> in CC) that has changed the way sequence metadata is handled. The
>> failure happens in CatalogTupleUpdate() which uses
>> simple_heap_update() that caller can only use if updates are
>> concurrent safe. But since 1753b1b0 that is not true as the sequence
>> is locked with AccessShareLock.
>
> I think you are confusing locking the sequence versus locking the
> pg_sequence catalog. The error is coming from CatalogTupleUpdate() on
> pg_sequence, which is locked using RowExclusiveLock, which is what we
> use for most DDL commands doing catalog changes.
Yes, and that's fine, taking a stronger lock on pg_sequence would be
disruptive for other sessions, including the ones updating pg_sequence
for different sequences. The point I am trying to make here is that
the code path updating pg_sequence should make sure that the
underlying object is properly locked first, so as the update is
concurrent-safe because this uses simple_heap_update that assumes that
the operation will be concurrent-safe. For example, take tablecmds.c,
we make sure that any relation ALTER TABLE works on gets a proper lock
with relation_open first, in what sequences would be different now
that they have their own catalog?
--
Michael