Re: foreign key locks, 2nd attempt

On Mon, Mar 12, 2012 at 1:50 PM, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
> On Mon, Mar 12, 2012 at 5:28 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>> In other words, we'd entirely avoid needing to make mxacts crash-safe,
>> and we'd avoid most of the extra SLRU lookups that the current
>> implementation requires; they'd only be needed when (and for as long
>> as) the locked tuple was not yet all-visible.
>
> The current implementation only requires additional lookups in the
> update/check case, which is the case that doesn't do anything other
> than block right now. Since we're replacing lock contention with
> physical access contention even the worst case situation is still
> better than what we have now. Please feel free to point out worst case
> situations and show that isn't true.

I think I already have:

http://archives.postgresql.org/pgsql-hackers/2012-02/msg01258.php

The case I'm worried about is where we are allocating mxids quickly,
and we end up having to wait for fsyncs on mxact segments. That might
be very slow, but you could argue that it could *possibly* be still
worthwhile if it avoids blocking. That doesn't strike me as a
slam-dunk, though, because we've already seen and fixed cases where
too many fsyncs causes the performance of the entire system to go down
the tubes (cf. commit 7f242d880b5b5d9642675517466d31373961cf98). But
it's really bad if there are no updates on the parent table - then,
whatever extra overhead there is will be all for naught, since the
more fine-grained locking doesn't help anyway.

> I've also pointed out how to avoid overhead of making mxacts crash
> safe when the new facilities are not in use, so I don't see problems
> with the proposed mechanism. Given that I am still myself reviewing
> the actual code.

The closest thing I can find to a proposal from you in that regard is
this comment:

# I was really thinking we could skip the fsync of a page if we've not
# persisted anything important on that page, since that was one of
# Robert's performance points.

It might be possible to do something with that idea, but at the moment
I'm not seeing how to make it work.

> So those things are not something we need to avoid.
>
> My feeling is that overwriting xmin is a clever idea, but arrives too
> late to require sensible analysis in this stage of the CF. It's not
> solving a problem, its just an alternate mechanism and at best an
> optimisation of the mechanism. Were we to explore it now, it seems
> certain that another person would observe that design were taking
> place and so the patch should be rejected, which would be unnecessary
> and wasteful.

Considering that nobody's done any work to resolve the uncertainty
about whether the worst-case performance characteristics of this patch
are acceptable, and considering further that it was undergoing massive
code churn for more than a month after the final CommitFest, I think
it's not that unreasonable to think it might not be ready for prime
time at this point. In any event, your argument is exactly backwards:
we need to first decide whether the patch needs a redesign and then,
if it does, postpone it. Deciding that we don't want to postpone it
first, and therefore we're not going to redesign it even if that is
what's really needed makes no sense.

> I also think it would alter our ability to diagnose
> problems, not least the normal test that xmax matches xmin across an
> update.

There's nothing stopping the new tuple from being frozen before the
old one, even today.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company