Re: INSERT...ON DUPLICATE KEY LOCK FOR UPDATE - Mailing list pgsql-hackers

On 12/30/2013 05:57 AM, Peter Geoghegan wrote:
> Now, when you actually posted the prototype, I realized that it was
> the same basic design that I'd cited in my very first e-mail on the
> IGNORE patch (the one that didn't have row locking at all) - nobody
> else wanted to do heap insertion first for promise tuples. I read that
> 2007 thread [1] a long time ago, but that recognition only came when
> you posted your first prototype, or perhaps shortly before when you
> started participating on list.

Ah, I didn't remember that thread. Yeah, apparently I proposed the exact 
same design back then. Simon complained about the dead tuples being left 
behind, but I don't think that's a big issue with the design we've been 
playing around now; you only end up with dead tuples when two backends 
try to insert the same value concurrently, which shouldn't happen very 
often. Other than that, there wasn't any discussion on whether that's a 
good approach or not.

> In short, I think that my approach may be better because it doesn't
> conflate row locking with value locking (therefore making it easier
> to reason about, maintaining modularity),

You keep saying that, but I don't understand what you mean. With your 
approach, an already-inserted heap tuple acts like a value lock, just 
like in my approach. You have the page-level locks on b-tree pages in 
addition to that, but the heap-tuple based mechanism is there too.

> I'm not sure that this is essential to your design, and I'm not sure
> what your thoughts are on this, but Andres has defended the idea of
> promise tuples that lock old values indefinitely pending the outcome
> of another xact where we'll probably want to update, and I think this
> is a bad idea. Andres recently seemed less convinced of this than in
> the past [2], but I'd like to hear your thoughts. It's very pertinent,
> because I think releasing locks needs to be cheap, and rendering old
> promise tuples invisible is not cheap.

Well, killing an old promise tuple is not cheap, but it shouldn't happen 
often. In both approaches, what probably matters more is the overhead of 
the extra heavy-weight locking. But this is all speculation, until we 
see some benchmarks.

> I said that I "have limited enthusiasm for
> expanding the modularity violation that exists within the btree AM".
> Based on what Andres has said in the recent past on this thread about
> the current btree code, that "in my opinion, bt_check_unique() doing
> [locking heap and btree buffers concurrently] is a bug that needs
> fixing" [3], can you really blame me? What this patch does not need is
> another controversy. It seems pretty reasonable and sane that we'd
> implement this by generalizing from the existing mechanism.

_bt_check_unique() is a modularity violation, agreed. Beauty is in the 
eye of the beholder, I guess, but I don't see either patch making that 
any better or worse.

>> Now, enough with the venting. Back to drawing board, to figure out how best
>> to fix the deadlock issue with the insert_on_dup-kill-on-conflict-2.patch.
>> Please help me with that.
>
> I will help you. I'll look at it tomorrow.

Thanks!

> [1] http://www.postgresql.org/message-id/1172858409.3760.1618.camel@silverbirch.site
>
> [2] http://www.postgresql.org/message-id/20131227075453.GB17584@alap2.anarazel.de
>
> [3] http://www.postgresql.org/message-id/20130914221524.GF4071@awork2.anarazel.de

- Heikki