Re: Concurrent ALTER SEQUENCE RESTART Regression

From: Andres Freund <andres(at)anarazel(dot)de>
To: Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, pgsql-hackers(at)postgresql(dot)org
Cc: Noah Misch <noah(at)leadboat(dot)com>, Jason Petersen <jason(at)citusdata(dot)com>, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, PostgreSQL mailing lists <pgsql-bugs(at)postgresql(dot)org>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Concurrent ALTER SEQUENCE RESTART Regression
Date: 2017-05-07 23:43:34
Message-ID: 20170507234334.yimkp6aq2o43wtt2@alap3.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs pgsql-hackers

Hi,

Moving discussion to -hackers, this isn't really a bug, it's an
architectural issue with the new in-tree approach.

Short recap: With the patch applied in [1] ff, sequences partially
behave transactional (because pg_sequence is updated transactionally),
partially non-transctionally (because there's no locking enforcing it,
and it's been stated as undesirable to change that). This leads to
issues like described in [2]. For more context, read the whole
discussion.

On 2017-05-03 23:29:29 -0400, Peter Eisentraut wrote:
> I'm working on this and will report on Friday.

What's the plan here? I think the problem is that the code as is is
trying to marry two incompatible things: You're trying to make nextval()
not block, but have ALTER SEQUENCE be transactional. Given MAXVAL,
INCREMENT, etc. those two simply aren't compatible.

I think there's three basic ways to a resolution here:
1. Revert the entire patch for now. That's going to be very messy because
of the number of followup patches & features.
2. Keep the catalog, but only ever update the records using
heap_inplace_update. That'll make the transactional behaviour very
similar to before. It'd medium-term also allow to move the actual
sequence block into the pg_sequence catalog itself.
3. Keep the catalog, make ALTER properly transactional, blocking
concurrent nextval()s. This resolves the issue that nextval() can't
see the changed definition of the sequence.

I think this really needs design agreement from multiple people, because
any of these choices will have significant impact.

To me 3. seems like the best approach long-term, because both the
current and the <v10 behaviour certainly is confusing and inconsistent
(but in different ways). But it'll be quite noticeable to users.

- Andres

[1] http://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=1753b1b027035029c2a2a1649065762fafbf63f3
[2] http://archives.postgresql.org/message-id/20170427062304.gxh3rczhh6vblrwh%40alap3.anarazel.de

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message Michael Paquier 2017-05-08 02:27:20 Re: Concurrent ALTER SEQUENCE RESTART Regression
Previous Message zosrothko 2017-05-07 13:25:02 Re: BUG #14633: ecpg : nothing is generated when using option -v

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Langote 2017-05-08 00:47:52 Re: Declarative partitioning - another take
Previous Message Jeff Janes 2017-05-07 23:17:31 logical replication deranged sender