Re: RI_FKey_check: foreign key constraint blocks parallel

I've got a question about the foreign key constraint behavior.

It looks to me that inserts within transactions into a child table, which have the same FK value back to the parent will block until the first txn will commit or rollback. (see example below)

This seems to be based on the fact that the RI_FKey_check function will lock the parent row for update, so any other child row referring the same row will be locked out.

I've added a debug stmt into the RI_FKey_check function to see the query it does:
NOTICE: RI_FKey_check: PLAN2: SELECT 1 FROM ONLY "public"."parent" x WHERE "id" = $1 FOR UPDATE OF x

I think I basically understand, why this is done. To make sure that the parent row can't be deleted before the child row is committed and there would have an orphan reference.

But, if a lot of inserts happens into the child table and there is a mix of short and long running transactions, the likelihood of blocking is very high, even the inserts are independent and everything is ok (prim. key etc.). This is even more extreme, the smaller parent table is.

FYI, I've tried the same with Oracle and there is no such problem. The insert in the second session will come back immediately without blocking, though it will still maintain the integrity from other txns.

I wonder if there is a lower level way to maintain the locking and having the same behavior as oracle.
So, instead of using a "SELECT ... FOR UPDATE", using some pg function to lock a row with a different mode?

Overall, I find this restriction pretty bad and renders the use of foreign key constraints almost useless from the performance point of view as that leads to real serialization of transaction, even they don't have any overlaps.

in session1:
============
drop table child;
drop table parent;

create table parent (id integer not null);
ALTER TABLE parent ADD CONSTRAINT parent_PK PRIMARY KEY(ID);

create table child (id integer not null, parent_id integer not null);
ALTER TABLE child ADD CONSTRAINT child_PK PRIMARY KEY(ID);
ALTER TABLE child ADD CONSTRAINT child_parent_id FOREIGN KEY (parent_id) REFERENCES parent (ID);

insert into parent values (1);
insert into parent values (2);

begin;
insert into child values (1,1);
<this will be ok>

in session2 after the last insert in session1:
==============================================
begin;
insert into child values (2,1);
<this will block now until the session1 does commit or rollback>

--
Best regards,
Peter Schindler

On Wed, 13 Nov 2002, Peter Schindler wrote:

> But, if a lot of inserts happens into the child table and there is a
> mix of short and long running transactions, the likelihood of blocking
> is very high, even the inserts are independent and everything is ok
> (prim. key etc.). This is even more extreme, the smaller parent table
> is.
>
> FYI, I've tried the same with Oracle and there is no such problem. The
> insert in the second session will come back immediately without
> blocking, though it will still maintain the integrity from other txns.
>
> I wonder if there is a lower level way to maintain the locking and
> having the same behavior as oracle. So, instead of using a "SELECT ...
> FOR UPDATE", using some pg function to lock a row with a different
> mode?

I've been working on something of the sort. I've got a test patch
(against about 7.3b2) that I'm trying to validate which cases it does and
does not work for. I'm still looking for more volunteers if you've got a
dev system you're willing to use. :)

Right now, I know that it has a hole that lets through invalid data in one
case that it got while trying to fix a deadlock case. Hopefully in the
next week or so I'll have figured out a way around it.

Stephan Szabo wrote:
> I've been working on something of the sort. I've got a test patch
> (against about 7.3b2) that I'm trying to validate which cases it does and
> does not work for. I'm still looking for more volunteers if you've got a
> dev system you're willing to use. :)
I'd willing to do some testing. Though, I can't promise too much time as we are
just in the middle of final. qualification for our release. Also we are still
using 7.2.1 and didn't port to 7.3 yet. But, I could do some systematic manual
testing with psql if you want. Could you sent me the patch please.

BTW, I forgot to mention this in my orig. mail, even it probably obvious to you,
this behavior is there for several (if not all) pg releases. I've tested it with
7.2.1, 7.3b2 and 7.3b5.

> Right now, I know that it has a hole that lets through invalid data in one
> case that it got while trying to fix a deadlock case. Hopefully in the
> next week or so I'll have figured out a way around it.
After our and the pg7.3 release is out we'll port there and I really would like
to get rid of this restriction with that release than. So it would be wonderful
if that still goes into the final of 7.3.

Rgs,
Peter

> After our and the pg7.3 release is out we'll port there and I
> really would like
> to get rid of this restriction with that release than. So it
> would be wonderful
> if that still goes into the final of 7.3.

I'm not a core developer, but I'll tell you right now that there's pretty
much zero chance of it being in 7.3 - it's about to go to release candidate.
Since it changes pretty important functionality, it will be left for 7.4.

Chris

On Wed, 13 Nov 2002 14:22:51 -0800 (PST), Stephan Szabo
<sszabo(at)megazone23(dot)bigpanda(dot)com> wrote:
>Right now, I know that it has a hole that lets through invalid data

Stephan, your patch has been posted to -general (Subject: Re:
[GENERAL] Help..Help...). Is this version still valid?

> void
> heap_mark4fk_lock_acquire(Relation relation, HeapTuple tuple) {
> [...]
> /* try to find the list for the table in question */
This part of the patch works, if the list
(a) is initially empty or
(b) already contains relid or
(c) starts with a table > relid.

> while (ptr!=NULL) {
> if (relid>ptr->table) {
> ptr=ptr->next;
> oldptr=ptr;
// AFAICT above two lines should be swapped ...
> }
> else
> break;
> }

... otherwise
(d) if the new relid is to be inserted between two existing entries,
we get two items pointing to each other
(e) if the new relid is > the last table in the list, we lose the
whole list.

Servus
Manfred

On Fri, 15 Nov 2002, Manfred Koizar wrote:

> On Wed, 13 Nov 2002 14:22:51 -0800 (PST), Stephan Szabo
> <sszabo(at)megazone23(dot)bigpanda(dot)com> wrote:
> >Right now, I know that it has a hole that lets through invalid data
>
> Stephan, your patch has been posted to -general (Subject: Re:
> [GENERAL] Help..Help...). Is this version still valid?

I have a newer version of it on my machine, but I was still sending out
that version of the patch. :( Thanks for letting me know before even more
people got a version that was broken. :)

For anyone working with the patch, you need to fix the lines below as
noted by Manfred. This is mostly unrelated to the hole mentioned in the
quoted message above (it's a bug that with the bug you actually partially
fill the hole but instead deadlock). I wonder if there were any other
stupdities in there.

> > void
> > heap_mark4fk_lock_acquire(Relation relation, HeapTuple tuple) {
> > [...]
> > /* try to find the list for the table in question */
> This part of the patch works, if the list
> (a) is initially empty or
> (b) already contains relid or
> (c) starts with a table > relid.
>
> > while (ptr!=NULL) {
> > if (relid>ptr->table) {
> > ptr=ptr->next;
> > oldptr=ptr;
> // AFAICT above two lines should be swapped ...
> > }
> > else
> > break;
> > }
>
> ... otherwise
> (d) if the new relid is to be inserted between two existing entries,
> we get two items pointing to each other
> (e) if the new relid is > the last table in the list, we lose the
> whole list.

(Just noticed that this was sent to hackers, but -general would probably
get to more of the people who might want to see it)

On Fri, 15 Nov 2002, Stephan Szabo wrote:

> On Fri, 15 Nov 2002, Manfred Koizar wrote:
>
> > On Wed, 13 Nov 2002 14:22:51 -0800 (PST), Stephan Szabo
> > <sszabo(at)megazone23(dot)bigpanda(dot)com> wrote:
> > >Right now, I know that it has a hole that lets through invalid data
> >
> > Stephan, your patch has been posted to -general (Subject: Re:
> > [GENERAL] Help..Help...). Is this version still valid?
>
> I have a newer version of it on my machine, but I was still sending out
> that version of the patch. :( Thanks for letting me know before even more
> people got a version that was broken. :)
>
> For anyone working with the patch, you need to fix the lines below as
> noted by Manfred. This is mostly unrelated to the hole mentioned in the
> quoted message above (it's a bug that with the bug you actually partially
> fill the hole but instead deadlock). I wonder if there were any other
> stupdities in there.
>
> > > void
> > > heap_mark4fk_lock_acquire(Relation relation, HeapTuple tuple) {
> > > [...]
> > > /* try to find the list for the table in question */
> > This part of the patch works, if the list
> > (a) is initially empty or
> > (b) already contains relid or
> > (c) starts with a table > relid.
> >
> > > while (ptr!=NULL) {
> > > if (relid>ptr->table) {
> > > ptr=ptr->next;
> > > oldptr=ptr;
> > // AFAICT above two lines should be swapped ...
> > > }
> > > else
> > > break;
> > > }
> >
> > ... otherwise
> > (d) if the new relid is to be inserted between two existing entries,
> > we get two items pointing to each other
> > (e) if the new relid is > the last table in the list, we lose the
> > whole list.