Re: foreign key locks, 2nd attempt - Mailing list pgsql-hackers

Excerpts from Simon Riggs's message of jue feb 23 06:18:57 -0300 2012:
>
> On Wed, Feb 22, 2012 at 5:00 PM, Noah Misch <noah@leadboat.com> wrote:
>
> >> 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.

Now this is an interesting turn of events.  I must thank you for your
extensive review effort in the current version of the patch, and also
thank you and credit you for the idea that initially kicked this patch
from the older, smaller, simpler version I wrote during the 9.1 timeline
(which you also reviewed exhaustively).  Without your and Simon's
brilliant ideas, this patch wouldn't exist at all.

I completely understand that you don't want to review this latest
version of the patch; it's a lot of effort and I wouldn't inflict it on
anybody who hasn't not volunteered.  However, it doesn't seem to me that
this is reason to boot the patch from the commitfest.  I think the thing
to do would be to remove yourself from the reviewers column and set it
back to "needs review", so that other reviewers can pick it up.

As for the late code churn, it mostly happened as a result of your
own feedback; I would have left most of it in the original state, but as
I went ahead it seemed much better to refactor things.  This is mostly
in heapam.c.  As for multixact.c, it also had a lot of churn, but that
was mostly to restore it to the state it has in the master branch,
dropping much of the code I had written to handle multixact truncation.
The new code there and in the vacuum code path (relminmxid and so on) is
a lot smaller than that other code was, and it's closely based on
relfrozenxid which is a known piece of technology.

> My view would be that with 90 files touched this is a very large
> patch, so that alone makes me wonder whether we should commit this
> patch, so I agree with Noah and compliment him on an excellent
> detailed review.

I note, however, that the bulk of the patch is in three files --
multixact.c, tqual.c, heapam.c, as is clearly illustrated in the diff
stats I posted.  The rest of them are touched mostly to follow their new
APIs (and of course to add tests and docs).

To summarize, of 94 files touched in total:
* 22 files are in src/test/isolation/ (new and updated tests and expected files)
* 19 files are in src/include/
* 10 files are in contrib/
* 39 files are in src/backend;    * in that subdir, there are 3097 insertions and 1006 deletions    * 3047 (83%) of
whichare in heapam.c multixact.c tqual.c    * one is a README 

> However, review of such a large patch should not be simply pass or
> fail. We should be looking back at the original problem and ask
> ourselves whether some subset of the patch could solve a useful subset
> of the problem. For me, that seems quite likely and this is very
> definitely an important patch.
>
> Even if we can't solve some part of the problem we can at least commit
> some useful parts of infrastructure to allow later work to happen more
> smoothly and quickly.
>
> So please let's not focus on the 100%, lets focus on 80/20.

Well, we have the patch I originally posted in the 9.1 timeframe.
That's a lot smaller and simpler.  However, that solves only part of the
blocking problem, and in particular it doesn't fix the initial deadlock
reports from Joel Jacobson at Glue Finance (now renamed Trustly, in case
you wonder about his change of email address) that started this effort
in the first place.  I don't think we can cut down to that and still
satisfy the users that requested this; and Glue was just the first one,
because after I started blogging about this, some more people started
asking for it.

I don't think there's any useful middlepoint between that one and the
current one, but maybe I'm wrong.

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