Re: ALTER TABLE lock strength reduction patch is unsafe

On Fri, Jun 24, 2011 at 4:27 PM, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
> On Fri, Jun 24, 2011 at 9:00 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>> On Fri, Jun 24, 2011 at 3:46 PM, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
>>> Test case please. I don't understand the problem you're describing.
>>
>> S1: select * from foo;
>> S2: begin;
>> S2: alter table foo alter column a set storage plain;
>> S1: select * from foo;
>> <blocks>
>
> Er,,.yes, that what locks do. Where is the bug?

I didn't say it was a bug. I said that the additional locking you
added acted to remove the much of the benefit of reducing the lock
strength in the first place. If a brief exclusive lock (such as would
be taken by ALTER TABLE SET STORAGE in 9.0 and prior release) wasn't
acceptable, a brief exclusive lock on a different lock tag that blocks
most of the same operations isn't going to be any better. You might
still see some improvement in the cases where some complicated
operation like scanning or rewriting the table can be performed
without holding the relation definition lock, and then only get the
relation definition lock afterward. But for something like the above
example (or the ALTER TABLE SET (n_distinct) feature you mentioned in
your previous email) the benefit is going to be darned close to zero.

> This patch has locking, but its the most reduced form of locking that
> is available for a non invasive patch for 9.1

I don't like the extra lock manager traffic this adds to operations
that aren't even performing DDL, I'm not convinced that it is safe,
the additional locking negates a significant portion of the benefit of
the original patch, and Tom's made a fairly convincing argument that
even with this pg_dump will still become significantly less reliable
than heretofore even if we did apply it. In my view, what we need to
do is revert to using AccessExclusiveLock until some of these other
details are worked out. I wasn't entirely convinced that that was
necessary when Tom first posted this email, but having thought through
the issues and looked over your patch, it's now my position that that
is the best way forward. The fact that your proposed patch appears
not to be thinking very clearly about when locks need to be taken and
released only adds to my feeling that we are in for a bad time if we
go down this route.

In short: -1 from me.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company