Re: foreign key locks, 2nd attempt

On Mon, Feb 13, 2012 at 07:16:58PM -0300, Alvaro Herrera wrote:
> Okay, so this patch fixes the truncation and wraparound issues through a
> mechanism much like pg_clog's: it keeps track of the oldest possibly
> existing multis on each and every table, and then during tuple freezing
> those are removed. I also took the liberty to make the code remove
> multis altogether (i.e. resetting the IS_MULTI hint bit) when only the
> update remains and lockers are all gone.
>
> I also cleaned up the code in heapam so that there's a couple of tables
> mapping MultiXactStatus to LockTupleMode and back, and to heavyweight
> lock modes (the older patches used functions to do this, which was
> pretty ugly). I had to add a little helper function to lock.c to make
> this work. I made a rather large bunch of other minor changes to close
> minor bugs here and there.
>
> Docs have been added, as have new tests for the isolation harness, which
> I've ensured pass in both read committed and serializable modes; WAL
> logging for locking updated versions of a tuple, when an old one is
> locked due to an old snapshot, was also added; there's plenty of room
> for growth in the MultiXact flag bits; the bit that made tables with no
> keys lock the entire row all the time was removed; multiple places in
> code comments were cleaned up that referred to this feature as "FOR KEY
> LOCK" and ensured that it also mentions FOR KEY UPDATE; the pg_rowlocks,
> pageinspect, pg_controldata, pg_resetxlog utilities have been updated.

All of the above sounds great. I especially like the growing test coverage.

> All in all, I think this is in pretty much final shape. Only pg_upgrade
> bits are still missing. If sharp eyes could give this a critical look
> and knuckle-cracking testers could give it a spin, that would be
> helpful.

Lack of pg_upgrade support leaves this version incomplete, because that
omission would constitute a blocker for beta 2. This version changes as much
code compared to the version I reviewed at the beginning of the CommitFest as
that version changed overall. In that light, it's time to close the books on
this patch for the purpose of this CommitFest; I'm marking it Returned with
Feedback. Thanks for your efforts thus far.

On Mon, Jan 30, 2012 at 06:48:47PM -0500, Noah Misch wrote:
> On Tue, Jan 24, 2012 at 03:47:16PM -0300, Alvaro Herrera wrote:
> > * Columns that are part of the key
> > Noah thinks the set of columns should only consider those actually referenced
> > by keys, not those that *could* be referenced.
>
> Well, do you disagree? To me it's low-hanging fruit, because it isolates the
> UPDATE-time overhead of this patch to FK-referenced tables rather than all
> tables having a PK or PK-like index (often just "all tables").

You have not answered my question above.

nm