Re: INSERT...ON DUPLICATE KEY LOCK FOR UPDATE

On Wed, Sep 11, 2013 at 8:47 PM, Peter Geoghegan <pg@heroku.com> wrote:
> In practice the vast majority of insertions don't involve TOASTing.
> That's not an excuse for allowing the worst case to be really bad in
> terms of its impact on query response time, but it may well justify
> having whatever ameliorating measures we take result in bloat. It's at
> least the kind of bloat we're more or less used to dealing with, and
> have already invested a lot in controlling. Plus bloat-wise it can't
> be any worse than just inserting the tuple and having the transaction
> abort on a duplicate, since that already happens after toasting has
> done its work with regular insertion.

Andres is being very polite here, but the reality is that this
approach has zero chance of being accepted.  You can't hold buffer
locks for a long period of time across complex operations.  Full stop.It's a violation of the rules that are clearly
documentedin
 
src/backend/storage/buffer/README, which have been in place for a very
long time, and this patch is nowhere near important enough to warrant
a revision of those rules.  We are not going to risk breaking every
bit of code anywhere in the backend or in third-party code that takes
a buffer lock.  You are never going to convince me, or Tom, that the
benefit of doing that is worth the risk; in fact, I have a hard time
believing that you'll find ANY committer who thinks this approach is
worth considering.

Even if you get the code to run without apparent deadlocks, that
doesn't mean there aren't any; it just means that you haven't found
them all yet.  And even if you managed to squash every such hazard
that exists today, so what?  Fundamentally, locking protocols that
don't include deadlock detection don't scale.  You can use such locks
in limited contexts where proofs of correctness are straightforward,
but trying to stretch them beyond that point results not only in bugs,
but also in bad performance and unmaintainable code.  With a much more
complex locking regimen, even if your code is absolutely bug-free,
you've put a burden on the next guy who wants to change anything; how
will he avoid breaking things?  Our buffer locking regimen suffers
from painful complexity and serious maintenance difficulties as is.
Moreover, we've already got performance and scalability problems that
are attributable to every backend in the system piling up waiting on a
single lwlock, or a group of simultaneously-held lwlocks.
Dramatically broadening the scope of where lwlocks are used and for
how long they're held is going to make that a whole lot worse.  What's
worse, the problems will be subtle, restricted to the people using
this feature, and very difficult to measure on production systems, and
I have no confidence they'd ever get fixed.

A further problem is that a backend which holds even one lwlock can't
be interrupted.  We've had this argument before and it seems that you
don't think that non-interruptibility is a problem, but it project
policy to allow for timely interrupts in all parts of the backend and
we're not going to change that policy for this patch.  Heavyweight
locks are heavy weight precisely because they provide services - like
deadlock detection, satisfactory interrupt handling, and, also
importantly, FIFO queuing behavior - that are *important* for locks
that are held over an extended period of time.  We're not going to go
put those services into the lightweight lock mechanism because then it
would no longer be light weight, and we're not going to ignore the
importance of them, either.

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