Re: ALTER TABLE lock strength reduction patch is unsafe

On 3 March 2014 18:57, Noah Misch <noah@leadboat.com> wrote:
> On Mon, Mar 03, 2014 at 10:19:55AM -0500, Robert Haas wrote:
>> On Thu, Feb 27, 2014 at 3:12 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
>> > Removing SELECT privilege while running a SELECT would be a different
>> > matter.  This is all a matter of definition; we can make up any rules
>> > we like. Doing so is IMHO a separate patch and not something to hold
>> > up the main patch.
>>
>> So I think this is an interesting point.  There are various things
>> that could go wrong as a result of using the wrong lock level.  Worst
>> would be that the server crashes or corrupts data.  A little less bad
>> would be that sessions error out with inexplicable error conditions,
>> as in SnapshotNow days.  Alternatively, we could just have arguably
>> wrong behavior, like basing query results on the old version of the
>> table's metadata even after it's been changed.
>
> I would order the concerns like this:
>
> 1. Data corruption
> 2. Transient, clearly-wrong answers without an error
> 3. Server crash
> 4. Catalog logical inconsistency
> 5. Inexplicable, transient errors
> 6. Valid behavior capable of surprising more than zero upgraders

I like your model for risk assessment. How can we apply it in detail
in a way that helps us decide? Or do we just go on gut feel?

My experience with mentioning such topics is that without structure it
results in an assessment of "unacceptable risk" just simply because
somebody has mentioned some scary words.


>> I don't really care about that second category of behavior.  If
>> somebody changes some property of a table and existing sessions
>> continue to use the old value until eoxact, well, we can argue about
>> that, but at least until we have concrete reports of really
>> undesirable behavior, I don't think it's the primary issue.  What I'm
>> really concerned about is whether there are other things like the
>> SnapshotNow issues that can cause stuff to halt and catch fire.  I
>> don't know whether there are or are not, but that's my concern.
>
> Since we can't know whether something qualifies as (2) or (6) without
> analyzing it, I don't find waiting for user complaints to be a good strategy
> here.  An ownership change not immediately affecting ACL checks does fall
> under (6), for me.  (However, changing ownership without AccessExclusiveLock
> might also create hazards in category (4) for concurrent DDL that performs
> owner checks.)

err, guys, you do realise that changing ownership is staying at
AccessExclusiveLock in this patch?
(and I haven't ever suggested lowering that).

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