Re: [HACKERS] Concurrent ALTER SEQUENCE RESTART Regression
On Mon, May 8, 2017 at 8:43 AM, Andres Freund wrote: > 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-transactionally (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. Thanks for summarizing. > 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. In this case it is enough to use ShareUpdateExclusiveLock on the sequence object, not pg_sequence. > 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. This will need an even stronger lock, AccessExclusiveLock to make this work properly. > 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 (but in different ways). But it'll be quite noticeable to users. Count me in for the 3. bucket. This loses the properly of having nextval() available to users when a parallel ALTER SEQUENCE is running, but consistency matters even more. I think that the final patch closing this thread should also have proper isolation tests. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Concurrent ALTER SEQUENCE RESTART Regression
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 http://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=1753b1b027035029c2a2a1649065762fafbf63f3 [2] http://archives.postgresql.org/message-id/20170427062304.gxh3rczhh6vblrwh%40alap3.anarazel.de -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Concurrent ALTER SEQUENCE RESTART Regression
On Fri, Apr 28, 2017 at 04:55:45PM +0900, Michael Paquier wrote:
> On Thu, Apr 27, 2017 at 4:10 PM, Andres Freund wrote:
> > On April 27, 2017 12:06:55 AM PDT, Michael Paquier
> > wrote:
> >>On Thu, Apr 27, 2017 at 3:23 PM, Andres Freund
> >>wrote:
> >>> More fun:
> >>>
> >>> A: CREATE SEQUENCE someseq;
> >>> A: BEGIN;
> >>> A: ALTER SEQUENCE someseq MAXVALUE 10;
> >>> B: SELECT nextval('someseq') FROM generate_series(1, 1000);
> >>>
> >>> => ignores maxvalue
> >>
> >>Well, for this one that's because the catalog change is
> >>transactional...
> >
> > Or because the locking model is borked.
>
> The operation actually relies heavily on the fact that the exclusive
> lock on the buffer of pg_sequence is hold until the end of the catalog
> update. And using heap_inplace_update() seems mandatory to me as the
> metadata update should be non-transactional, giving the attached. I
> have added some isolation tests. Thoughts? The attached makes HEAD map
> with the pre-9.6 behavior.
[Action required within three days. This is a generic notification.]
The above-described topic is currently a PostgreSQL 10 open item. Peter,
since you committed the patch believed to have created it, you own this open
item. If some other commit is more relevant or if this does not belong as a
v10 open item, please let us know. Otherwise, please observe the policy on
open item ownership[1] and send a status update within three calendar days of
this message. Include a date for your subsequent status update. Testers may
discover new open items at any time, and I want to plan to get them all fixed
well in advance of shipping v10. Consequently, I will appreciate your efforts
toward speedy resolution. Thanks.
[1]
https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
