Re: foreign key locks, 2nd attempt

On Mon, Feb 27, 2012 at 2:47 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Thu, Feb 23, 2012 at 11:01 AM, Alvaro Herrera
> <alvherre(at)commandprompt(dot)com> wrote:
>>> This
>>> seems like a horrid mess that's going to be unsustainable both from a
>>> complexity and a performance standpoint.  The only reason multixacts
>>> were tolerable at all was that they had only one semantics.  Changing
>>> it so that maybe a multixact represents an actual updater and maybe
>>> it doesn't is not sane.
>>
>> As far as complexity, yeah, it's a lot more complex now -- no question
>> about that.
>>
>> Regarding performance, the good thing about this patch is that if you
>> have an operation that used to block, it might now not block.  So maybe
>> multixact-related operation is a bit slower than before, but if it
>> allows you to continue operating rather than sit waiting until some
>> other transaction releases you, it's much better.
>
> That's probably true, although there is some deferred cost that is
> hard to account for.  You might not block immediately, but then later
> somebody might block either because the mxact SLRU now needs fsyncs or
> because they've got to decode an mxid long after the relevant segment
> has been evicted from the SLRU buffers.  In general, it's hard to
> bound that latter cost, because you only avoid blocking once (when the
> initial update happens) but you might pay the extra cost of decoding
> the mxid as many times as the row is read, which could be arbitrarily
> many.  How much of a problem that is in practice, I'm not completely
> sure, but it has worried me before and it still does.  In the worst
> case scenario, a handful of frequently-accessed rows with MXIDs all of
> whose members are dead except for the UPDATE they contain could result
> in continual SLRU cache-thrashing.

Cases I regularly see involve wait times of many seconds.

When this patch helps, it will help performance by algorithmic gains,
so perhaps x10-100.

That can and should be demonstrated though, I agree.

> From a performance standpoint, we really need to think not only about
> the cases where the patch wins, but also, and maybe more importantly,
> the cases where it loses.  There are some cases where the current
> mechanism, use SHARE locks for foreign keys, is adequate.  In
> particular, it's adequate whenever the parent table is not updated at
> all, or only very lightly.  I believe that those people will pay
> somewhat more with this patch, and especially in any case where
> backends end up waiting for fsyncs in order to create new mxids, but
> also just because I think this patch will have the effect of
> increasing the space consumed by each individual mxid, which imposes a
> distributed cost of its own.

That is a concern also.

It's taken me a while reviewing the patch to realise that space usage
is actually 4 times worse than before.

> I think we should avoid having a theoretical argument about how
> serious these problems are; instead, you should try to construct
> somewhat-realistic worst case scenarios and benchmark them.  Tom's
> complaint about code complexity is basically a question of opinion, so
> I don't know how to evaluate that objectively, but performance is
> something we can measure.  We might still disagree on the
> interpretation of the results, but I still think having some real
> numbers to talk about based on carefully-thought-out test cases would
> advance the debate.

It's a shame that the isolation tester can't be used directly by
pgbench - I think we need something similar for performance regression
testing.

So yes, performance testing is required.

--
 Simon Riggs                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services