Re: ALTER TABLE lock strength reduction patch is unsafe - Mailing list pgsql-hackers

From Andres Freund
Subject Re: ALTER TABLE lock strength reduction patch is unsafe
Date
Msg-id 20140226152540.GZ6718@awork2.anarazel.de
Whole thread Raw
In response to Re: ALTER TABLE lock strength reduction patch is unsafe  (Simon Riggs <simon@2ndQuadrant.com>)
Responses Re: ALTER TABLE lock strength reduction patch is unsafe
Re: ALTER TABLE lock strength reduction patch is unsafe
List pgsql-hackers
On 2014-02-26 15:15:00 +0000, Simon Riggs wrote:
> On 26 February 2014 13:38, Andres Freund <andres@2ndquadrant.com> wrote:
> > Hi,
> >
> > On 2014-02-26 07:32:45 +0000, Simon Riggs wrote:
> >> > * This definitely should include isolationtester tests actually
> >> >   performing concurrent ALTER TABLEs. All that's currently there is
> >> >   tests that the locklevel isn't too high, but not that it actually works.
> >>
> >> There is no concurrent behaviour here, hence no code that would be
> >> exercised by concurrent tests.
> >
> > Huh? There's most definitely new concurrent behaviour. Previously no
> > other backends could have a relation open (and locked) while it got
> > altered (which then sends out relcache invalidations). That's something
> > that should be tested.
> 
> It has been. High volume concurrent testing has been performed, per
> Tom's original discussion upthread, but that's not part of the test
> suite.

Yea, that's not what I am looking for.

> For other tests I have no guide as to how to write a set of automated
> regression tests. Anything could cause a failure, so I'd need to write
> an infinite set of tests to prove there is no bug *somewhere*. How
> many tests are required? 0, 1, 3, 30?

I think some isolationtester tests for the most important changes in
lock levels are appropriate. Say, create a PRIMARY KEY, DROP INHERIT,
...  while a query is in progress in a nother session.

> >> > * Why does ChangeOwner need AEL?
> >>
> >> Ownership affects privileges, which includes SELECTs, hence AEL.
> >
> > So?
> 
> That reply could be added to any post. Please explain your concern.

I don't understand why that means it needs an AEL. After all,
e.g. changes in table inheritance do *not* require an AEL. I think it's
perfectly ok to not go for the minimally required locklevel for all
subcommands, but then it should be commented as such and not with
"change visible to SELECT" where other operations that do so as well
require lower locklevels.

Greetings,

Andres Freund

-- Andres Freund                       http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services



pgsql-hackers by date:

Previous
From: Stephen Frost
Date:
Subject: Re: Custom Scan APIs (Re: Custom Plan node)
Next
From: Andrew Dunstan
Date:
Subject: Re: jsonb and nested hstore