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

On 2013-09-13 11:59:54 -0700, Peter Geoghegan wrote:
> On Fri, Sep 13, 2013 at 9:23 AM, Robert Haas <robertmhaas@gmail.com> wrote:
> > Andres is being very polite here, but the reality is that this
> > approach has zero chance of being accepted.
> 
> I quite like Andres, but I have yet to see him behave as you describe
> in a situation where someone proposed what was fundamentally a bad
> idea. Maybe you should let him speak for himself?

Unfortunately I have to agree with Robert here, I think it's a complete
nogo to do what you propose so far and I've several times now presented
arguments why I think so.
The reason I wasn't saying "this will never get accepted" are twofold:
a) I don't want to stiffle alternative ideas to the "promises" idea,
just because I think it's the way to go. That might stop a better idea
from being articulated. b) I am not actually in the position to say it's
not going to be accepted.

*I* think that unless you make some fundamental and very, very clever
modifications to your algorithm that end up *not holding a lock over
other operations at all*, it's not going to get committed. And I'll chip
in with my -1.
And clever modification doesn't mean slightly restructuring heapam.c's
operations.

> The importance of this patch is a value judgement. Our users have been
> screaming for this for over ten years, so to my mind it has a fairly
> high importance. Also, every other database system of every stripe
> worth mentioning has something approximately equivalent to this,
> including ones with much less functionality generally. The fact that
> we don't is a really unfortunate omission.

I aggree it's quite important but that doesn't mean we have to do stuff
that we think are unacceptable, especially as there *are* other ways to
do it.

> As to the rules you refer to, you must mean "These locks are intended
> to be short-term: they should not be held for long". I don't think
> that they will ever be held for long. At least, when I've managed the
> amount of work that a heap_insert() can do better. I expect to produce
> a revision where toasting doesn't happen with the locks held soon.
> Actually, I've already written the code, I just need to do some
> testing.

I personally think - and have stated so before - that doing a
heap_insert() while holding the btree lock is unacceptable.

> The btree code is different, though: It implements a well-defined
> interface, with much clearer separation of concerns.

Which you're completely violating by linking the btree buffer locking
with the heap locking. It's not about the btree code alone.

At this point I am a bit confused why you are asking for review.

> I mean, if we do the promise tuple thing, and there are multiple
> unique indexes, what happens when an inserter needs to block pending
> the outcome of another transaction?  They had better go clean up the
> promise tuples from the other unique indexes that they're trying to
> insert into, because they cannot afford to hold value locks for a long
> time, no matter how they're implemented.

Why? We're using normal transaction visibility rules here. We don't stop
*other* values on the same index getting updated or similar.
And anyway. It doesn't matter which problem the "promises" idea
has. We're discussing your proposal here.

Greetings,

Andres Freund

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