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

On 2013-12-29 19:57:31 -0800, Peter Geoghegan wrote:
> On Sun, Dec 29, 2013 at 8:18 AM, Heikki Linnakangas
> <hlinnakangas@vmware.com> wrote:
> >> My position is not based on a gut feeling. It is based on carefully
> >> considering the interactions of the constituent parts, plus the
> >> experience of actually building a working prototype.
> >
> >
> > I also carefully considered all that stuff, and reached a different
> > conclusion. Plus I also actually built a working prototype (for some
> > approximation of "working" - it's still a prototype).
> 
> Well, clearly you're in agreement with me about unprincipled
> deadlocking. That's what I was referring to here.

Maybe you should describe what you mean with "unprincipled". Sure, the
current patch deadlocks, but I don't see anything fundamental,
unresolvable there. So I don't understand what the word unprincipled
means in that sentence..

> Andres recently seemed less convinced of this than in
> the past [2], but I'd like to hear your thoughts.

Not really, I just don't have the time/energy to fight for it (aka write
a POC) at the moment.
I still think any form of promise tuple, be it index, or heap based,
it's a much better, more general, approach than yours. That doesn't
preclude other approaches from being workable though.

> I didn't say that locking index pages is obviously better, and I
> certainly didn't say anything about what you've done being
> fundamentally flawed. 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?

Uh. But that was said in the context of *your* approach being
flawed. Because it - at that time, I didn't look at the newest version
yet - extended the concept of holding btree page locks across external
operation to far much more code, even including user defined code!. And
you argued that that isn't a problem using _bt_check_unique() as an
argument.

I don't really see why your patch is less of a modularity violation than
Heikki's POC. It's just a different direction.

> > PS. In btreelock_insert_on_dup_v5.2013_12_28.patch, the language used in the
> > additional text in README is quite difficult to read. Too many difficult
> > sentences and constructs for a non-native English speaker like me. I had to
> > look up "concomitantly" in a dictionary and I'm still not sure I understand
> > that sentence :-).

+1 on the simpler language front as a fellow non-native speaker.

Personally, the biggest thing I think you could do in favor of your
position, is trying to be a bit more succinct in the mailing list
discussions. I certainly fail at times at that as well, but I really try
to work on it...

Greetings,

Andres Freund

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