Re: foreign key locks

On Sunday, October 28, 2012 11:47:16 PM Simon Riggs wrote:
> On 27 October 2012 00:06, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
> > On Thursday, October 18, 2012 09:58:20 PM Alvaro Herrera wrote:
> >> Here is version 22 of this patch. This version contains fixes to issues
> >> reported by Andres, as well as a rebase to latest master.
> >
> > Ok, I now that pgconf.eu has ended I am starting to do a real review:
> >
> > * Is it ok to make FOR UPDATE somewhat weaker than before? In 9.2 and
> > earlier you could be sure that if you FOR UPDATE'ed a row you could
> > delete it. Unless I miss something now this will not block somebody else
> > acquiring a FOR KEY SHARE lock, so this guarantee is gone.
>
> Yes, that is exactly the point of this.

Yes, sure. The point is the introduction of a weaker lock level which can be
used by the ri triggers. I don't see any imperative that the semantics of the
old lock level need to be redefined. That just seems dangerous to me.

We need to take care to reduce the complications of upgrades not introduce
changes that require complex code reviews.

> > This seems likely to introduce subtle problems in user applications.
>
> Yes, it could. So we need some good docs on explaining this.
>
> Which applications use FOR UPDATE?

Any that want to protect themselves against deadlocks and/or visibility
problems due to READ COMMITTED. Thats quite a bunch.

> Any analysis of particular situations would be very helpful in doing
> the correct thing here.

Usual things include

* avoiding problems due to lock upgrades in combination with foreign keys (as
far as I can see some of those still persist).
* prevent rows being deleted by other transactions
* prepare for updating if computation of the new values take some time
* guarantee order of locking to make sure rows are DELETE/UPDATEed in the same
order (still no ORDER BY in UPDATE/DELETE)
...

> I think introducing FOR DELETE would be much clearer as an addition/
> synonym for FOR KEY UPDATE.

Hm. Not really convinced. For me that seems to just make the overall situation
even more complex.

> > I propose renaming FOR UPDATE => FOR NO KEY UPDATE, FOR KEY UPDATE => FOR
> > UPDATE or similar (or PREVENT KEY UPDATE?). That keeps the old meaning of
> > FOR UPDATE.
>
> Which is essentially unwinding the feature, to some extent.

Why? The overall features available are just the same? The only thing is that
existing semantics aren't changed.

> Does FOR UPDATE throw an error if the user later issues an UPDATE of
> the PK or a DELETE? That sequence of actions would cause lock
> escalation in the application, which could also lead to
> deadlock/contention.

Unless I miss something it precisely does *not* result in lock escalation with
the 9.2 semanticsbut it *would* with fklocks applied. Thats exactly my point.

> This sounds like we need a GUC or table-level default to control
> whether FOR UPDATE means FOR UPDATE or FOR DELETE

I don't like adding a new guc for something that should be solveable with some
creative naming. If a new user doesn't get a bit more concurrency due to
manually issued 9.2 FOR UPDATE implicitly being converted into a FOR NO KEY
UPDATE its not too bad. The code needs to be checked whether thats valid
anyway. The reverse is not true...

> More thought/input required on this point, it seems important.

Yep, more input welcome.

Greetings,

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