Re: [HACKERS] [BUGS] Concurrent ALTER SEQUENCE RESTART Regression

On 2017-06-01 19:08:33 +0200, Petr Jelinek wrote: > On 01/06/17 16:51, Robert Haas wrote: > > On Wed, May 31, 2017 at 8:07 PM, Andres Freund wrote: > >> Here's a patch doing what I suggested above. The second patch addresses > >> an independent oversight where the post alter

Re: [HACKERS] [BUGS] Concurrent ALTER SEQUENCE RESTART Regression

On 01/06/17 16:51, Robert Haas wrote: > On Wed, May 31, 2017 at 8:07 PM, Andres Freund wrote: >> Here's a patch doing what I suggested above. The second patch addresses >> an independent oversight where the post alter hook was invoked before >> doing the catalog update. > >

Re: [HACKERS] [BUGS] Concurrent ALTER SEQUENCE RESTART Regression

On Wed, May 31, 2017 at 8:07 PM, Andres Freund wrote: > Here's a patch doing what I suggested above. The second patch addresses > an independent oversight where the post alter hook was invoked before > doing the catalog update. 0002 is a slam-dunk. I don't object to 0001

Re: [HACKERS] [BUGS] Concurrent ALTER SEQUENCE RESTART Regression

On 2017-05-31 14:24:38 -0700, Andres Freund wrote: > On 2017-05-24 10:52:37 -0400, Robert Haas wrote: > > On Wed, May 24, 2017 at 10:32 AM, Andres Freund wrote: > > > Well, but then we should just remove minval/maxval if we can't rely on > > > it. > > > > That seems like a

Re: [HACKERS] [BUGS] Concurrent ALTER SEQUENCE RESTART Regression

On 2017-05-24 10:52:37 -0400, Robert Haas wrote: > On Wed, May 24, 2017 at 10:32 AM, Andres Freund wrote: > > Well, but then we should just remove minval/maxval if we can't rely on > > it. > > That seems like a drastic overreaction to me. Well, either they work, or they

Re: [HACKERS] [BUGS] Concurrent ALTER SEQUENCE RESTART Regression

On Wed, May 24, 2017 at 10:32 AM, Andres Freund wrote: > Well, but then we should just remove minval/maxval if we can't rely on > it. That seems like a drastic overreaction to me. > I wonder if that's not actually very little new code, and I think we > might end up

Re: [HACKERS] [BUGS] Concurrent ALTER SEQUENCE RESTART Regression

On 2017-05-24 10:24:19 -0400, Robert Haas wrote: > On Wed, May 24, 2017 at 9:04 AM, Andres Freund wrote: > > At the very least we'll have to error out. That's still not nice usability > > wise, but it sure beats returning flat out wrong values. > > I'm not sure. That seems

Re: [HACKERS] [BUGS] Concurrent ALTER SEQUENCE RESTART Regression

On Wed, May 24, 2017 at 9:04 AM, Andres Freund wrote: > At the very least we'll have to error out. That's still not nice usability > wise, but it sure beats returning flat out wrong values. I'm not sure. That seems like it might often be worse. Now you need manual

Re: [HACKERS] [BUGS] Concurrent ALTER SEQUENCE RESTART Regression

On May 24, 2017 6:57:08 AM EDT, Robert Haas wrote: >On Tue, May 23, 2017 at 11:25 PM, Andres Freund >wrote: >> On 2017-05-23 22:47:07 -0400, Robert Haas wrote: >>> On Mon, May 22, 2017 at 11:42 AM, Andres Freund >wrote: >>> >

Re: [HACKERS] [BUGS] Concurrent ALTER SEQUENCE RESTART Regression

On Tue, May 23, 2017 at 11:25 PM, Andres Freund wrote: > On 2017-05-23 22:47:07 -0400, Robert Haas wrote: >> On Mon, May 22, 2017 at 11:42 AM, Andres Freund wrote: >> > Ooops. >> > >> > Two issues: Firstly, we get a value smaller than seqmin, obviously >>

Re: [HACKERS] [BUGS] Concurrent ALTER SEQUENCE RESTART Regression

On 2017-05-23 22:47:07 -0400, Robert Haas wrote: > On Mon, May 22, 2017 at 11:42 AM, Andres Freund wrote: > > Ooops. > > > > Two issues: Firstly, we get a value smaller than seqmin, obviously > > that's not ok. But even if we'd error out, it'd imo still not be ok, > > because

Re: [HACKERS] [BUGS] Concurrent ALTER SEQUENCE RESTART Regression

On Mon, May 22, 2017 at 11:42 AM, Andres Freund wrote: > Ooops. > > Two issues: Firstly, we get a value smaller than seqmin, obviously > that's not ok. But even if we'd error out, it'd imo still not be ok, > because we have a command that behaves partially transactionally >

Re: [HACKERS] [BUGS] Concurrent ALTER SEQUENCE RESTART Regression

On 2017-05-19 08:31:15 -0400, Robert Haas wrote: > On Thu, May 18, 2017 at 4:54 PM, Andres Freund wrote: > > There's still weird behaviour, unfortunately. If you do an ALTER > > SEQUENCE changing minval/maxval w/ restart in a transaction, and abort, > > you'll a) quite

Re: [HACKERS] [BUGS] Concurrent ALTER SEQUENCE RESTART Regression

On Thu, May 18, 2017 at 4:54 PM, Andres Freund wrote: > There's still weird behaviour, unfortunately. If you do an ALTER > SEQUENCE changing minval/maxval w/ restart in a transaction, and abort, > you'll a) quite possibly not be able to use the sequence anymore, > because it

Re: [HACKERS] [BUGS] Concurrent ALTER SEQUENCE RESTART Regression

On 2017-05-15 10:34:02 -0400, Peter Eisentraut wrote: > On 5/10/17 09:12, Michael Paquier wrote: > > Looking at 0001 and 0002... So you are correctly blocking nextval() > > when ALTER SEQUENCE holds a lock on the sequence object. And > > concurrent calls of nextval() don't conflict. As far as I

Re: [HACKERS] [BUGS] Concurrent ALTER SEQUENCE RESTART Regression

On 5/10/17 09:12, Michael Paquier wrote: > Looking at 0001 and 0002... So you are correctly blocking nextval() > when ALTER SEQUENCE holds a lock on the sequence object. And > concurrent calls of nextval() don't conflict. As far as I can see this > matches the implementation of 3. > > Here are

Re: [HACKERS] [BUGS] Concurrent ALTER SEQUENCE RESTART Regression

On Mon, May 08, 2017 at 12:28:38PM -0400, Peter Eisentraut wrote: > On 5/8/17 02:58, Noah Misch wrote: > > IMMEDIATE ATTENTION REQUIRED. This PostgreSQL 10 open item is long past due > > for your status update. Please reacquaint yourself with the policy on open > > item ownership[1] and then

Re: [HACKERS] [BUGS] Concurrent ALTER SEQUENCE RESTART Regression

On 5/11/17 23:59, Tom Lane wrote: > Right, but the existing code is *designed* to hold the lock till end of > top-level transaction, regardless of what happens in any subtransaction. > My understanding of your complaint is that you do not think that's OK > for any lock stronger than

Re: [HACKERS] [BUGS] Concurrent ALTER SEQUENCE RESTART Regression

On 5/11/17 17:28, Andres Freund wrote: > Isn't that pretty much the point? The whole open_share_lock() > optimization looks like it really only can make a difference with > subtransactions? Yeah that confused me too. That keep-the-lock-for-the-whole-transaction logic was introduced in

Re: [HACKERS] [BUGS] Concurrent ALTER SEQUENCE RESTART Regression

On 5/11/17 16:34, Andres Freund wrote: >>> This'd probably need to be removed, as we'd otherwise would get very >>> weird semantics around aborted subxacts. >> Can you explain in more detail what you mean by this? > Well, right now we don't do proper lock-tracking for sequences, always > assigning

Re: [HACKERS] [BUGS] Concurrent ALTER SEQUENCE RESTART Regression

Andres Freund writes: > On 2017-05-11 17:21:18 -0400, Tom Lane wrote: >> I doubt my machine is 6X faster than yours, >> so this indicates that the subtransaction overhead is pretty real. > Isn't that pretty much the point? The whole open_share_lock() > optimization looks

Re: [HACKERS] [BUGS] Concurrent ALTER SEQUENCE RESTART Regression

On 2017-05-11 17:21:18 -0400, Tom Lane wrote: > Peter Eisentraut writes: > > I ran this script > > > CREATE SEQUENCE seq1; > > > DO LANGUAGE plpythonu $$ > > plan = plpy.prepare("SELECT nextval('seq1')") > > for i in range(0, 1000): > >

Re: [HACKERS] [BUGS] Concurrent ALTER SEQUENCE RESTART Regression

Andres Freund writes: > On 2017-05-11 16:27:48 -0400, Peter Eisentraut wrote: >> (So without contention fast-path locking beats the extra dance that >> open_share_lock() does.) > That's kind of surprising, I really wouldn't have thought it'd be faster > without. I guess it's

Re: [HACKERS] [BUGS] Concurrent ALTER SEQUENCE RESTART Regression

Peter Eisentraut writes: > I ran this script > CREATE SEQUENCE seq1; > DO LANGUAGE plpythonu $$ > plan = plpy.prepare("SELECT nextval('seq1')") > for i in range(0, 1000): > plpy.execute(plan) > $$; > and timed the "DO". It occurred to me that

Re: [HACKERS] [BUGS] Concurrent ALTER SEQUENCE RESTART Regression

Hi, On 2017-05-11 16:27:48 -0400, Peter Eisentraut wrote: > On 5/10/17 12:24, Andres Freund wrote: > > Upthread I theorized whether > > that's actually still meaningful given fastpath locking and such, but I > > guess we'll have to evaluate that. > > I did some testing. That's with the

Re: [HACKERS] [BUGS] Concurrent ALTER SEQUENCE RESTART Regression

Peter Eisentraut writes: > On 5/10/17 12:24, Andres Freund wrote: >> Upthread I theorized whether >> that's actually still meaningful given fastpath locking and such, but I >> guess we'll have to evaluate that. > [ with or without contention, fast-path locking

Re: [HACKERS] [BUGS] Concurrent ALTER SEQUENCE RESTART Regression

On 2017-05-11 11:35:22 -0400, Peter Eisentraut wrote: > On 5/10/17 12:24, Andres Freund wrote: > > The issue isn't the strength, but that we currently have this weird > > hackery around open_share_lock(): > > /* > > * Open the sequence and acquire AccessShareLock if needed > > * > > * If we

Re: [HACKERS] [BUGS] Concurrent ALTER SEQUENCE RESTART Regression

On 5/10/17 12:24, Andres Freund wrote: > Upthread I theorized whether > that's actually still meaningful given fastpath locking and such, but I > guess we'll have to evaluate that. I did some testing. I ran this script CREATE SEQUENCE seq1; DO LANGUAGE plpythonu $$ plan = plpy.prepare("SELECT

Re: [HACKERS] [BUGS] Concurrent ALTER SEQUENCE RESTART Regression

On 5/10/17 12:24, Andres Freund wrote: > The issue isn't the strength, but that we currently have this weird > hackery around open_share_lock(): > /* > * Open the sequence and acquire AccessShareLock if needed > * > * If we haven't touched the sequence already in this transaction, > * we need

Re: [HACKERS] [BUGS] Concurrent ALTER SEQUENCE RESTART Regression

Andres Freund writes: > On 2017-05-10 10:29:02 -0400, Tom Lane wrote: >> As long as it doesn't block, the change in lock strength doesn't actually >> make any speed difference does it? > The issue isn't the strength, but that we currently have this weird > hackery around

Re: [HACKERS] [BUGS] Concurrent ALTER SEQUENCE RESTART Regression

On 2017-05-10 10:29:02 -0400, Tom Lane wrote: > Petr Jelinek writes: > > On 10/05/17 07:09, Peter Eisentraut wrote: > >> I think the correct fix is to have nextval() and ALTER SEQUENCE use > >> sensible lock levels so that they block each other. Since > >> nextval()

Re: [HACKERS] [BUGS] Concurrent ALTER SEQUENCE RESTART Regression

Petr Jelinek writes: > On 10/05/17 07:09, Peter Eisentraut wrote: >> I think the correct fix is to have nextval() and ALTER SEQUENCE use >> sensible lock levels so that they block each other. Since >> nextval() currently uses AccessShareLock, the suggestion was for

Re: [HACKERS] [BUGS] Concurrent ALTER SEQUENCE RESTART Regression

On 10/05/17 07:09, Peter Eisentraut wrote: > On 5/7/17 19:43, Andres Freund wrote: >> 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 was the

Re: [HACKERS] [BUGS] Concurrent ALTER SEQUENCE RESTART Regression

On Wed, May 10, 2017 at 2:09 PM, Peter Eisentraut wrote: > I think I have more clarity about the different, overlapping issues: > > 1. Concurrent ALTER SEQUENCE causes "tuple concurrently updated" >error, caused by updates to pg_sequence catalog. This can be

Re: [HACKERS] [BUGS] Concurrent ALTER SEQUENCE RESTART Regression

On 5/7/17 19:43, Andres Freund wrote: > 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 was the intended choice. I think I have more clarity about the

Re: [HACKERS] [BUGS] Concurrent ALTER SEQUENCE RESTART Regression

On 2017-05-08 12:28:38 -0400, Peter Eisentraut wrote: > On 5/8/17 02:58, Noah Misch wrote: > > IMMEDIATE ATTENTION REQUIRED. This PostgreSQL 10 open item is long past due > > for your status update. Please reacquaint yourself with the policy on open > > item ownership[1] and then reply

Re: [HACKERS] [BUGS] Concurrent ALTER SEQUENCE RESTART Regression

On 5/8/17 02:58, Noah Misch wrote: > IMMEDIATE ATTENTION REQUIRED. This PostgreSQL 10 open item is long past due > for your status update. Please reacquaint yourself with the policy on open > item ownership[1] and then reply immediately. If I do not hear from you by > 2017-05-09 07:00 UTC, I

Re: [HACKERS] [BUGS] Concurrent ALTER SEQUENCE RESTART Regression

On Sat, May 06, 2017 at 07:02:46PM +, Noah Misch wrote: > On Wed, May 03, 2017 at 11:29:29PM -0400, Peter Eisentraut wrote: > > On 4/30/17 04:05, Noah Misch wrote: > > > The above-described topic is currently a PostgreSQL 10 open item. Peter, > > > since you committed the patch believed to

Re: [HACKERS] [BUGS] Concurrent ALTER SEQUENCE RESTART Regression

On Wed, May 03, 2017 at 11:29:29PM -0400, Peter Eisentraut wrote: > On 4/30/17 04:05, Noah Misch wrote: > > 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

Re: [HACKERS] [BUGS] Concurrent ALTER SEQUENCE RESTART Regression

On 4/30/17 04:05, Noah Misch wrote: > 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