foreign key locks

Hi,

This is v12 of the foreign key locks patch.

I gave a talk about this patch at PGCon:
http://www.pgcon.org/2012/schedule/events/483.en.html
There is an article there that explains this patch.

I described the original goal of this in the thread starting here:
http://archives.postgresql.org/message-id/1290721684-sup-3951@alvh.no-ip.org

The patch itself was first submitted here:
http://archives.postgresql.org/message-id/1320343602-sup-2290@alvh.no-ip.org

There were many problems that we had to shake off during the 9.2
development cycle; this new version covers most of them. There is a
github clone here: http://github.com/alvherre/postgres branch fklocks
If you see the "compare" view, changes in this submission that weren't
present in v11 start with commit 19dc85f1, here:
https://github.com/alvherre/postgres/compare/master...fklocks

I know of one significant issue left, possible cause of nasty bugs,
which is the way this patch interacts with EvalPlanQual. I mentioned
erroneously in my PGcon talk that the way we handle this is by shutting
off EPQ recursion -- this was wrong. What I did was shut off
heap_lock_tuple from following the update chain, letting it walk only in
the cases where it comes from high-level callers. Others such as EPQ,
which do their own update-chain walking, do not request locks to follow
update. I believe this is correct. This was changed in this commit:
https://github.com/alvherre/postgres/commit/e00e6827c55128ed737172a6dd35c581d437f969

There is also a matter of a performance regression we measured in stock
pgbench. I have profiled this to come from GetMultiXactMembers and
AFAICS the way to fix it is to improve multixact.c's cache of old
multis. Right now we ditch the cache at end of transaction, which was
already considered wrong in comments in the code but never fixed. I am
going to make the cache entries live until the oldest Xid in each multi
is below its visibility horizon (so RecentXmin for multis that only
contain locks, and something like FreezeLimit for multis that contain
updates).

I apologize for the size of this patch. I am not really sure that there
are pieces to split out for separate review.

--
Álvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>

On Thu, Jun 14, 2012 at 11:41 AM, Alvaro Herrera
<alvherre(at)alvh(dot)no-ip(dot)org> wrote:
> Hi,
>
> This is v12 of the foreign key locks patch.
>

Hi Álvaro,

Just noticed that this patch needs a rebase because of the refactoring
Tom did in ri_triggers.c

--
Jaime Casanova         www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación

Jaime Casanova <jaime(at)2ndquadrant(dot)com> writes:
> On Thu, Jun 14, 2012 at 11:41 AM, Alvaro Herrera
> <alvherre(at)alvh(dot)no-ip(dot)org> wrote:
>> This is v12 of the foreign key locks patch.

> Just noticed that this patch needs a rebase because of the refactoring
> Tom did in ri_triggers.c

Hold on a bit before you work on that code --- I've got one more bit of
hacking I want to try before walking away from it. I did some oprofile
work on Dean's example from
<CAEZATCWm8M00RA814o4DC2cD_aj44gQLb0tDdxMHA312qg7HCQ(at)mail(dot)gmail(dot)com>
and noticed that it looks like ri_FetchConstraintInfo is eating a
noticeable fraction of the runtime, which is happening because it is
called to deconstruct the relevant pg_constraint row for each tuple
we consider firing the trigger for (and then again, if we do fire the
trigger). I'm thinking it'd be worth maintaining a backend-local cache
for the deconstructed data, and am going to go try that.

regards, tom lane

Excerpts from Tom Lane's message of mié jun 20 12:54:24 -0400 2012:
> Jaime Casanova <jaime(at)2ndquadrant(dot)com> writes:
> > On Thu, Jun 14, 2012 at 11:41 AM, Alvaro Herrera
> > <alvherre(at)alvh(dot)no-ip(dot)org> wrote:
> >> This is v12 of the foreign key locks patch.
>
> > Just noticed that this patch needs a rebase because of the refactoring
> > Tom did in ri_triggers.c
>
> Hold on a bit before you work on that code --- I've got one more bit of
> hacking I want to try before walking away from it. I did some oprofile
> work on Dean's example from
> <CAEZATCWm8M00RA814o4DC2cD_aj44gQLb0tDdxMHA312qg7HCQ(at)mail(dot)gmail(dot)com>
> and noticed that it looks like ri_FetchConstraintInfo is eating a
> noticeable fraction of the runtime, which is happening because it is
> called to deconstruct the relevant pg_constraint row for each tuple
> we consider firing the trigger for (and then again, if we do fire the
> trigger). I'm thinking it'd be worth maintaining a backend-local cache
> for the deconstructed data, and am going to go try that.

I had merged already when I got your email, but merging that additional
change did not cause any conflicts. So here's the rebased version.

--
Álvaro Herrera <alvherre(at)commandprompt(dot)com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support