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

On Mon, Dec 23, 2013 at 5:59 PM, Peter Geoghegan <pg(at)heroku(dot)com> wrote:
> On Mon, Dec 23, 2013 at 7:49 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>> 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.
>
> That may be. Attention from reviewers has been in relatively short
> supply. Not that that isn't always true.

I think concrete concerns about usability have largely been
subordinated to abstruse discussions about locking protocols. A
discussion strictly about what syntax people would consider
reasonable, perhaps on another thread, might elicit broader
participation (although this week might not be the right time to try
to attract an audience).

> Having looked at the code for the first time recently, I'd agree that
> hash indexes are a disaster. A major advantage of The Lehman and Yao
> Algorithm, as prominently noted in the paper, is that exclusive locks
> are only acquired on leaf pages to increase concurrency. Since I only
> propose to extend this to a heavyweight page lock, and still only for
> an instant, it seems reasonable to assume that the performance will be
> acceptable for an initial version of this. It's not as if most places
> will have to pay any heed to this heavyweight lock - index scans and
> non-upserting inserts are generally unaffected. We can later optimize
> performance as we measure a need to do so. Early indications are that
> the performance is reasonable.

OK.

>> 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
>
> Discussions around visibility issues have not been very useful. As
> I've said, I don't like the term "MVCC violation", because it's
> suggestive of some classical, codified definition of MVCC, a
> definition that doesn't actually exist anywhere, even in research
> papers, AFAICT.

I don't know whether or not that's literally true, but like Potter
Stewart, I don't think there's any real ambiguity about the underlying
concept. The concepts of read->write, write->read, and write->write
dependencies between transactions are well-described in textbooks such
as Jim Gray's Transaction Processing: Concepts and Techniques and this
paper on MVCC:

http://citeseerx.ist.psu.edu/viewdoc/download?doi=10.1.1.142.552&rep=rep1&type=pdf

I think the definition of an MVCC violation is that a snapshot sees
the effects of a transaction which committed after that snapshot was
taken. And maybe it's good and right that this patch is introducing a
new way for that to happen, or maybe it's not, but the point is that
we get to decide.

> I've been very consistent even in the face of strong criticism. What I
> have now is essentially the same design as back in early September.
> After the initial ON DUPLICATE KEY IGNORE patch in August, I soon
> realized that value locking and row locking could not be sensibly
> considered in isolation, and over the objections of others pushed
> ahead with integrating the two. I believe now as I believed then that
> value locks need to be cheap to release (or it at least needs to be
> possible), and that it was okay to drop all value locks when we need
> to deal with a possible conflict/getting an xid shared lock - if those
> unlocked pages have separate conflicts on our next attempt, the
> feature is being badly misused (for upserting) or it doesn't matter
> because we only need one conclusive "No" answer (for IGNOREing, but
> also for upserting).

I'm not saying that you haven't been consistent, or that you've done
anything wrong at all. I'm just saying that the default outcome is
that we change nothing, and the fact that nobody's been able to
demonstrate an approach is clearly superior to what you've proposed
does not mean we have to accept what you've proposed. I am not
necessarily advocating for rejecting your proposed approach, although
I do have concerns about it, but I think it is clear that it is not
backed by any meaningful amount of consensus. Maybe that will change
in the next two months, and maybe it won't. If it doesn't, whether
through any fault of yours or not, I don't think this is going in. If
this is all perfectly clear to you already, then I apologize for
belaboring the point.

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