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

On Sun, Dec 22, 2013 at 6:42 PM, Peter Geoghegan <pg(at)heroku(dot)com> wrote:
> On Fri, Dec 20, 2013 at 11:59 PM, Peter Geoghegan <pg(at)heroku(dot)com> wrote:
>> I think that the way forward is to refine my design in order to
>> upgrade locks from exclusive buffer locks to something else, managed
>> by the lock manager but perhaps through an additional layer of
>> indirection. As previously outlined, I'm thinking of a new SLRU-based
>> granular value locking infrastructure built for this purpose, with
>> btree inserters marking pages as having an entry in this table.
>
> I'm working on a revision that holds lmgr page-level exclusive locks
> (and buffer pins) across multiple operations. This isn't too
> different to what you've already seen, since they are still only held
> for an instant. Notably, hash indexes currently quickly grab and
> release lmgr page-level locks, though they're the only existing
> clients of that infrastructure. I think on reflection that
> fully-fledged value locking may be overkill, given the fact that these
> locks are only held for an instant, and only need to function as a
> choke point for unique index insertion, and only when upserting
> occurs.
>
> This approach seems promising. It didn't take me very long to get it
> to a place where it passed a few prior test-cases of mine, with fairly
> varied input, though the patch isn't likely to be posted for another
> few days. I think I can get it to a place where it doesn't regress
> regular insertion at all. I think that that will tick all of the many
> boxes, without unwieldy complexity and without compromising conceptual
> integrity.
>
> I mention this now because obviously time is a factor. If you think
> there's something I need to do, or that there's some way that I can
> more usefully coordinate with you, please let me know. Likewise for
> anyone else following.

I don't think this is a project to rush through. We've lived without
MERGE/UPSERT for several years now, and we can live without it for
another release cycle while we try to reach agreement on the way
forward. I can tell that you're convinced you know the right way
forward here, and you may be right, but I don't think you've convinced
everyone else - maybe not even anyone else.

I wouldn't suggest modeling anything you do on the way hash indexes
using heavyweight locks. That is a performance disaster, not to
mention being basically a workaround for the fact that whoever wrote
the code originally didn't bother figuring out any way that splitting
a bucket could be accomplished in a crash-safe manner, even in theory.
If it weren't for that, we'd be using buffer locks there. That
doesn't necessarily mean that page-level heavyweight locks aren't the
right thing here, but the performance aspects of any such approach
will need to be examined carefully.

To be honest, I am still not altogether sold on any part of this
feature. I don't like the fact that it violates MVCC - although I
admit that some form of violation is inevitable in any feature in this
area unless we're content to live with many serialization failures, I
don't like the particular way it violates MVCC, I don't like the
syntax (returns rejects? blech), and I don't like the fact that
getting the locking right, or even getting the semantics right, seems
to be so darned hard. I think we're in real danger of building
something that will be too complex, or just too weird, for users to
use, and too complex to maintain as well.

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