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.





- Heikki

Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:

Reply via email to